Re: [lmi] [lmi-commits] master 5567c43 1/3: Begin to tackle an observed unit-test failure

[Greg Chicares]

On 2020-08-17 11:02, Vadim Zeitlin wrote:
> On Mon, 17 Aug 2020 01:50:09 +0000 Greg Chicares <gchicares@sbcglobal.net> 
> wrote:

Thanks for again offering different points of view, because I
awoke this morning with two thoughts:
 - that I had dismissed mkstemp() too hastily; and
 - that I had probably attacked the wrong problem first.

The reported failure arose because user 'nemo', with some default
group like 'nobody' that no one else belongs to, ran the unit test,
creating a directory with a static name that everyone else had to
use (in order to run a particular unit test), but no one else had
permission to use it. Was the problem that...

 (1) the temporary directory's name was static?
     (then mkstemp() would be the solution); or

 (2) user 'nemo' had a default group that no one else belongs to?
     (then its default group should be changed to 'lmi'); or

 (3) the notionally "temporary" directory was physically permanent
     because I failed to remove it? (then filesystem::remove() is
     the solution)

I started by addressing (1), just because it came to mind first.
Then I skipped (2) because (3) occurred to me, and now I think that
failure to remove that directory (i.e., (3)) was the cardinal sin.

By the time I reached that conclusion, I had applied (1) to three
files, then found that two of them wouldn't link, because (1)
introduced a dependency on 'path_utility.o'; feeling that (3) was
the real problem, I discarded those changes. As for (2), I figure
it can wait: it seems like a prima facie defect, but it unmasked
real problems, so maybe it's a "feature".

> GC> Or would my use case call for mkdtemp()?
> 
>  Unfortunately, mkdtemp() doesn't seem to be available in MinGW.

If mkdtemp() is the answer, then (2) is a "feature", because
mkdtemp() creates a directory with 0700 permissions.

> GC> It looked like it would take so much effort to figure all that stuff
> GC> out that I'd be better off using my own (tested) function.
> 
>  This function suffers from the same problem as mktemp(), except it's even
> worse because it doesn't even try to randomize anything at all.

The customary quote about reinventing *nix, poorly, applies here.
Thanks for pointing that out; I thought I was being clever. In a
pending commit I'll be adding this comment:

+    // Although it's okay for this unit test, using unique_filepath()
+    // as a substitute for the nonstandard mkstemp() is a bad idea.
+    // See:
+    //   https://lists.nongnu.org/archive/html/lmi/2020-08/msg00015.html
     fs::path const u = unique_filepath("/tmp/" + fs::basename(__FILE__), "");