Re: Bug analysis

On 11 Jan, 22:52, jacob navia <ja...@xxxxxxxxxx> wrote:
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;}

---------------------------------------
S N I P

The writer of this code is an experienced C programmer. The fact that he
has this bug, that is a classical bug with zero terminated strings,
proves ONCE AGAIN that it is better to use counted strings where the
programmer has less bug surface.

For instance, in the implementation of the string library in lcc-win you
would write

String str = StrFromFile("text.dat",0); // Zero for text mode

and you would obtain a string, eliminating the need of all that code
above.
------------------------------------------
What is "bug surface"?

It is the amount of details that the programmer must keep in mind when
using the libraries of the programming environment.

Boring details, lead to more bugs. Higher level functions lead to
less bugs.

I agree that more details increase the probability of bugs.
But I don't see how having a string length would help on
this occasion. What would help would be to have arrays
whose size increases dynamically without explicit
intervention by the programmer. Writing such a library in
a portable manner is easy enough.

Having said that, I believe that what led to the bugs on
this occasion, apart from perhaps the programmer having
a bad day, was making the code unnecessarily complicated.
Here's my effort:

#include <stdio.h>
#include <stdlib.h>

char *ReadTextFile(FILE *fp,int *Error) {
size_t space_available = 0 ,
pos = 0 ;
const size_t increase = 1000 ;
char *p = 0 , *q ;
int a ;

#define place_in_string(a) { \
if ( pos == space_available ) { \
space_available += increase ; \
q = realloc(p , space_available) ; \
if ( q == 0 ) { \
*Error = 1 ; \
return p ; \
} \
p = q ; \
} \
p[pos++] = (a) ; \
}

while ( (a = fgetc(fp)) != EOF ) {
place_in_string(a)
}
if ( ferror(fp) ) *Error = 2 ;
else *Error = 0 ;
place_in_string(0)
return p ;

#undef place_in_string
}

int main(void) {
int err ;
char *p = ReadTextFile(stdin , &err) ;
if (err) return EXIT_FAILURE ;
printf("%s",p) ;
return 0 ;
}

The only bug it had was forgetting the *Error = 0 line. It's
longer than the original code but it's much easier to
follow and it tests for file error. Even this is not 100%
robust. Ideally it should check for overflow of
space_available += increase and perhaps also check that we
haven't read a 0 byte from the file but these are easy
enough to add and the code would still be clear.

One deficiency of C which can be seen from the above code
is that it doesn't allow you to define functions within
functions. It would be cleaner if place_in_string() could be
written as a function while still being able to share with
ReadTextFile() the variables place_in_string() uses. So
ideally I would want to be able to write this:

char *ReadTextFile(FILE *fp,int *Error) {
size_t space_available = 0 ,
pos = 0 ;
const size_t increase = 1000 ;
char *p = 0 ;
int a ;

void place_in_string( int a ) {
char *q ;

if ( pos == space_available ) {
space_available += increase ;
q = realloc(p , space_available) ;
if ( q == 0 ) {
*Error = 1 ;
return_from ReadTextFile p ;
/* This returns the value p from the function ReadTextFile */
}
p = q ;
}
p[pos++] = (a) ;
}

while ( (a = fgetc(fp)) != EOF ) {
place_in_string(a) ;
}
if ( ferror(fp) ) *Error = 2 ;
else *Error = 0 ;
place_in_string(0) ;
return p ;
}

So not just the ability to define functions within functions
but also a statement like return_from which allows you to
return from an enclosing function. This sort of thing,
available in Common Lisp, I would find way more useful than
a special string type which can be done very easily in
standard C.


By the way, apologies if I'm duplicating anything which has
been written in the thread "(part 9) More Schildt-like
errors in Dicky Heathen's book". As a principle I don't open
threads started by trolls.

.