Value inside a double pointer isn't updated correctly (C)

Question

I have a function that reads an input file and is supposed to modify the contents of a char** and a int*. The function is as follows:

void
input_parser(arguments* args, char** input, int* files) {
   char buffer[MAX];
   FILE *fr;
   fr = fopen(args->file,"r");
   if (fr == NULL) {
       printf("No correct input file was entered\n");
       exit(0);
   } 
   while(fgets(buffer,MAX,fr) != NULL) {
       input[*files] = strtok(buffer,"\n");
       (*files)++;
   }
   fclose(fr);
   return;
}

I have defined input and files as follows in the main program:

char* input[25];
files = 0;

I call the function as follows:

input_parser(args, input, &files);

The input file contains 3 lines as follows:

output1.xml
output2.xml
output3.xml

I notice that during the while loop the 'current' value is read correctly but stored in all input[*] resulting in:

input[0] = output3.xml
input[1] = output3.xml
input[2] = output3.xml

I would greatly appreciate if someone has any idea what is going wrong here.

The core problem is that your file reading function, as it is designed, has to concern itself with a lot of things that are not related to file handling. Since you are reading some sort of xml files, I will assume that the format and length of those files are known at compile time. Correct? If so, there is no need for dynamic memory. You should be able to allocate exactly as much memory as needed, in the form of a true 2D array in the caller. Then pass this 2D array and its sizes to the file reading function. Make sure that the function does not read out of bounds. — Lundin, Oct 17 '12 at 14:38

hmjd · Accepted Answer · 2012-10-17T14:22:54.867

3

The function is storing the address of the local variable buffer to each element in the input array: you need to copy the value returned by strtok(). The code as it stands is undefined behaviour as the buffer is out of scope once input_parser() returns, even it was not the logic is incorrect anyway.

If you have strdup(), you just use it:

input[*files] = strdup(strtok(buffer,"\n")); /* NULL check omitted. */

otherwise malloc() and strcpy(). Remember to free() the elements of input when no longer required.

Initialise input to be able determine which elements point to valid strings:

char* input[25] = { NULL };

edited Oct 17 '12 at 14:22

answered Oct 17 '12 at 14:17

hmjd

120,187
20
207
252

Shouldn't I use strcpy than as strdup would also allocate memory even if input[x] is allready assigned as a char[25]? – Bas Jansen Oct 17 '12 at 14:31
I assigned '\0' to all values of input in the main but did not include that chunk in the original question. I use that in other parts to check how many elements are in there. – Bas Jansen Oct 17 '12 at 14:32
@BasJansen, you need to allocate memory to store the string. `input` is an array of 25 `char*`: there is nowhere to store the characters of a string directly in that array. If you are going to use `strcpy()` you _must_ allocate memory to copy into to, using `malloc()`. – hmjd Oct 17 '12 at 14:33
I'm not really a fan of dynamic memory allocation but it does the trick. I feel a bit silly now for forgetting to not allocate memory for where input was pointing to. – Bas Jansen Oct 17 '12 at 14:37
@BasJansen I don't think there is a single programmer who is a fan of dynamic memory :) But it is a necessary evil in various desktop applications. As for this specific case, you might not need dynamic memory. It depends on the nature of the xml files you are reading. – Lundin Oct 17 '12 at 14:40

score 0 · Answer 2 · answered Oct 17 '12 at 14:17

0

You are going to end up having danging pointers, which are pointing inside your buffer after the buffer has been deallocated.

answered Oct 17 '12 at 14:17

Vaughn Cato

63,448
5
82
132

Value inside a double pointer isn't updated correctly (C)

2 Answers2