Re: [PATCH v3 3/4] Provide a dirname() function when NO_LIBGEN_H=YesPlease

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 11, 2016, 22:56 UTC
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:

Show 25 quoted lines

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