0

I have a simple file formatted as such

NAME|VALUE
NAME|VALUE
NAME|VALUE

I am trying to read these in, and store them in an array of structs, the struct is this

struct data
{
    char* name;
    char* value;
};

For now, I know the size of the array will be 3, so I did this:

struct data pairs[3];

Here is my code as I am trying to read it in from the file:

char *tempVal;
int i =0;
    if(file != NULL)
        {
            char curLine [128];
            while(fgets(curLine, sizeof curLine, stockFile) != NULL)
            {   

                tempVal = strtok(curLine,"|");
                printf("i:%i\n",i);
                pairs[i].name= tempVal;
                printf("name at pos %i is %s\n",i, pairs[i].name);
                tempVal = strtok(NULL,"|");
                pairs[i].value= tempVal;
                printf("value at pos %i is %s\n",i, pairs[i].value);
                ++i;
            }
            fclose(file);
        }

and each of those printf statments prints the correct thing along the way, then I try to print the array with this

int j
for(j = 0; j < 3; j++)
    {   

        printf("ENTRY# %i\NAME:%s\VALUE:%s\n\n",j,pairs[j].name, pairs[j].value);
    }

Sorry the indentation is a little weird, tried messing with the code blocks but couldn't get it quite perfect. However, I am wondering why it shows the correct thing during the while loop as it goes along, but then after it is complete the for loop shows all three of the array entries having the same name(the value is correct for the third entry but for the first and second entries the value field contains half of the correct value for the third entry)

Thanks!

Andrew
  • 830
  • 3
  • 10
  • 27

1 Answers1

1

The value returned from strtok() will be pointing to an element in curLine, so all entries in the array of structs will be pointing to an element in curLine which is overwritten with each call to fgets() (and will only be valid for the current iteration).

You should make a copy of the value returned from strtok() during the while, possibly using strdup():

while(fgets(curLine, sizeof curLine, stockFile) != NULL)
{   
    tempVal = strtok(curLine,"|");
    printf("i:%i\n",i);
    pairs[i].name = strdup(tempVal);
    printf("name at pos %i is %s\n",i, pairs[i].name);
    tempVal = strtok(NULL,"|");
    pairs[i].value = strdup(tempVal);
    printf("value at pos %i is %s\n",i, pairs[i].value);
    ++i;
}

free() them later when no longer required:

for (j = 0; j < 3; j++)
{
    free(pairs[j].name);
    free(pairs[j].value);
}
hmjd
  • 120,187
  • 20
  • 207
  • 252
  • Updated answer for `free()`. You need to copy the values so that the pairs in the array are not referencing `curLine`, which is changing with each call to `fgets()`. – hmjd May 15 '12 at 16:43
  • Okay, that makes sense because the curLine pointer is changing with each call and I can't have the struct value referencing that pointer. Freeing them will do what exactly? – Andrew May 15 '12 at 16:45
  • `strdup()` allocates memory (using `malloc()`), which must be `free()`d to avoid a memory leak. – hmjd May 15 '12 at 16:46
  • Makes sense, I'm using gcc and when I try to run the program with the for loop at the end to free the memory, it gives me a warning: "incompatible implicit declaration of built-in function 'free' [enabled by default]" – Andrew May 15 '12 at 16:49
  • Oh, right, thanks. Also, is there any other way to do this with out strdup() ? I am thinking there should be a way to take a copy of the value returned from strtok, store the value not as a pointer and set the to the value in the struct, or am I just confused on how it works? – Andrew May 15 '12 at 16:54
  • An alternative would be to have `data.name` and `data.value` as `char[]` rather than `char*`. BUT, you need to ensure that token returned by `strtok()` is not bigger than the size of `name` and `value` arrays. You could then use `strncpy()` instead of `strdup()`. – hmjd May 15 '12 at 16:56
  • let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/11271/discussion-between-andrew-and-hmjd) – Andrew May 15 '12 at 17:02
  • That make sense, thank you. Last question, I have the .value member stored as char* and the value is correct now. I want to convert it to a double or float, I tried atof and strtod and both gave me garbage as answers, why and how do I correct that? – Andrew May 15 '12 at 17:03
  • 1
    @Andrew, can you post another question for that? Is is different from the title of this question and will attract attentions from other people on SO and would be more useful. – hmjd May 15 '12 at 17:05
  • here is the other question http://stackoverflow.com/questions/10605653/converting-char-to-float-or-double – Andrew May 15 '12 at 17:11
  • are you `free()`ing before using `atof()` and `strtod()`? – hmjd May 15 '12 at 17:22
  • No, I didn't free anything until the end of the program. Is there someway I could use atof/strtod as I read in the file like atof(strdup(..)) but then I don't know when/how I would free it – Andrew May 15 '12 at 17:28
  • see if you can help me with this too, thank you. http://stackoverflow.com/questions/10607027/reading-from-file-and-using-strtod-program-misses-last-line-in-file – Andrew May 15 '12 at 18:54