[patch] fix invalid free() in HEAD's readlink.c; take two

[Rob Landley]

On Monday 12 September 2005 14:21, Bernhard Fischer wrote:
> Rob Landley wrote:
> > On Sunday 11 September 2005 04:42, Bernhard Fischer wrote:
> > > Attached patch should be correct.
> > >
> > > - free the buffer allocated by xreadlink(), i.e. when
> > >   CONFIG_FEATURE_READLINK_FOLLOW isn't set or no -f was given.
> >
> > Except that xreadlink is returning malloced (well, realloced) memory
> > that
> > you're now not freeing...
>
> hm? i said:
> -       if (ENABLE_FEATURE_CLEAN_UP) free(buf);
> +       if (ENABLE_FEATURE_CLEAN_UP) {
> +               if (!(opt & READLINK_FLAG_f))
> +                       free(buf);
> +       }
>
> which did only free the memory allocated by xreadlink(), so that was
> correct, i think. But anyway.

Sorry, replied to "take two" but the file I had open was take one...

> > I took a whack at it, taking advantage of glibc's ability to malloc
> > memory in
> > realpath and cleaning up the #ifdefs while I was there.  Tell me what
> > you
> > think...
>
> In this particular case of realpath i'd prefer not to rely on the
> underlying libc but to use RESERVE_CONFIG_BUFFER there.

Hmmm...  Makes it bigger, and I think this feature is supported by both glibc 
and uClibc.

And it just sets my teeth on edge to have two different memory allocation 
strategies going into the same pointer.  (I've had to clean up some real 
horrors.  Feature Install had _five_ different allocation functions.  
(malloc, SOMAlloc, _wpAlloc, and two more I can't even remember, and yes 
objects allocated in different ways wound up intermixed in the same linked 
list, all the time...  And on top of that I found an instance or two of 
functions returning pointers to the stack which had coincidentally worked 
until I made unrelated changes to the calling code.)  Obviously we haven't 
got anything that tangled here, but...

Can we get a third opinion?

Rob