Re: [PATCH 2/2] commit: sign commit after mutating buffer

["brian m. carlson" <sandals@crustytoothpaste.net>]

[-- Attachment #1: Type: text/plain, Size: 1796 bytes --]

On 2026-04-22 at 15:10:29, Elijah Newren wrote:
> On Mon, Apr 20, 2026 at 3:14 PM brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
> > diff --git a/commit.c b/commit.c
> > index 790dd2faed..bc41859be1 100644
> > --- a/commit.c
> > +++ b/commit.c
> > @@ -1747,6 +1747,11 @@ int commit_tree_extended(const char *msg, size_t msg_len,
> >                 oidcpy(&parent_buf[i++], &p->item->object.oid);
> >
> >         write_commit_tree(&buffer, msg, msg_len, tree, parent_buf, nparents, author, committer, extra);
> > +
> > +       /* And check the encoding. */
> > +       if (encoding_is_utf8 && !ensure_utf8(&buffer))
> > +               fprintf(stderr, _(commit_utf8_warn));
> > +
> >         if (sign_commit && sign_buffer(&buffer, &sig, sign_commit,
> >                                        SIGN_BUFFER_USE_DEFAULT_KEY)) {
> >                 result = -1;
> > @@ -1780,6 +1785,9 @@ int commit_tree_extended(const char *msg, size_t msg_len,
> >                 free_commit_extra_headers(compat_extra);
> >                 free(mapped_parents);
> >
> > +               if (encoding_is_utf8 && !ensure_utf8(&compat_buffer))
> > +                       fprintf(stderr, _(commit_utf8_warn));
> > +
> 
> So the users might see "commit message did not conform to UTF-8..."
> twice? (Isn't compat_buffer likely to have invalid UTF-8 whenever
> buffer does?)  Do we want to avoid that double printing?

Yeah, I'll fix that in v2.

> Did the change in this patch also fix a short-circuiting error?
> Previously, when both buffers had invalid UTF-8, we'd only call
> ensure_utf8() on the first one and fix it, and then short-circuit and
> not handle compat_buffer, right?

I believe it did, yes.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 325 bytes --]