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

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

 



Junio C Hamano wrote:

[...]
> Then the additional test can become part of the patch that corrects the
> parsing logic, no?

Yes, that works, too.  All I was trying to say was that the
description in the patch I quoted didn't make sense to me, since it
included a mention of a buffer overflow without giving any explanation
of what it was talking about.  I don't actually care in this case
whether it is fixed by mentioning which patch this is testing the fix
from or by squashing the two patches (though the latter certainly
seems reasonable).

Incidentally, Ram might wonder why I fuss so much about commit
messages.  It's actually very simple --- I think of them as part of
the code.  Suppose someone discovers a regression was introduced by
such-and-such part of the patch 1.7.7 -> 1.7.8, but at first glance it
is not clear whether that code change was supposed to have any effect
on the behavior of the code at all.  Such a person is likely to make
mistakes in fixing it, right?  So after getting the right behavior,
patch authors spend a few extra minutes to make sure the code is
intuitive to humans, too, and this includes making sure the rationale
description is clear.

Just like the code for the computer, this is very much something that
isn't always going to be right the first time and sometimes takes some
debugging.  So, sorry for the fuss, but I hope it helps.

Jonathan
--
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]