Re: [PATCH] tmpdir.c (path_search_alloc): New function.

[Bruno Haible]

Hi John,

As this is your first major contribution to Gnulib, can you please make sure
you have signed a copyright assignment with the FSF regarding 'GNULIB'?

> * lib/tmpdir.c (path_search_alloc): Define new function similar to
> path_search,  but which allocates the buffer for the result instead
> of relying on the caller to preallocate it.

This is a good idea, because it gets rid of an EINVAL error return.

> +/* Like path_search, except that TMPL is allocated automatically.
> +   TMPL may not be null.  *TMPL must be freed by the caller, when no longer 
> needed.
> +   After calling this function *TMPL_LEN will be set to the lenght of *TMPL. 
>  */
> +extern int path_search_alloc (char **tmpl, size_t *tmpl_len, const char 
> *dir, const char *pfx,
> +                       bool try_tmpdir);

The calling convention is odd: If the caller is only meant to use *TMPL and
later free() it, why does he need *TMPL_LEN? It seems redundant to return it
from this function. And then, if *TMPL is the only output (besides the error
condition), why not make it the return value? That is:

  extern char * path_search_alloc (const char *dir, const char *pfx, bool 
try_tmpdir);

In case of error, this function would return NULL with errno set.

Indentation: Please make the second arglist line align better.

Typo: s/lenght/length/

> +static int
> +__path_search (char *tmpl, const char **dirx, const char *pfx, bool 
> try_tmpdir)
> ...
> +static void
> +__impute_lengths (const char *pfx, const char *dir, size_t *plen, size_t 
> *dlen, bool *add_slash)

Please don't choose function names that start with underscore. These function
names are in the scope of the vendor libc. And in fact, such function names
would make merging with glibc harder, since glibc already has a function
'__path_search' (defined in sysdeps/posix/tempname.c).

Also, __path_search is a misnomer now: it does not search anything; it
determines the temporary directory in which to place a temporary file.

Bruno