All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Sixt <j.sixt@viscovery.net>
To: "René Scharfe" <rene.scharfe@lsrfire.ath.cx>
Cc: Git Mailing List <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH] fix crash in path.c on Windows
Date: Thu, 05 Feb 2009 08:57:53 +0100	[thread overview]
Message-ID: <498A9C01.6020602@viscovery.net> (raw)
In-Reply-To: <498A1E1E.8010901@lsrfire.ath.cx>

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.

> @@ -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().

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).

> @@ -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).

> +		}
> +
>  		len = normalize_absolute_path(buf, buf);
>  		/* Strip "trailing slashes" from "/". */
>  		if (len == 1)

-- Hannes

  parent reply	other threads:[~2009-02-05  7:59 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-02-04 23:00 [PATCH] fix crash in path.c on Windows René Scharfe
2009-02-04 23:51 ` Junio C Hamano
2009-02-05  7:57 ` Johannes Sixt [this message]
2009-02-05 16:48   ` René Scharfe
2009-02-05 17:13     ` Johannes Sixt
2009-02-05 20:41     ` Robin Rosenberg
2009-02-05 19:35 ` [PATCH] fix t1504 " René Scharfe
2009-02-06 12:55   ` Johannes Sixt
2009-02-06 13:11     ` Johannes Schindelin
2009-02-06 13:17       ` Johannes Sixt
2009-02-06 13:26         ` Johannes Schindelin
2009-02-06 13:36           ` Johannes Sixt
2009-02-06 17:18     ` René Scharfe
2009-02-06 19:23       ` Johannes Sixt
2009-02-06 21:45         ` René Scharfe
2009-02-07 15:08           ` [PATCH 0/5] Consolidate path normalization functions Johannes Sixt
2009-02-07 15:08             ` [PATCH 1/5] Make test-path-utils more robust against incorrect use Johannes Sixt
2009-02-07 15:08               ` [PATCH 2/5] Move sanitary_path_copy() to path.c and rename it to normalize_path_copy() Johannes Sixt
2009-02-07 15:08                 ` [PATCH 3/5] Fix GIT_CEILING_DIRECTORIES on Windows Johannes Sixt
2009-02-07 15:08                   ` [PATCH 4/5] Test and fix normalize_path_copy() Johannes Sixt
2009-02-07 15:08                     ` [PATCH 5/5] Remove unused normalize_absolute_path() Johannes Sixt
2009-02-08  0:08                     ` [PATCH 4/5] Test and fix normalize_path_copy() Robin Rosenberg
2009-02-08  8:52                       ` Johannes Sixt
2009-02-08 14:46                     ` René Scharfe
2009-02-08 15:50                       ` Johannes Sixt
2009-02-07  0:25     ` [PATCH] fix t1504 on Windows René Scharfe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=498A9C01.6020602@viscovery.net \
    --to=j.sixt@viscovery.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=rene.scharfe@lsrfire.ath.cx \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.