Re: [PATCH v3 05/35] upload-pack: factor out processing lines

From: Stefan Beller <sbeller@google.com>
Date: Feb 22, 2018, 19:31 UTC
Message-ID: <CAGZ79kanWXGa3v4oJmoerh3XzisS+3V2oyO7WXK-tPTiHgSgxg@mail.gmail.com>
In-Reply-To: <20180207011312.189834-6-bmwill@google.com>

On Tue, Feb 6, 2018 at 5:12 PM, Brandon Williams <bmwill@google.com> wrote:

Show 6 quoted lines

> Factor out the logic for processing shallow, deepen, deepen_since, and
> deepen_not lines into their own functions to simplify the
> 'receive_needs()' function in addition to making it easier to reuse some
> of this logic when implementing protocol_v2.
>
> Signed-off-by: Brandon Williams <bmwill@google.com>

Reviewed-by: Stefan Beller <sbeller@google.com>
for the stated purpose of just refactoring existing code for better reuse later.

I do have a few comments on the code in general, which might be out of scope for this series.

A close review would have been fastest if we had some sort of https://public-inbox.org/git/20171025224620.27657-1-sbeller@google.com/ which I might revive soon for this purpose. (it showed that I would need it)

> +               *depth = (int)strtol(arg, &end, 0);

strtol is not used quite correctly here IMHO, as we do not inspect errno for ERANGE

Thanks, Stefan