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

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

On Wed, 13 May 2020 at 02:58, brian m. carlson
<sandals@crustytoothpaste.net> wrote:
> +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