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

[Bruno Haible]

Hi John,

First of all, thanks for having signed a copyright assignment for Gnulib!

Then, let me see what Gnulib modules we already have, to see how your proposed
patch fits in.

1) We have modules that provide auxiliary functions.
    step 1 : tmpdir - find an appropriate parent dir
    step 2 : tempname - use random bits to fill in the XXXXXX

2) Modules that create a file:

  Create a file, return an fd, includes step 2, but step 1 left to the caller:
    mkstemp (char *template)
    mkstemps (char *template, int suffixlen)
    mkostemp (char *template, int flags)
    mkostemps (char *template, int suffixlen, int suffixlen)

  Create a file, return a FILE *, includes step 1 and step 2:
    tmpfile (void)
    tmpfile_safer (void)

3) Modules that create a directory:

  Create a directory, includes step 2, but step 1 left to the caller:
    mkdtemp (char *template)

  Create a directory, includes step 1 and step 2, with automatic cleanup:
    clean-temp : create_temp_dir (prefix, parentdir, cleanup_verbose)

Your proposed function falls into the category 3:
  Create a directory, includes step 1 and step 2.

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?

Further remarks:

* You have better comments regarding PFX and TRY_TMPDIR that those found
  for 'path_search' in lib/tmpdir.h and lib/tmpdir.c. It would make sense
  to improve these comments first.

* 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.

* 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?

Bruno