From: Junio C Hamano <gitster@pobox.com>
Date: Mon, 11 Jan 2016 22:56:30 GMT
Subject: Re: [PATCH v3 3/4] Provide a dirname() function when NO_LIBGEN_H=YesPlease
Message-ID: <xmqqtwmjvi75.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAPig+cQmhc=DmptyYWy1p3z4rz7_h-3XRrtFH7XxoW77z5Mz-A@mail.gmail.com>

Eric Sunshine <sunshine@sunshineco.com> writes:

> I wonder if this would be a bit easier to follow if it was structured
> something like this:
>
>     static struct strbuf buf = STRBUF_INIT;
>
>     if ((dos_drive_prefix = skip_dos_drive_prefix(&p)) && !*p)
>         goto dot;
>
>     ...
>     if (is_dir_sep(*p)) {
>         ...
>     }
>     ...
>     while ((c = *(p++)))
>         ...
>
>     if (slash) {
>         *slash = '\0';
>         return path;
>     }
>
>     dot:
>     strbuf_reset(&buf);
>     strbuf_addf(&buf, "%.*s.", dos_drive_prefix, path);
>     return buf.buf;

I'll queue the one from Dscho as-is for today, but avoiding the
"jump back to a place where it happens to have an identical clean-up
that need to happen" and defining the clean-up path at the end like
this would probably be easier to follow.  It certainly would have
saved one comment in the previous review cycle from me.

Thanks.