Re: Bug analysis

jacob navia said:

C unleashed page 266, Listing 8.2
---------------------------------
char *ReadTextFile(FILE *fp,int *Error)
{
size_t size=0;
size_t len;
char *p = NULL;
char *q;
char buffer[120];

*Error = 0;
while (fgets(buffer, sizeof buffer, fp) {
len = strlen(buffer);
q = realloc(p,size+len);
if (q != NULL) {
p = q;
strcpy(p+size,buffer);
size += len;
}
else *Error = 1;
}
return p;
}
---------------------------------------
Let's follow this program.

The first time, the fgets function fills our buffer with a line or
119 characters and the terminating zero. In any case, we have a
well formed string in buffer.

The reallocation asks for "len" more characters, forgetting that
we will copy the terminating zero with strcpy into the string.

Thank you for bringing this code to my attention. In fact it has two
bugs, neither of which is to do with the realloc, which is actually
correct.

Firstly, size should be initialised to 1, not 0. This is a mistake
also made elsewhere in the book.

Secondly, the strcpy should deduct 1 from size+len when working out
where to write the new data.

Note that if the realloc fails, the program will go on trying to
reallocate the buffer, what probably will always fail. This is
not really a bug, but it would have been obviously better to add
a "break" statement to stop reading after we can't reallocate.

It's a fair point, and in my view it /is/ a bug.

I have modified the relevant errata page accordingly.

--
Richard Heathfield <http://www.cpax.org.uk>
Email: -http://www. +rjh@
Google users: <http://www.cpax.org.uk/prg/writings/googly.php>
"Usenet is a strange place" - dmr 29 July 1999
.

Relevant Pages

  • Bug analysis
    ... char *ReadTextFile ... the fgets function fills our buffer with a line or 119 ... The reallocation asks for "len" more characters, ... this bug can very well go completely undetected in many occasions giving ...
    (comp.lang.c)
  • Re: Bug analysis
    ... char *ReadTextFile ... while (fgets(buffer, sizeof buffer, fp) { ... and in my view it /is/ a bug. ... Well, no, it is better to change the realloc. ...
    (comp.lang.c)
  • Re: Bug analysis
    ... char *ReadTextFile ... while (fgets(buffer, sizeof buffer, fp) { ... and in my view it /is/ a bug. ... Well, no, it is better to change the realloc. ...
    (comp.lang.c)
  • Re: Bug analysis
    ... char *ReadTextFile ... while (fgets(buffer, sizeof buffer, fp) { ... a "break" statement to stop reading after we can't reallocate. ...
    (comp.lang.c)
  • Re: Cannot return values of char variable
    ... - buffer = ... Since you seem to be trying to return a char pointer ... int id = random; ... content is interpreted as a string. ...
    (comp.lang.c)