"C Unleashed is not a book for beginners. Therefore, this little
section should not be necessary. Regrettably, there are C programmers
out there in the big wide world who claim to have many years of
experience using the language, are earning considerable amounts of
money (at consultancy rates) in return for offering their expertise,
and yet cannot see anything wrong with this code ... I'm sad to
say I know some of these people personally. I won't name and shame
them here, but this section is for them and people like them ...
To those of you who are still reading--please listen, because this
is really important ... It's extremely important to pay careful
attention to how your code and the standard library use memory ...
Here's another example of memory abuse, this time straight from
the annals of Usenet (lightly edited to disguise the handiwork
of the guilty) ... Here's the answer some bright spark came
up with ... I've seen source not to dissimilar to this in
production code." 

[Richard Heathfield, in chapter 8 of his book, before winning his 
"Most Condescending C Author of the Year" award.]


Error #1 (minor, but major for Schildt) - pg. 263
=================================================

"If there isn't a newline early enough, however, gets() will
start to trample over memory that it shouldn't be touching,
with undefined results."

Undefined behavior means that the program may not start
trampling over memory at all. 

Error #2 (very minor, but somewhat major for Schildt) - pg. 265
===============================================================

"And it's illegal to write to a NULL pointer."

Could be clarified/disambiguated for the reader.

int n = 42;
int *p = NULL;  
p = &n;         /* write to a NULL pointer */   

OK, I'm kidding. But don't laugh -- go read the so-called
Schildt "errata".

Error #3 (major, but already covered in part 9)
===============================================

Listing 8.1 has the same overflow error as listing 8.2. Already
covered in part 9 of my series.

Error #4 (minor/dumb, but probably not so for Schildt) - pg. 268
================================================================

char *BuildPhoneNumber(int code, int num)
{
  char telno[16];
  sprintf(telno, "(%05d) %d", code, num);
}

Conceivable overflow, depending on the implementation and on
how 'code' and 'num' are controlled.

Error #5 (major Heathen Klutz, not even Schildt-like)
============================================================

I have saved the best for last (actually, as you may have
noticed, I've restarted chapter 8 in sequence). 

Listing 8.3  
-----------

static char *buffer = NULL;
static size_t bufsize = 0;
while(strlen(s) >= bufsize) 
{
    p = realloc(buffer, bufsize * 2);
    if(p != NULL)
    {
        bufsize *= 2;
        buffer = p;
    }
    else
    {
        printf("er...now what?\n");
    }
}

The above is highly flawed. To be charitable to Heathfield
(a charitableness not extended to Schildt), we won't tackle
the 'else' block, where we'll assume Heathfield does something
smart. We will instead examine the case where the 'else' block 
possibly never gets entered.

Now, note that the initial call to realloc() is equivalent to
realloc(NULL, 0) (why is he doing that to begin with?). This 
means that realloc() will behave like malloc(0). The standard 
says malloc() may return NULL in that case or a non-null pointer 
that may not be used. If we get a non-null pointer that can't 
be used, we end up in the 'if' block. In the 'if' block, note 
that "bufsize *= 2" never causes bufsize to be greater than 0. 
So we loop around again, and assuming realloc() behavior remains
consistent, we have an infinite loop. If realloc() at some
point decides to return a NULL pointer (it needn't do so,
since one scenario is that no memory may be allocated even with
the non-null returns we're getting), and if Heathfield does
something in the 'else' block that gets him out of the
loop, he'll be left with a non-null 'buffer' pointer that 
can't be used.

On many modern Unix and Unix-like systems, the above is in
fact what happens. We get an infinite loop.


Listing 8.4
-----------

static char *buffer = NULL;
static size_t bufsize = 0;
size_t len = strlen(s) + 1;

while(len > bufsize) 
{
    p = realloc(buffer, bufsize * 2);
    if(p != NULL)
    {
        bufsize *= 2;
        buffer = p;
    }
    else
    {
        p = realloc(buffer, len);
        if(p != NULL)
        {
            bufsize = len;
            buffer = p;
        }
        else
        {
            printf("What we need here is a design decision!\n");
        }
    }
}

"This code is better."

Actually, it's not. It contains the same infinite-loop possibility 
with the first 'if' block as in Listing 8.3. 

I'll end off at Listing 8.4 for today. Those who are keeping track
can see that so far we have found a gross error in Listings 8.1,
8.2, 8.3, and 8.4. I promise that the worst is yet to come.

Keep insulting me, Heathfield. It drives me forward and only
makes you look more and more foolish with your petty approach
to updating your errata.

Yours,
Han from China

--
"Only entropy comes easy." -- Anton Chekhov

Han from China - Master Troll wrote:
> Error #4 (minor/dumb, but probably not so for Schildt) - pg. 268
> ================================================================
> 
> char *BuildPhoneNumber(int code, int num)
> {
>   char telno[16];
>   sprintf(telno, "(%05d) %d", code, num);
> }
> 
> Conceivable overflow, depending on the implementation and on
> how 'code' and 'num' are controlled.

In a 32 bit system:
#include <stdio.h>
#include <string.h>
int main(void)
{
         char buf[256];

         sprintf(buf,"(%05d) %d",-1234567890,-1234567890);
         printf("strlen(\"%s\"=%d\n",buf,strlen(buf));
}

Output:
strlen("(-1234567890) -1234567890"=25

Actually, the number of digits that a %d specification
can need can be precisely calculated with this simple formula:

Number of digits N = 1 + ceil(log10((double)INT_MAX));

For a 32 bit system this is 11. The addition of 1 is needed
for the possible sign ('-').

It is just a pity that C99 doesn't define a standard constant
for this quantity.

-- 
jacob navia
jacob at jacob point remcomp point fr
logiciels/informatique
http://www.cs.virginia.edu/~lcc-win32

On Jan 12, 11:10=A0pm, Han from China - Master Troll <autistic-
pedan...@comp.lang.c> wrote:

<snip trolling crap>

> static char *buffer =3D NULL;
> static size_t bufsize =3D 0;
> while(strlen(s) >=3D bufsize)
> {
> =A0 =A0 p =3D realloc(buffer, bufsize * 2);
> =A0 =A0 if(p !=3D NULL)
> =A0 =A0 {
> =A0 =A0 =A0 =A0 bufsize *=3D 2;
> =A0 =A0 =A0 =A0 buffer =3D p;
> =A0 =A0 }
> =A0 =A0 else
> =A0 =A0 {
> =A0 =A0 =A0 =A0 printf("er...now what?\n");
> =A0 =A0 }
>
> }
>
> The above is highly flawed. To be charitable to Heathfield
> (a charitableness not extended to Schildt), we won't tackle
> the 'else' block, where we'll assume Heathfield does something
> smart. We will instead examine the case where the 'else' block
> possibly never gets entered.
>
> Now, note that the initial call to realloc() is equivalent to
> realloc(NULL, 0) (why is he doing that to begin with?). This
> means that realloc() will behave like malloc(0).

You're wrong. It's implementation-defined whether it'll behave like
malloc(0) or free(NULL)
More specifically,

realloc(p, 0); may mean free(p) or malloc(0);

On Jan 13, 12:22=A0am, jacob navia <ja...@nospam.org> wrote:
<snip>
> Actually, the number of digits that a %d specification
> can need can be precisely calculated with this simple formula:
>
> Number of digits N =3D 1 + ceil(log10((double)INT_MAX));
>
> For a 32 bit system this is 11. The addition of 1 is needed
> for the possible sign ('-').
>
> It is just a pity that C99 doesn't define a standard constant
> for this quantity.

Actually this is a nice idea. Any reason why it was rejected? If it
was proposed/discussed that is.

vippstar wrote:
> Han from China - Master Troll wrote:
> 
> <snip trolling crap>
> 
> > static char *buffer = NULL;
> > static size_t bufsize = 0;
> > while(strlen(s) >= bufsize)
> > {
> >     p = realloc(buffer, bufsize * 2);
[...]
> > Now, note that the initial call to realloc() is equivalent to
> > realloc(NULL, 0) (why is he doing that to begin with?). This
> > means that realloc() will behave like malloc(0).
> 
> You're wrong. It's implementation-defined whether it'll behave like
> malloc(0) or free(NULL)

How could it mean simply free(NULL)? realloc returns a value, and
free() does not. Regardless, the C99 standard disagrees:

7.20.3.4p3:
| If ptr is a null pointer, the realloc function behaves like
| the malloc function for the specified size.

> More specifically,
> 
> realloc(p, 0); may mean free(p) or malloc(0);

If you're suggesting that

    void* p = malloc( 1 );
    p = realloc( p, 0 );
    free( p );

may leak memory depending on the implementation, please refer me to
the relevant section of the standard.

On Jan 13, 5:18=A0pm, blargg....@gishpuppy.com (blargg) wrote:
> vippstar wrote:
> > Han from China - Master Troll wrote:
>
> > <snip trolling crap>
>
> > > static char *buffer =3D NULL;
> > > static size_t bufsize =3D 0;
> > > while(strlen(s) >=3D bufsize)
> > > {
> > > =A0 =A0 p =3D realloc(buffer, bufsize * 2);
> [...]
> > > Now, note that the initial call to realloc() is equivalent to
> > > realloc(NULL, 0) (why is he doing that to begin with?). This
> > > means that realloc() will behave like malloc(0).
>
> > You're wrong. It's implementation-defined whether it'll behave like
> > malloc(0) or free(NULL)
>
> How could it mean simply free(NULL)? realloc returns a value, and
> free() does not. Regardless, the C99 standard disagrees:
>
> 7.20.3.4p3:
> | If ptr is a null pointer, the realloc function behaves like
> | the malloc function for the specified size.
>
> > More specifically,
>
> > realloc(p, 0); may mean free(p) or malloc(0);
>
> If you're suggesting that
>
> =A0 =A0 void* p =3D malloc( 1 );
> =A0 =A0 p =3D realloc( p, 0 );
> =A0 =A0 free( p );
>
> may leak memory depending on the implementation, please refer me to
> the relevant section of the standard.

You (and han) are right and I was wrong. It's implementation dependant
whether realloc(p, 0) does free(p) or malloc(0). However, if p is
NULL, it is well-defined, as you suggested.

vippstar wrote:
> blargg wrote:
[...]
> > If you're suggesting that
> >
> >     void* p =3D malloc( 1 );
> >     p =3D realloc( p, 0 );
> >     free( p );
> >
> > may leak memory depending on the implementation, please refer me to
> > the relevant section of the standard.
> 
> You (and han) are right and I was wrong. It's implementation dependant
> whether realloc(p, 0) does free(p) or malloc(0). However, if p is
> NULL, it is well-defined, as you suggested.

I still don't grasp what you mean that realloc(p, 0) does either free(p)
or malloc(0). If p is not null, surely realloc(p, 0) always does the
equivalent of free(p), followed by malloc(0). It sounds like you're
saying otherwise.

On Jan 13, 6:59=A0pm, blargg....@gishpuppy.com (blargg) wrote:
> vippstar wrote:
> > blargg wrote:
> [...]
> > > If you're suggesting that
>
> > > =A0 =A0 void* p =3D3D malloc( 1 );
> > > =A0 =A0 p =3D3D realloc( p, 0 );
> > > =A0 =A0 free( p );
>
> > > may leak memory depending on the implementation, please refer me to
> > > the relevant section of the standard.
>
> > You (and han) are right and I was wrong. It's implementation dependant
> > whether realloc(p, 0) does free(p) or malloc(0). However, if p is
> > NULL, it is well-defined, as you suggested.
>
> I still don't grasp what you mean that realloc(p, 0) does either free(p)
> or malloc(0). If p is not null, surely realloc(p, 0) always does the
> equivalent of free(p), followed by malloc(0). It sounds like you're
> saying otherwise.

malloc(0) may return NULL. (it's implementation defined whether it's
NULL or a pointer). Which means realloc(p, 0) will either return
memory or just free the pointer.

On Tue, 13 Jan 2009 09:18:14 -0600, blargg wrote:
>> More specifically,
>> 
>> realloc(p, 0); may mean free(p) or malloc(0);
> 
> If you're suggesting that
> 
>     void* p = malloc( 1 );
>     p = realloc( p, 0 );
>     free( p );
> 
> may leak memory depending on the implementation, please refer me to the
> relevant section of the standard.

7.20.3.4p4 specifies that realloc returns either a pointer to an object,
or a null pointer if the new object could not be allocated. A null pointer
is not a pointer to an object, so if a null pointer is returned, the
implementation is telling you the new object could not be allocated. And
p3 states that if the new object could not be allocated, the original
block is not deallocated.

(7.20.3p1 says malloc(0) and realloc(p, 0) are permitted to return a null
pointer. This doesn't contradict the description of realloc.)

jacob navia wrote:
> Actually, the number of digits that a %d specification
> can need can be precisely calculated with this simple formula:
.....
> It is just a pity that C99 doesn't define a standard constant
> for this quantity.

Your first comment probably explains why.

-- 
Mark McIntyre

CLC FAQ <http://c-faq.com/>
CLC readme: <http://www.ungerhu.com/jxh/clc.welcome.txt>

Harald van Dijk wrote:
> On Tue, 13 Jan 2009 09:18:14 -0600, blargg wrote:
> >> More specifically,
> >> 
> >> realloc(p, 0); may mean free(p) or malloc(0);
> > 
> > If you're suggesting that
> > 
> >     void* p = malloc( 1 );
> >     p = realloc( p, 0 );
> >     free( p );
> > 
> > may leak memory depending on the implementation, please refer me to the
> > relevant section of the standard.
> 
> 7.20.3.4p4 specifies that realloc returns either a pointer to an object,
> or a null pointer if the new object could not be allocated. A null pointer
> is not a pointer to an object, so if a null pointer is returned, the
> implementation is telling you the new object could not be allocated. And
> p3 states that if the new object could not be allocated, the original
> block is not deallocated.

I was hoping to avoid bogging it down with this, but I see now that the 
"allocation" in malloc(0) really could fail: if malloc(0) doesn't return 
null, then it has to allocate some space, thus it could fail. So the 
above really could leak. See my reply to vippstar for updated code.

> (7.20.3p1 says malloc(0) and realloc(p, 0) are permitted to return a null
> pointer. This doesn't contradict the description of realloc.)

blargg wrote:
> vippstar wrote:
>
.... snip ...
> 
> How could it mean simply free(NULL)? realloc returns a value, and
> free() does not. Regardless, the C99 standard disagrees:
> 
> 7.20.3.4p3:
> | If ptr is a null pointer, the realloc function behaves like
> | the malloc function for the specified size.
> 
>> More specifically,
>>
>> realloc(p, 0); may mean free(p) or malloc(0);
> 
> If you're suggesting that
> 
>     void* p = malloc( 1 );
>     p = realloc( p, 0 );
>     free( p );
> 
> may leak memory depending on the implementation, please refer me
> to the relevant section of the standard.

No, vippstar is misunderstanding the standard.  That quote means
that ONLY when p has the value NULL will realloc(p, 0) behave like
malloc(0).  Whether that returns a NULL or a pointer value is
implementation defined, which is a pain.

If malloc returns a non-NULL, and realloc returns a NULL, the
realloc may have failed, and the pointer to the memory allocated by
malloc is lost.  If the realloc didn't fail, no memory has been
leaked.

This is why I detest the standards permission of returning NULL for
malloc(0) and/or realloc(p, 0).  It means you cannot simply test
the returned value to detect failure.  Since it is permissable, my
malloc package (nmalloc.zip) always acts as if the size was at
least 1.  Users will not run into this problem.  However, getting
used to using it tends to make you forget to protect against it in
portable code.  Bah.

-- 
 [mail]: Chuck F (cbfalconer at maineline dot net) 
 [page]: <http://cbfalconer.home.att.net>
            Try the download section.

Han from China - Master Troll <autistic-pedantry@comp.lang.c> wrote:

> Error #1 (minor, but major for Schildt) - pg. 263
> =================================================
> 
> "If there isn't a newline early enough, however, gets() will
> start to trample over memory that it shouldn't be touching,
> with undefined results."
> 
> Undefined behavior means that the program may not start
> trampling over memory at all. 

That is a point of semantics - it might be trampling, but softly enough
to leave no footprints.

Richard

Han from China - Master Troll wrote:
> If realloc() at some point decides to return a NULL pointer (it 
> needn't do so, since one scenario is that no memory may be allocated 
> even with the non-null returns we're getting),


    realloc(NULL, 0)

behaves like

    malloc(0)

which, if it chooses to return a non-null pointer, must behave as
if it had been called as malloc(N), with N > 0 (7.20.3{1}). 

So in our (practically) infinite loop of calls to

    realloc(NULL, 0)

we could conceivably starve time and memory through a (practically)
infinite loop of calls to

    malloc(1) 

that leak memory, assuming consistent realloc() behavior.

Doesn't seem to affect the designation of "infinite loop", since even

    while(1);

may not strictly be "infinite" if it pegs the CPU and halts the
system.

And the C99's abstract machine doesn't rule out a computer that
can manufacture new memory / virtual memory components for itself 
and do a whole lot of magic to extend the realloc() loop far into 
the distant future.

On many Unix and Unix-like systems, listings 8.3 and 8.4 loop
indefinitely, if not infinitely. I'm not sure how those listings 
passed code review to be honest, both Heathfield's and the 
publisher's.

Something stinks.

Yours,
Han from China

--
"Only entropy comes easy." -- Anton Chekhov