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

[Bruce Evans]

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.

> 	char *TMP;
> 	char *map;
> 	collate_info_t *info;
> @@ -120,11 +120,13 @@ __collate_load_tables_l(const char *enco
> 		return (_LDP_CACHE);
> 	}
>
> -	(void) snprintf(buf, sizeof (buf), "%s/%s/LC_COLLATE",
> -	    _PathLocale, encoding);
> +	asnprintf(&buf, "%s/%s/LC_COLLATE", _PathLocale, encoding);
> +	if (buf == NULL)
> +		return (_LDP_ERROR);

There was no error checking for snprintf().  In fact, its value was
voided.  But error checking for snprintf() can be added just as easily
as error checking for asprintf().

I think this code doesn't compile, because asnprintf() makes no sense
and doesn't exist.
>
> 	if ((fd = _open(buf, O_RDONLY)) < 0)

Memory leak here.

> 		return (_LDP_ERROR);
> +	free(buf);

The memory leak can be fixed better than in the committed version using:

 	fd = _open(...);
 	free(buf);
 	if (fd < 0)
 		return (_LDP_ERROR);

> 	if (_fstat(fd, &sbuf) < 0) {
> 		(void) _close(fd);
> 		return (_LDP_ERROR);

Bruce