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

[David Chisnall]

On 9 Aug 2015, at 17:39, Baptiste Daroussin <bapt at FreeBSD.org> wrote:
> 
> Well on the review David recommended to use asprintf rather than snprintf.
> 
> While I dislike the option (as I stated in the review) given he wrote most of
> the recent changes (xlocale) I followed his recommendations.

Having a large (1028 byte currently) buffer on the stack won’t push you over the guard page if you run out of stack, but will mean that if you’re near the end of the stack you’ll get a segfault.  And if there are bugs in the length calculation then we just get a bigger allocation than expected, we don’t get a stack buffer overflow.

In both places, the buffer is only used in one place and so it should only need freeing in one place, though it will require splitting the next lines so that the open call is followed by the free and then the check of the error return.

We have this pattern in enough places in libc that it’s probably worth pulling it out into a function something like this:

int
open_format(const char * restrict format, int oflag, ...)
{
	char *path;
	int file;
	va_list ap;

	va_start(ap, format);
	vasprintf(&path, format, ap);
	va_end(ap);
	if (path == NULL)
		return (NULL);
	file = open(path, oflag);
	free(path);
	return (file);
}

I think every single one of the locale lookup data loading functions would benefit from using this instead of the existing code path.  There are a few other places in libc that seem to do this as well.

As I said in the code review, when we’re doing a printf and some file I/O, the cost of a short-lived heap allocation is likely to be in the noise.

David