[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Incorrect parsing of DOS/Windows paths ??

From: Eli Zaretskii
Subject: Re: Incorrect parsing of DOS/Windows paths ??
Date: Sun, 18 Dec 2016 19:33:34 +0200

> From: Paul Smith <address@hidden>
> Date: Sun, 18 Dec 2016 11:44:52 -0500
>   #ifdef HAVE_DOS_PATHS
>     /* For DOS paths, skip a "C:\..." or a "C:/..." until we find the
>        first colon which isn't followed by a slash or a backslash.
>        Note that tokens separated by spaces should be treated as separate
>        tokens since make doesn't allow path names with spaces */
>     if (stopmap & MAP_COLON)
>       while (p != 0 && !ISSPACE (*p) &&
>              (p[1] == '\\' || p[1] == '/') && isalpha ((unsigned char)p[-1]))
>         p = find_char_unquote (p + 1, stopmap|MAP_VMSCOMMA|MAP_BLANK);
>   #endif
> As the bug points out the if is clearly broken; it will always be true.

It just means we've been burning cycles when we could have avoided

> However the content of the if-statement looks weird to me as well; I've
> checked and it's been like this almost forever though.  We're trying to
> find the end of the current path.  Why do we keep iterating as long as
> there's a colon followed by a slash or backslash?

Because there's no formal definition of what such a line could
contain, AFAIR.  All that is known for sure is that on Posix hosts we
stop at the first colon or blank.  The loop tries to find such a place
that is not a drive letter, without assuming anything about what else
can or cannot be on that line.

> E.g., from what I can see this will accept the following as a valid,
> single pathname:
>   foo:/bar:\biz
> ???

Yes, it will.  And then Make will properly display an error message
where the results are used as file names.  Is that wrong?  Won't the
same happen on Unix, if a file /bar:\biz doesn't exist?

The code we are talking about doesn't know it is parsing file names.
Its only purpose is to find the first colon that separates one string
from another, like it does on Unix.

> Why wouldn't the correct algorithm be: if we stopped due to a drive
> specifier (the pathname starts with "[A-Za-z]:")

A nit: DOS drive letters can be A-z, including the few characters
between Z and a.

>   #ifdef HAVE_DOS_PATHS
>     /* If we stopped due to a drive specifier, skip it.
>        Tokens separated by spaces are treated as separate paths since make
>        doesn't allow path names with spaces */
>     if (p && p == s+1 && p[0] == ':' && isalpha ((unsigned char)s[0]))
>         p = find_char_unquote (p+1, stopmap|MAP_VMSCOMMA|MAP_BLANK);
>   #endif

If you say so.  IMO, there are too many assumptions here that only a
GNU Make maintainer can make ;-)  E.g., who said that the drive spec
must begin at s?  Can't there be whitespace there?

> Note that this doesn't require the drive specifier to be followed by a
> slash/backslash: e.g., this:
>   C:foo.o:C:foo.c
> Breaks down as:
>   C:foo.o
>   C:foo.c

Which I'd rather not support, because it could also be an error, with
the real target being just C.  IOW, it's ambiguous, and Make had
better not second-guessed the luser.

Bottom line: can you tell what can and cannot be on lines that are
parsed by that function?  Does it have to be one or two valid file
names?  Otherwise I feel like we are talking about an issue at least
one of us -- myself -- doesn't fully understand.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]