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'