Re: [PATCH] tmpdir: Add function to create a unique temp directory.

[John Darrington]

On Thu, Dec 17, 2020 at 12:26:20AM +0100, Bruno Haible wrote:
     
     So, it differs from 'create_temp_dir' only in the aspect that it does NOT
     do automatic cleanup.
     
     Since you are already aware of the 'clean-temp' module — you contributed to
     it 8 years ago :) — what could we write in the documentation, what is the
     advantage of your proposed function?

I'd forgotten about that.   I guess there is little advantage except in the
implementation.  See below

     * Given that 'tmpdir', so far, is an auxiliary module (used by other 
modules),
       I don't like to add higher-level function to it. I would prefer if the
       higher-level function were in a separate module.
       We can rename the existing module 'tmpdir' to, say, 'find-tmpdir', if 
that
       helps.


I think that would make sense.  But so far as I can tell, path_search is used
only by the module clean-temp so why bother with it at all?  Why not move the
function into the clean-temp module and obsolete the tmpdir module?

     
     * The comment "TRY_TMPDIR is ignored if BASEDIR is non-null." suggests that
       it would be better to have two different functions, instead of trying to
       force it into one.
     
     * Much of the code looks like a copy of the 'path_search' function. Why not
       use 'path_search'? What is missing?
     
This was the original motivation for the change.  The path_search function in
its existing form is a pain to use:

One must pass it a buffer containing enough bytes to contain the result.  But
until you've called it, one doesn't know how many bytes are "enough".  One
solution is to dynamically allocate a buffer of 2  bytes, and call path_search
in a loop, increasing the buffer size each iteration, until it succeeds.  But
that of course is hardly efficient.

Another solution is to declare a buffer of length PATH_MAX but PATH_MAX does not
exist on the Hurd.   I see that what the clean-temp module does is to define
PATH_MAX as 1024 if it is not already defined.   This can be problematic for
two reasons:

1.  How do you know that 1024 is enough.

2.  What happens if the operating system *does* define PATH_MAX, but defines it
    to something very large: eg:  0xFFFFFFFF  ?
    In this case, clean-temp will fail when it gets to the line

    xtemplate = (char *) xmalloca (PATH_MAX);


So I think what really needs to happen is, that path_search needs to be
rewritten (and renamed) so that it dynamically allocates its output, and
clean-temp needs to be updated to use the rewritten version.

J'