Re: [PATCH] fix crash in path.c on Windows
- From
René Scharfe <rene.scharfe@lsrfire.ath.cx>
- Date
- Feb 5, 2009, 16:48 UTC
- Message-ID
- <498B1862.5070703@lsrfire.ath.cx>
- In-Reply-To
- <498A9C01.6020602@viscovery.net>
Johannes Sixt schrieb:
Show 22 quoted lines
> René Scharfe schrieb:
>> set a= getenv("a")
>> ======= ===========
>> c c
>> /c c:/
>> c/ c/
>> /c/ c:/
>> c:c c:c
>> /c:c c:c
>> c:/c c:/c
>> /c:/c c:/c
>> c/:/c c\;c:\
>> /c:c/ c:c/
>> /c/:c /c/:c
>> /c/:/c c:\;c:\
>> /c:/c/ c:/c/
>> /c/:/c/ c:\;c:\
>
> Bash translates leading single-letter path components to drive prefix
> notation if it invokes a non-MSYS program; and it also translates ':' to
> ';' if the value looks like a path list. Sometimes there is an ambiguity
> and bash guesses wrong.Sure, but what rules or heuristics does it follow? Do we need to post-process the results or can we simply change the test case in t1504?
Show 12 quoted lines
>> @@ -387,7 +387,7 @@ int normalize_absolute_path(char *buf, const char *path)
>> assert(path);
>>
>> while (*comp_start) {
>> - assert(*comp_start == '/');
>> + assert(is_absolute_path(comp_start));
>> while (*++comp_end && *comp_end != '/')
>> ; /* nothing */
>> comp_len = comp_end - comp_start;
>
> Junio has pointed out your thinko here. Furthermore, *all* uses of '/' in
> this loop must be replaced by is_dir_sep().It's not a thinko because I didn't think there. :) It's more a case of mechanical refactoring resulting in confusing code. If we inline is_absolute_path() manually there, we get the following:
assert(*comp_start == '/' || has_dos_drive_prefix(comp_start));
The loop works because DOS drive prefixes never contain slashes.
Is is_absolute_path() too forgiving on Windows, i.e. should it stop classifying paths starting with a slash as absolute on that platform?
> But I would really appreciate if you could unify normalize_absolute_path() > with setup.c's sanitary_path_copy() because they do almost the same thing > (and the latter already works on Windows).
Good idea.
Show 20 quoted lines
>> @@ -438,11 +438,20 @@ int longest_ancestor_length(const char *path, const char *prefix_list)
>> return -1;
>>
>> for (colon = ceil = prefix_list; *colon; ceil = colon+1) {
>> - for (colon = ceil; *colon && *colon != ':'; colon++);
>> + for (colon = ceil; *colon && *colon != PATH_SEP; colon++);
>> len = colon - ceil;
>> if (len == 0 || len > PATH_MAX || !is_absolute_path(ceil))
>> continue;
>> strlcpy(buf, ceil, len+1);
>> +
>> + if (has_dos_drive_prefix(buf)) {
>> + char *p;
>> + for (p = buf; *p; p++) {
>> + if (*p == '\\')
>> + *p = '/';
>> + }
>
> IMNSHO this is a kind of normalization and, therefore, must be done by
> normalize_absolute_path() (sanitary_path_copy() already does this).Makes sense.
René