Re: [PATCH 3/6] revert: fix buffer overflow in insn sheet parser

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jonathan Nieder <jrnieder@xxxxxxxxx> writes:

> Looks good, except I would explain it differently, to avoid referring
> to hypothetical implementation details ("What buffer overflow?"):
>
> 	test: git cherry-pick --continue should cope with long object names
>
> 	A naive implementation that uses a commit-id-shaped buffer
> 	to store the word after "pick" in .git/sequencer/todo lines
> 	would crash often.  Our implementation is not so naive, but
> 	add a test anyway to futureproof it.
>
> Or:
>
> 	test: make sure the "cherry-pick --continue" buffer overflow doesn't come back
>
> 	Before commit ..., "git cherry-pick --continue" would overflow
> 	under ... circumstance.  Add a test to make sure it doesn't
> 	happen again.

I doubt you would need any of that.

You can just explain the commit that stops copying the lines into a
private, fixed buffer a bit better (e.g. "such copying is not just
wasteful but is wrong by unnecessary placing an artificial limit on the
line length"), and say "Incidentally, this fixes a bug in the earlier
round of this series that failed to read lines that are too long to fit on
the buffer, demonstrated by the test added by this patch", or something.

Then the additional test can become part of the patch that corrects the
parsing logic, no?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]