On 2025-05-22 at 21:09:23, Junio C Hamano wrote:
> "brian m. carlson" <sandals@xxxxxxxxxxxxxxxxxxxx> writes:
> > + const char *author, *committer;
> > + size_t author_len, committer_len;
> > + const char *p;
> > + const char *expected = "git stash <git@stash> 1000684800 +0000";
>
> Makes me wonder if we can somehow share this with [4/5] where we
> establish the author and committer ident (timestamp spelled in a
> different form). Three strings that has to stay in sync is not a
> huge deal, though.
I'll take a look and see if I can come up with a nice way to solve this.
> > + p = strstr(buffer, "\n\n");
> > + if (!p) {
> > + res = error(_("cannot parse commit %s"), oid_to_hex(&this->object.oid));
> > + goto out;
> > + }
> > +
> > + p += 2;
> > + if (((size_t)(bufsize - (p - buffer)) < strlen(prefix)) ||
> > + memcmp(prefix, p, strlen(prefix))) {
> > + res = error(_("found stash commit %s with unexpected prefix"), oid_to_hex(&this->object.oid));
> > + goto out;
> > + }
>
> I'd call that "without expected prefix" (the difference being "we
> expected 'git stash:' prefix but you have 'git stosh:' prefix" vs
> "your commit title is 'hello world'"), but OK. The important thing
> is that we are being careful not to get confused.
Will fix.
> It is unfortunate historical practice to measure almost everything
> in "unsigned long" and bufsize here is one of them, but do we need
> that cast? We know strlen(prefix) is a small known constant
> integer, we know p is not smaller than buffer, we know (p-buffer) is
> not larger than bufsize.
Yes, we do, because Windows will complain about a comparison between
signed and unsigned integer. `p - buffer` is `ptrdiff_t`, which is 64
bits, but `unsigned long` is only 32 bite, so they both get promoted to
long long, which is 64 bits and signed, and then the comparison is to
`size_t`, which is 64 bits and unsigned. The CI jobs fail without this.
I'll make a note in the commit message to that effect.
> OK. Again, I suspect that commit_list may be a more natural thing
> to use to accumulate the stash entries, but that is not a huge deal.
I'll try to fix that in v7.
--
brian m. carlson (they/them)
Toronto, Ontario, CA
Attachment:
signature.asc
Description: PGP signature
