Re: [PATCH 5/9] serve: provide "receive" function for session-id capability

["Martin Ågren" <martin.agren@gmail.com>]

On Tue, 14 Sept 2021 at 17:34, Jeff King <peff@peff.net> wrote:
>
> Rather than pulling the session-id string from the list of collected
> capabilities, we can handle it as soon as we receive it. This gets us
> closer to dropping the collected list entirely.

Looking good.

> As this removes the last caller of the static has_capability(), we can
> remove it, as well (and in fact we must to avoid -Wunused-function
> complaining).

> I had originally dropped has_capability() in a separate patch, to keep
> this one more readable. That breaks bisectability, but only with
> -Werror. I'm not sure where we should fall on that spectrum (I generally
> bisect with -Wno-error just because warnings may come and go when
> working with different compilers than what was normal at the time).
>
> Not that big a deal either way for this patch, but I wonder if people
> have opinions in general.

First of all, agreed about the "not that big a deal" part. Just a random
thought: You could do the opposite of what Elijah sometimes does by
first adding a "MAYBE_UNUSED" function, then actually using it. You'd
add "MAYBE_UNUSED" here, then the next commit would drop the whole
thing. It could be worth it if you're removing many many lines so that
the "actual" change gets lost in the noise. But this patch isn't near
any such threshold, IMHO (if there even is such a "threshold").

> +static void session_id_receive(struct repository *r,
> +                              const char *client_sid)
> +{
> +       if (!client_sid)
> +               client_sid = "";
> +       trace2_data_string("transfer", NULL, "client-sid", client_sid);
> +}

Handling NULL. Nice. :)


Martin