Re: [PATCH bw/realpath-wo-chdir] real_path: canonicalize directory separators in root parts

From: Brandon Williams <bmwill@google.com>
To: Johannes Sixt <j6t@kdbg.org>
Cc: git@vger.kernel.org, sbeller@google.com, peff@peff.net,
	jacob.keller@gmail.com, gitster@pobox.com,
	ramsay@ramsayjones.plus.com, tboegi@web.de, pclouds@gmail.com
Subject: Re: [PATCH bw/realpath-wo-chdir] real_path: canonicalize directory separators in root parts
Date: Thu, 22 Dec 2016 09:33:01 -0800	[thread overview]
Message-ID: <20161222173301.GB119874@google.com> (raw)
In-Reply-To: <0c9aa347-d64e-b7d7-9b07-52d844d76252@kdbg.org>

On 12/22, Johannes Sixt wrote:
> Am 21.12.2016 um 23:33 schrieb Brandon Williams:
> >On 12/21, Johannes Sixt wrote:
> >>+/* copies root part from remaining to resolved, canonicalizing it on the way */
> >>+static void get_root_part(struct strbuf *resolved, struct strbuf *remaining)
> >>+{
> >>+	int offset = offset_1st_component(remaining->buf);
> >>+
> >>+	strbuf_reset(resolved);
> >>+	strbuf_add(resolved, remaining->buf, offset);
> >>+#ifdef GIT_WINDOWS_NATIVE
> >>+	convert_slashes(resolved->buf);
> >>+#endif
> >
> >So then the only extra cononicalization that is happening here is
> >converting '\\server\share' to '//server/share'? (or 'c:\' to 'c:/')
> 
> Correct. All other directory separators are canonicalized by the
> primary function, strbuf_realpath.

Sounds good. Logically everything looks good to me.  And I like that
setting 'resolved' to the root of an abs path is pulled out into a
helper function.  It took me a couple extra seconds to realize that
offset_1st_component returns 0 with a relative path, which makes causes
the call to get_root_part to essentially be a noop (ie nothing is
resolved).

Thanks for helping get this to work on windows!

-- 
Brandon Williams