Re: [PATCH 07/44] connect: add function to fetch value of a v2 server capability

From: Martin Ågren <martin.agren@gmail.com>
Date: May 13, 2020, 19:37 UTC
Message-ID: <CAN0heSq1ULyJE6FLsNMc-V+J1ke8ZqyVF-MghK_d+0i7SSfTDA@mail.gmail.com>
In-Reply-To: <20200513005424.81369-8-sandals@crustytoothpaste.net>

On Wed, 13 May 2020 at 02:58, brian m. carlson <sandals@crustytoothpaste.net> wrote:

Show 15 quoted lines

> +int server_feature_v2(const char *c, const char **v)
> +{
> +       int i;
> +
> +       for (i = 0; i < server_capabilities_v2.argc; i++) {
> +               const char *out;
> +               if (skip_prefix(server_capabilities_v2.argv[i], c, &out) &&
> +                   (*out == '=')) {
> +                       *v = out + 1;
> +                       return 1;
> +               }
> +       }
> +       return 0;
> +}
> +

This looks like it was based on `server_supports_feature()`, which explains the "1 means yup got it, 0 means no match". The name of `server_supports_feature()` does suggest the boolean nature of return value. For this new function, I would perhaps have expected "0 means success, negative means error". That said, I'm not familiar with connect.c. Let's see how this is used...

>  int server_supports_feature(const char *c, const char *feature,
>                             int die_on_error)
>  {

Just a thought:

Maybe this existing function could learn to take a pointer (or NULL) and assign to it if we have a '=' (possibly even requiring a '=' if this new pointer is non-NULL). I dunno, maybe two similar functions are better after all than having one with modes like that.

Martin