[lmi] Uniquification of '....' [Was: PATCH: Switch to using std::filesystem]

[Greg Chicares]

On 4/28/21 12:17 PM, Greg Chicares wrote:
> On 4/26/21 9:53 PM, Vadim Zeitlin wrote:
>> 
>>  The branch "std-fs" of https://github.com/vadz/lmi.git repository
> I get the following unit-test error for both msw builds:
> 
> Running path_utility_test:
> 
> ???? test failed:   '...-20210428T115328Z.' == '....'
> [file /opt/lmi/src/lmi/path_utility_test.cpp, line 302]

Now that I think about this, I wonder if that failure indicates
a need to rework unique_filepath(). The motivation is that if
a user has 'foo.xls' open in 'excel', then that file is locked
(under native msw) and cannot be altered by any other process;
therefore, when lmi wants to overwrite it, it cannot, and must
therefore write something like 'foo-20210428T115328Z.xls'.
This is a real problem, and this solution has worked well for
a long time.

To determine whether an existing file can be replaced,
unique_filepath() does this:

        fs::remove(filepath);
        LMI_ASSERT(!fs::exists(filepath));

and if that throws, then the timestamp is appended to the
name of the new file.

My first thought is that deleting '....' seems potentially
catastrophic on an OS where that's shorthand for something like
  .\\..\\..
Maybe we can be sure that, today at least, this function is
called only with a 'filepath' generated by lmi, so that can't
ever happen; but I feel less sure of tomorrow. Maybe the
triple-dot shorthand is only an old, old thing:
  https://devblogs.microsoft.com/oldnewthing/20160202-00/?p=92953
that ms hasn't supported in this century; and maybe the 'wine'
maintainers would be absolutely sure not to do it even as a
matter of backward compatibility; sure, maybe. But, having
considered this special case in a unit test, shouldn't we
guard against it in unique_filepath()? E.g.:

  if(!has_path(original_filepath))
    throw "absurdity encountered!";

What would happen if we, unintentionally of course, executed
  fs::remove("/");
? Is that like starting to type
  rm -rf /
intending to finish entering an absolute path, and then
having someone lean his large canine chin on the Enter key?
(Luckily, he doesn't know 'sudo'; but we're discussing msw here.)
I think the absurdity trap above would catch that.

Here's a second thought. Reconsidering the two lines quoted
above, before "and if that throws", do we really need a
try...catch here in this code now? I forget what the boost
implementation did, but now we have this overload [fs.op.remove]

  bool remove(const path& p, error_code& ec) noexcept;
  Postconditions: exists(symlink_status(p)) is false.

that doesn't throw, and in the unlikely case that I truly
understand symlink_status(), the postcondition would seem
to make this assertion
        LMI_ASSERT(!fs::exists(filepath));
pointless. If so, could we replace "try...catch" with "if"?

And, even if we write code that a conforming implementation
must execute as intended, do we yet have faith that any
std::filesystem implementation is correct in all details?
E.g., might a library author just check static permissions
to see whether the file is erasable (ignoring an actual
filesystem's locking), instead of verifying that the
erased file truly does not exist? And how would that work
on msw anyway, where users are almost certainly running
"anti-malware" software that may keep a handle to a file
open longer than our software or the OS anticipates?
Still, we could do this:

-    try
-        {
-        fs::remove(filepath);
-        LMI_ASSERT(!fs::exists(filepath));
+    if(fs::remove(filepath), !fs::exists(filepath))

or s/,/&&/ if the comma operator seems passé.

It would probably be a good exercise for me to try to figure
this out all by myself, but that could take whole days, so
it's probably wiser to ask.