Quick and dirty malloc() support for realpath.

[Rob Landley]

On Wednesday 28 October 2009 02:47:03 Mike Frysinger wrote:
> On Tuesday 27 October 2009 15:44:42 Rob Landley wrote:
> > On Tuesday 27 October 2009 05:51:05 Bernhard Reutner-Fischer wrote:
> > > On Tue, Oct 27, 2009 at 08:54:50AM +0100, Ricard Wanderlof wrote:
> > > >On Mon, 26 Oct 2009, Rob Landley wrote:
> > > >>... Also, in my experience _Bool is about as real-world useful as
> > > >>the bit field notation with the colons, and is really there to keep
> > > >>the language pedants and the c++ guys happy without actually
> > > >>accomplishing much.  I've never seen it actually produce better
> > > >>code.
> > > >
> > > >It can produce more readable, less error-prone C code though. We use
> > >
> > > $ size libc/stdlib/realpath.o*
> > >    text	   data	    bss	    dec	    hex	filename
> > >     555	      0	      0	    555	    22b	libc/stdlib/realpath.os.oorig
> > >     735	      0	      0	    735	    2df	libc/stdlib/realpath.os.rob
> > >     586	      0	      0	    586	    24a	libc/stdlib/realpath.os
> > >
> > > perhaps that would do as well and doesn't cost 180b (!) but about 31b..
> >
> > The problem is that leaks memory if realpath ever returns failure, such
> > as the lines right after that patch:
> >
> >         path_len = strlen(path);
> >         if (path_len >= PATH_MAX - 2) {
> >                 __set_errno(ENAMETOOLONG);
> >                 return NULL;
> >         }
> >
> > And at least a half-dozen other "return NULL;" error cases later in the
> > function.
> >
> > Possibly there's some way to not inline the alloca?  This was, as I said,
> > a quick and dirty fix.  Someday we should properly make it handle more
> > than PATH_MAX, but that's a big change and I'm just trying to get
> > software using Linux libc extensions (and now SUSv4) to work.
>
> while the memory leakage needs to be addressed, the answer isnt with
> alloca. the spec states that it must be via malloc(), but even ignoring
> that, it also states that the caller must call free() on the returned
> pointer.

Please read the patch I submitted.

@@ -168,5 +172,6 @@
                new_path--;
        /* Make sure it's null terminated. */
        *new_path = '\0';
+       if (allocated) got_path = strdup(got_path);
        return got_path;
 }

> obviously alloca-ed memory is not valid outside of the return of
> realpath() (which makes me wonder how your code even worked in the first
> place), nor can you call free() on it.

You believed I wasn't aware of this?

> -mike

Rob
-- 
Latency is more important than throughput. It's that simple. - Linus Torvalds