svn commit: r286521 - projects/collation/lib/libc/locale

[Jilles Tjoelker]

On Sun, Aug 09, 2015 at 10:54:15PM +1000, Bruce Evans wrote:
> On Sun, 9 Aug 2015, Baptiste Daroussin wrote:

> > Log:
> >  Use asprintf/free instead of snprintf

> Why?  It takes 3 times as much code, and immediately gave you a memory
> leak when you wrote only twice as much.  You fixed the memory leak in the
> next commit, but it might not always be so easy to see.

> > Modified: projects/collation/lib/libc/locale/collate.c
> > ==============================================================================
> > --- projects/collation/lib/libc/locale/collate.c	Sun Aug  9 11:47:01 2015	(r286520)
> > +++ projects/collation/lib/libc/locale/collate.c	Sun Aug  9 11:50:50 2015	(r286521)
> > @@ -107,7 +107,7 @@ int
> > __collate_load_tables_l(const char *encoding, struct xlocale_collate *table)
> > {
> > 	int i, chains, z;
> > -	char buf[PATH_MAX];
> > +	char *buf;

> POSIX_ME_HARDER code would use {PATH_MAX} = sysconf(__PATH_MAX) and error
> handling for sysconf() and then for malloc()ing {PATH_MAX} bytes.  This
> would take 10 times as much code, except it could use a VLA with no
> error checking for the allocation starting with C99.  The asprintf()
> method would be better then.

No, POSIX_ME_HARDER code would use dynamic allocation (but without
asprintf() since that's not in POSIX). This is because {PATH_MAX} may be
indeterminate (pathconf() returns -1 but does not set errno).

Note that some usages of certain functions are invalid when {PATH_MAX}
is indeterminate, for example realpath() with a non-NULL resolved_path
parameter.

It seems uncommon to have a fixed {PATH_MAX} but not define it as a
compile-time constant. An indeterminate {PATH_MAX} is used in GNU Hurd
and its developers occasionally patch software to make it cope with
that.

I think it is fine to use dynamic allocation here.

-- 
Jilles Tjoelker