Re: [PATCH v8 3/3] credential: add WWW-Authenticate header to cred requests

From: Matthew John Cheetham <mjcheetham@outlook.com>
Date: Feb 15, 2023, 19:19 UTC
Message-ID: <AS2PR03MB9815361C9F2E52C71F7284D0C0A39@AS2PR03MB9815.eurprd03.prod.outlook.com>
In-Reply-To: <230206.867cwu5xmu.gmgdl@evledraar.gmail.com>

On 2023-02-06 12:45, Ævar Arnfjörð Bjarmason wrote:

Show 23 quoted lines

> 
> On Mon, Feb 06 2023, Matthew John Cheetham via GitGitGadget wrote:
> 
>> From: Matthew John Cheetham <mjcheetham@outlook.com>
> 
>> @@ -263,6 +263,16 @@ static void credential_write_item(FILE *fp, const char *key, const char *value,
>>  	fprintf(fp, "%s=%s\n", key, value);
>>  }
>>  
>> +static void credential_write_strvec(FILE *fp, const char *key,
>> +				    const struct strvec *vec)
>> +{
>> +	char *full_key = xstrfmt("%s[]", key);
> 
> FWIW you could avoid this allocation if you just renamed the current
> "credential_write_item()" to "credential_write_fmt()", and had it take a
> format instead of its current hardcoded "%s=%s\n".
> 
> Then you could have two wrappers, credential_write_item() and
> credential_write_items() (instead of "strvec"), the first passing
> "%s=%s\n", the other "%s[]=%s\n".
> 
> Just a thought.

Given this is the only `items/strvec` that I'm writing out, just inlining the for-loop and calling `credential_write_item` directly with the key as "wwwauth[]" would avoid the allocation without needing to make the write_item function more complicated.

-static void credential_write_strvec(FILE *fp, const char *key,
-                                   const struct strvec *vec)
-{
-       char *full_key = xstrfmt("%s[]", key);
-       for (size_t i = 0; i < vec->nr; i++) {
-               credential_write_item(fp, full_key, vec->v[i], 0);
-       }
-       free(full_key);
-}
-
 void credential_write(const struct credential *c, FILE *fp)
 {
        credential_write_item(fp, "protocol", c->protocol, 1);

@@ -280,7 +270,10 @@ void credential_write(const struct credential *c, FILE *fp)
        credential_write_item(fp, "path", c->path, 0);
        credential_write_item(fp, "username", c->username, 0);
        credential_write_item(fp, "password", c->password, 0);
-       credential_write_strvec(fp, "wwwauth", &c->wwwauth_headers);
+       for (size_t i = 0; i < c->wwwauth_headers.nr; i++)
+               credential_write_item(fp, "wwwauth[]", c->wwwauth_headers.v[i],
+                                     0);
 }


>> +	for (size_t i = 0; i < vec->nr; i++) {
>> +		credential_write_item(fp, full_key, vec->v[i], 0);
> 
> The {} here can be dropped in any case.
>