0

I am trying to load an unknown amount of data from a file in to a linked list

Load Function

void load(FILE *file, Node **head) // while feof somewhere
{
    char tempArtist[30]={'\0'}, tempAlbum[30]={'\0'}, tempTitle[30]={'\0'}, tempGenre[30]={'\0'},tempSpace='\0';
    char tempPlay[100]={'\0'}, tempRating[6]={'\0'}, tempMins[8]={'\0'}, tempSecs[8]={'\0'};

    Data *temp;

    temp=(Data*)malloc(sizeof(Data));

            while(!feof(file))
            {
            fscanf(file,"%s",tempArtist);
            fscanf(file,"%s",tempAlbum);
            fscanf(file,"%s",tempTitle);
            fscanf(file,"%s",tempGenre);
            fscanf(file,"%s",tempMins);
            fscanf(file,"%s",tempSecs);
            fscanf(file,"%s",tempPlay);
            fscanf(file,"%s",tempRating);

            temp->mins=strdup(tempMins);
            temp->secs=strdup(tempSecs);
            temp->album=strdup(tempAlbum);
            temp->artist=strdup(tempArtist);
            temp->genre=strdup(tempGenre);
            temp->song=strdup(tempTitle);
            temp->played=strdup(tempPlay);
            temp->rating=strdup(tempRating);    

            insertFront(head,temp);

            }

}

The problem I am having is that when I go to print out the list all the entries are the same as the last one read from the file. This is something to do with the strdup() but I can't get it to copy to the data type without getting access violations.

What is another method(proper method) that I could use to copy the strings read from the file and pass them to the insert?

InsertFront

void insertFront(Node **head, Data *data)
{
    Node *temp=makeNode(data);

    if ((*head) == NULL)
    {
        *head=temp;
    }
    else
    {
        temp->pNext=*head;
        *head=temp;
    }

}

Data struct

typedef struct data
{
    char *artist;
    char *album;
    char *song;
    char *genre;
    char *played;
    char *rating;
    char *mins;
    char *secs;
}Data;

testing file

snoop
heartbeat
swiggity
rap
03
10
25
4
hoodie
cakeboy
birthday
hiphop
02
53
12
5
Montop20
  • 23
  • 6
  • With your `struct` definition, you can't `strdup` to `temp->album`, which is an array. (In my compiler, the assignment is an error.) You could, however, `strcpy` to it. Why not make the `album` field a pointer, too, for consistency? – M Oehm Mar 25 '15 at 06:33
  • In this case, it might be useful to know the contents of your file. (Or an exceprt that reproduces thererror, if the file is large.) – M Oehm Mar 25 '15 at 06:37
  • @MOehm It was supposed to be a pointer, I was messing around to see if that might fix the issue. I'll add the file contents. – Montop20 Mar 25 '15 at 06:46

2 Answers2

1

You use the same instance of temp for all your lines. The allocation should go inside the loop, ideally after you have established that the whole entry was read in successfully.

By the way, feof is not a good way to control input loops. You should check the return value of fscanf instead.

while (1) {
    if (fscanf(file,"%s",tempAlbum) < 1 ) break;
    if (fscanf(file,"%s",tempArtist) < 1) break;
    // ...

    temp = (Data *) malloc(sizeof(Data));

    temp->album=strdup(tempAlbum);
    temp->artist=strdup(tempArtist);
    // ...

    insertFront(head, temp);
}

Note how the allocation of a new node happens only after a whole record was read.

There are other ways to improve the code. For example you have very short buffers for the strings that contain numerical data. What if there is a line slip-up and you read a longer line into into such a short buffer? Also, your input seems to be line-wise, so it might be good to use fgets instead of fscan(f, "%s"), which will only read "words" and will have problems with lines that have spaces.

Community
  • 1
  • 1
M Oehm
  • 28,726
  • 3
  • 31
  • 42
0

while(!eof) is wrong. You can do like.

for (;;) {
    ret = fscanf()
    if(ret == EOF) break;
}
sdev
  • 115
  • 1
  • 1
  • 6