Thread (28 messages) flat view 28 messages, 3 authors, 2017-02-18

Re: [PATCH v3 02/19] builtin/diff-tree: convert to struct object_id

From: brian m. carlson <hidden>
Date: 2017-02-18 19:12:31

On Fri, Feb 17, 2017 at 10:15:31PM -0500, Jeff King wrote:
So for this case, something like the patch below.

Incidentally, there's an off-by-one in the original loop of
stdin_diff_commit that reads past the end of the trailing NUL for the
final sha1 on the line. The problem is the:

  pos += GIT_SHA1_HEXSZ + 1;

which assumes we're slurping up the trailing space. This works in
practice because the caller will only permit a string which had a
newline (which it converted into a NUL).

I suspect that function could be more aggressive about complaining about
nonsense on the line, rather than silently ignoring it.
I'd come to basically the same patch, but I did pick up a few niceties
from your patch, like avoiding the off-by-one issue you mentioned above.
Can I place your sign-off on the resulting change?
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help