Re: [lmi] [lmi-commits] master 7fe7f97 2/4: Add and test a function to remove an alien msw root-name from a path

[Greg Chicares]

On 11/11/20 8:42 PM, Vadim Zeitlin wrote:
> On Wed, 11 Nov 2020 12:05:46 -0500 (EST) Greg Chicares 
> <gchicares@sbcglobal.net> wrote:
[...]
> GC> +fs::path remove_alien_msw_root(fs::path const& original_filepath)
> GC> +{
> GC> +#if defined LMI_POSIX
> GC> +    std::string s {original_filepath.string()};
> GC> +    std::string::size_type p = s.find_last_of(':');
> GC> +    if(std::string::npos != p)
> GC> +        {
> GC> +        s.erase(0, 1 + p);
> GC> +        }
> GC> +    return s;
[...]
>  To follow up my previous message, I'm afraid I must disagree with the fix
> as well: it would be better to test for exactly a singular letter followed
> by ":", as this would avoid false positives for POSIX paths that just
> happen to contain a colon somewhere in the middle without any danger of
> having false negatives, as a drive letter always is at most a single
> letter. But in addition to this, I think it's still possible to have false
> negatives too here if we can end up with UNC file names, i.e. those of the
> form "\\server\path". And it doesn't seem impossible for this to happen if
> the files are on a network drive (mounted via SMB) not mapped to any drive
> letter.

I did that in the mistaken belief that ':' is invalid in a posix filename
(whereas actually everything except NUL is accepted). Under that assumption,
though, this seemed like a real filesystem-library defect that needed to be
worked around, and the fix seemed valid enough.

>  Of course, all this is rather theoretical for now, as it only changes the
> behaviour under non-MSW platforms not currently used in production.

It's of great practical importance to me, because it made nychthemeral
tests (which cover multiple architectures) fail.

> But I
> still think that the best way to deal with this problem would be not to
> have it in the first place, by avoiding the reuse of the same configuration
> file in the different builds.

The problem has multiple root causes:

(1) For msw, the print directory might be C:\xyz one day, but M:\xyz another,
and that's a distinction that ought to be preserved. Therefore, the msw
print directory, and thus the whole configuration file, is system-dependent
(as you point out above).

(2) The configurable_settings ctor starts by calling load() [good], then
tests validity [good]; if it finds a problem, it tries to fix it [dubious],
and then calls save() [wrong].

I'm not going to redo these changes soon, as my practical problem is now
solved and I need to spend time on other things this year. I do I wish I had
attacked (1) instead, because that seems about as simple as what I did today,
while solving (2) seems much more difficult.