"Junio C Hamano via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> Currently, the cleansing of the reflog message is done by the files
> backend, before the log is written out. This is sufficient with the
> current code, as that is the only backend that writes reflogs. But
> new backends can be added that write reflogs, and we'd want the
> resulting log message we would read out of "log -g" the same no
> matter what backend is used.
I _think_ there is no correctness impact, but this slightly changes
the semantics. We used to take whatever random string from the
caller and at the lowest layer of the files backend, i.e.
> strbuf_addf(&sb, "%s %s %s", oid_to_hex(old_oid), oid_to_hex(new_oid), committer);
> if (msg && *msg)
> - copy_reflog_msg(&sb, msg);
> + strbuf_addstr(&sb, msg);
cleansed and added the message to strbuf.
With the updated code, normalize_reflog_message() is defined like so
> +static char *normalize_reflog_message(const char *msg)
> +{
> + struct strbuf sb = STRBUF_INIT;
> +
> + if (msg && *msg)
> + copy_reflog_msg(&sb, msg);
> + return strbuf_detach(&sb, NULL);
> +}
to always return a non-NULL but an empty string, even if the caller
gave us a NULL pointer. We always pass a non-NULL string around,
and the updated low-level code avoids calling strbuf_addstr()
because msg is not NULL, but *msg is '\0'.
We _might_ be able to optimize by tweaking the normalizer a bit
further, perhaps like so:
static char *normalize_reflog_message(const char *msg)
{
struct strbuf sb = STRBUF_INIT;
if (msg && *msg)
copy_reflog_msg(&sb, msg);
if (sb.len) {
return strbuf_detach(&sb, NULL);
} else {
strbuf_release(&sb);
return NULL;
}
}
to avoid allocating 1 byte '\0' on the heap by strbuf_detach() when
sb here ends up being truly empty.
But it would be a premature optimization to worry about such a thing
at this point, I think.
Thanks.
