0

I wrote a C code to implement a dictionary using linked-list(nodes are in sorted order) I want to save the data to a file and be able to reload the data when I run the program next time. I'm having trouble loading the data from file. Here is my code for reading and writing data:

struct node{
    char word[20];
    char meaning[5][100]; //2D array for saving multiple meanings of a word
    struct node *next;
};

void ReadData(struct node *head)
{
    struct node *tail;
    FILE *fp = fopen("dictionary.data", "rb");
    if(fp == NULL)
    {
        printf("Error opening file..\n");
        return;
    }
    while(!feof(fp))
    {
        tail = (struct node*)calloc(1, sizeof(struct node));
        fread(tail->word, sizeof(head->word), 1, fp);
        fread(tail->meaning, sizeof(head->meaning), 1, fp);
        if(head == NULL) //for fresh run head is initialized with NULL
        {
            tail->next = head;
            head = tail;
        }
        else
        {
            head->next = tail;
            head = head->next;
        }
    }
    fclose(fp);
}

I can't load data from the file to the linked list. Code is not working. I can't figure out where the problem is..
Here's how I'm writing the data to the file :

/*I think this code is working because size of the file increases after running the code*/
void WriteData(struct node *head)
{
    FILE *fp = fopen("dictionary.data", "wb");
    if(fp == NULL)
    {
        printf("Error opening file..\n");
        return;
    }
    while(head != NULL)
    {
        fwrite(head->word, sizeof(head->word), 1, fp);
        fwrite(head->meaning, sizeof(head->meaning), 1, fp);
        head = head->next;
    }
    fclose(fp);
}

I've used sizeof, instead of strlen, it's a string. It'll have null character at the end - no problem with string. It'll consume more memory though.

surjit
  • 338
  • 4
  • 15
  • that one is not for reading.. It's for writing the data to the next node. Reading will be done by `fread` – surjit May 30 '17 at 15:52
  • in `ReadData` reading is done by `fread` `if-else` part is for fixing the links between the nodes – surjit May 30 '17 at 15:55
  • in `else` part `head->next = tail` assigns the address of new node to the `next` file of the node pointed by `head`. Now we have to point the head to the new node(tail), address of the new node is assigned to the `next` filed of the previous node(`head->next = tail`). So in order to point the `head` to the new node I used `head = head->next` as `head->next`=address of `tail` – surjit May 30 '17 at 16:07
  • ok.. But I think it's the same thing. coz `head->next` holds the address of `tail` – surjit May 30 '17 at 16:09
  • I didn't get it, why do I need to access previous nodes. creating new nodes and linking them to the last node of the structure should be good enough.. – surjit May 30 '17 at 16:14
  • What exactly are you suggesting.. I'm not getting it.. – surjit May 30 '17 at 16:15
  • 1
    The main problem is that you can not change caller 's `head` in the function. In `ReadData`, no argument is necessary. `head` should be returned. – BLUEPIXY May 30 '17 at 16:18
  • @xing `head->next=tail` means we're assigning the address of `tail` to the `next` field of the node pointed by `head`, `head` is not a node, it's pointer pointing to the starting address of a node. – surjit May 30 '17 at 16:26
  • @BLUEPIXY I should pass the head by reference then, I think the other problem is `meaning` is a 2D array, I have to write the data one by one with loop. But how do I read it, coz different words will have different no of meanings. meaning field will vary with different words – surjit May 30 '17 at 16:32
  • I think that it is not a problem because you are reading and writing the entire (2D)array. (There is no problem if it is initialized before use) – BLUEPIXY May 30 '17 at 16:38
  • @BLUEPIXY where is the problem then, I tried passing the `head` by reference, but still the code is not working. Can I give you my full code. So that you can see where the fault is?? – surjit May 30 '17 at 16:44
  • E.g in main side. `struct node *head = NULL; ReadData(&head);`... in function side `void ReadData(struct node **head){ struct node *temp_head = *head; ... make list ... *head = temp_head; return ;}` – BLUEPIXY May 30 '17 at 16:50
  • And Read [Why is “while ( !feof (file) )” always wrong?](https://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong) – BLUEPIXY May 30 '17 at 16:51
  • make list (When keeping order): `tail->next = NULL; if(head == NULL) { curr = head = tail; } else { curr = curr->next = tail;}` – BLUEPIXY May 30 '17 at 17:42
  • the addresses in the linked list mean nothing when the file is read in. So the process of reading the data must rebuild the linked list from the data read in. Amongst other things, the file does not need the linking parameters, just the data. – user3629249 May 31 '17 at 15:40
  • @user3629249 That's why I'm copying only the data, i.e `word` and `meaning`. Check my code again – surjit Jun 01 '17 at 07:36

1 Answers1

2

try this (untested):

void ReadData(struct node **head){//or struct node *ReadData(void){ ... return head; }
    struct node temp = { .next = NULL };//{ {0}, {{0}}, NULL}
    struct node *hp, *curr;
    FILE *fp = fopen("dictionary.data", "rb");

    if(fp == NULL){
        printf("Error opening file..\n");
        return;
    }

    hp = *head;
    while(fread(temp.word, sizeof(temp.word), 1, fp) && fread(temp.meaning, sizeof(temp.meaning), 1, fp)){
        struct node *np = malloc(sizeof(*np));
        if(np == NULL){
            perror("couldn't make new node by malloc:");
            return ;//release list
        }
        *np = temp;

        if(hp == NULL){//for fresh run head is initialized with NULL
            curr = hp = np;
        } else {//If *head isn't NULL, you need to move to the last node first.
            curr = curr->next = np;
        }
    }
    fclose(fp);
    *head = hp;
}
//....................
int main(void){
    //...
    struct node *head = NULL;
    //...
    ReadData(&head);
    //...
BLUEPIXY
  • 39,699
  • 7
  • 33
  • 70
  • a `void` function cannot be returning `NULL`. and if `malloc()` fails, then must pass all pointers to `free()`, otherwise there is a massive memory leak. – user3629249 Jun 01 '17 at 15:11
  • @user3629249 Yes, it was my mistake that Null's return was. Thank you for pointing out. I just pointed out the release of the list when Malloc failed, as it is not central as an example. – BLUEPIXY Jun 01 '17 at 15:36