On Sat, Jan 28, 2023 at 9:08 AM M Hickford via GitGitGadget
<gitgitgadget@xxxxxxxxx> wrote:
> If password has expired, credential fill no longer returns early,
> so later helpers can generate a fresh credential. This is backwards
> compatible -- no change in behaviour with helpers that discard the
> expiry attribute. The expiry logic is entirely in the git credential
> layer; compatible helpers simply store and return the expiry
> attribute verbatim.
>
> Store new attribute in cache.
>
> Signed-off-by: M Hickford <mirth.hickford@xxxxxxxxx>
Just a few comments in addition to those already provided by Junio...
> diff --git a/credential.c b/credential.c
> @@ -234,11 +236,23 @@ int credential_read(struct credential *c, FILE *fp)
> + // if expiry date has passed, ignore password and expiry fields
> + if (c->password_expiry_utc != 0 && time(NULL) > c->password_expiry_utc) {
> + trace_printf(_("Password has expired.\n"));
Using `_(...)` marks a string for localization, but doing so is
undesirable for debugging messages which are meant for the developer,
not the end user (and it creates extra work for translators). No
existing[1] trace_printf() calls in the codebase use `_(...)`.
[1]: Unfortunately, a couple examples exist in
Documentation/MyFirstObjectWalk.txt using `_(...)` but they should be
removed.
> @@ -269,6 +283,13 @@ void credential_write(const struct credential *c, FILE *fp)
> + if (c->password_expiry_utc != 0) {
> + int length = snprintf( NULL, 0, "%ld", c->password_expiry_utc);
> + char* str = malloc( length + 1 );
Style in this project is `char *str`, not `char* str`. Also, drop
spaces around function arguments:
char *str = malloc(length + 1);
> + snprintf( str, length + 1, "%ld", c->password_expiry_utc );
Same.
> + credential_write_item(fp, "password_expiry_utc", str, 0);
> + free(str);
> + }
xstrfmt() from strbuf.h can help simplify this entire block:
char *s = xstrfmt("%ld", c->password_expiry_utc);
credential_write_item(fp, "password_expiry_utc", str, 0);
free(s);
