CODEWITHSUNDEEP
coverwriteprintf
Dubos
Dubos

Reputation: 3

C: vsprintf overwriting an array

I'm having a problem when using the function vsprintf.

I have 3 functions to open, close and write to a XML file. The open function stores the first word of the input text in an array, and the close function closes the tag with that word. The problem is that the array where I store the close tag to use gets overwritten in each call to the open or the write functions (even when the write function hasn't any reference to the array used to store the close tag).

int xml_level = 0;
char *xml_header[64];
FILE *xml_out;

void xmlopen(const char *format, ...){
   char buffer[256];
   va_list arglist;
   va_start(arglist,format);
   vsprintf(buffer,format,arglist);
   va_end(arglist);
   int i;
   for(i=0; i<xml_level; i++){
      fprintf(xml_out,"\t");  
   }
   fprintf(xml_out,"<%s>\n",buffer);
   xml_header[xml_level] = strtok (buffer, " ");
   xml_level++;
}

void xmlclose(){
   xml_level--;
   int i;
   for(i=0; i<xml_level; i++){
      fprintf(xml_out,"\t");  
   }
   fprintf(xml_out,"</%s>\n",xml_header[xml_level]);
}

void xmlwrite(const char *format, ...){
   char buffer[256];
   va_list arglist;
   va_start(arglist,format);
   vsprintf(buffer,format,arglist);
   va_end(arglist);
   int i;
   for(i=0; i<xml_level; i++){
      fprintf(xml_out,"\t");  
   }
   fprintf(xml_out,"<%s/>\n",buffer);
}

Example of use:

xmlopen("Hello Word");
xmlopen("Foo Bar");
xmlwrite("Potato");
xmlwrite("Sentence longer than the other ones");
xmlclose();
xmlclose();

Example of output:

<Hello Word>
        <Foo Bar>
                <Potato/>
                <Sentence longer than the other ones/>
        </Sentence longer than the>
</Sentence longer than the>

Where it should be:

<Hello Word>
        <Foo Bar>
                <Potato/>
            <Sentence longer than the other ones/>
        </Foo>
</Hello>

Thank you.

Upvotes: 0

Views: 280

Answers (2)

Jinghao Shi
Jinghao Shi

Reputation: 1087

Solution

Change this line:

xml_header[xml_level] = strtok (buffer, " ");

to

xml_header[xml_level] = strdup (strtok(buffer, " "));

And remember to free xml_headers when your program exit.

[UPDATE] Of course, you also need to check possible corner cases such as strtok returns NULL, etc...

Explanation

strtok won't allocate extra storage for the returned tokens. My personal suspect is that it will subsititute the delimiters with \0 in place and return you the pointer to the start of next token every time.

Note that in your code, you allocate 256 bytes of buffer both in xmlopen and xmlwrite at the beginning, and recall that this buffer will be allocated in stack. So in your calls to xmlopen or xmlwrite, buffer will actually point to the same address (you can print its value to verify this printf("buffer is %p\n", buffer), it's 0xbff2481c on my machine).

First, you call xmlopen("Hello World"), and xml_header[0] will point to "Hello", which is also the start of buffer. Then you call xmlopen("Foo Bar"), and xml_header[1] will point to "Foo", which also the start of buffer. Then you call xmlwrite("Portato") and xmlwrite("Sentence longer than the other ones"). Note that at this point, xml_header[1] still point to the start of buffer, which now is "Sentence longer than the other ones". So when you call xmlclose(), it will print out that sentence instead of your expected token, which is overwrite by your later buffer.

Interestingly, if you allocate different buffer size in xmlopen and xmlwrite, say 256 bytes for xmlopen and 128 bytes for xmlwrite, then you will see that xmlclose will print out some mess codes which is not readable.

You can verify all these by inspect (e.g., print) the values of xml_header[0], xml_header[1] and buffer.

Upvotes: 1

Paul R
Paul R

Reputation: 213060

You problem is that you only have one local variable called buffer in xmlopen and you're (a) storing pointers to it for use outside the function (undefined behaviour) and (b) trying to use it across multiple calls (logic bug).

You need to allocate storage for the strings returned by strtok, and make sure you dispose of them later, e.g. change:

xml_header[xml_level] = strtok (buffer, " ");

to:

char * s = strtok(buffer, " ");
if (s != NULL)
{
    xml_header[xml_level] = strdup(s);
}

(Disposing of the these strings later when they are no longer needed is left as an exercise for the reader.)

Upvotes: 1

Related Questions