On Wed, Mar 29, 2017 at 11:21:52PM +0000, brian m. carlson wrote: > On Tue, Mar 28, 2017 at 03:07:12AM -0400, Jeff King wrote: > > It took me a while to find it. This is the switch from "len == 48" to > > "len > 8" when matching "shallow" lines. I think this makes sense. > > > > > Note that in queue_command we are guaranteed to have a NUL-terminated > > > buffer or at least one byte of overflow that we can safely read, so the > > > linelen check can be elided. We would die in such a case, but not read > > > invalid memory. > > > > I think linelen is always just strlen(line). Since the queue_command > > function no longer cares about it, perhaps we can just omit it? > > I've just looked at this to put in a fix, and I don't agree. We are > guaranteed that we'll have overflow, but linelen can point to a newline, > not just a NUL, so it isn't always strlen(line). This is the case in > queue_commands_from_cert. > > We do use this value, in computing the reflen value, so I think it's > better to keep it for now. I agree that a future refactor could convert > it to be NUL-terminated, but I'd rather not make that change here.
Yeah, you might have missed my followup: http://public-inbox.org/git/20170328173536.ylwesrj7jbrez...@sigill.intra.peff.net/ Basically yes, I agree it's probably not worth trying to refactor further. -Peff
