Hi Peff, On Mon, 8 Dec 2014, Jeff King wrote: > On Mon, Dec 08, 2014 at 12:35:27PM +0100, Johannes Schindelin wrote: > > > On Mon, 8 Dec 2014, Duy Nguyen wrote: > > > > > On Mon, Dec 08, 2014 at 12:57:06AM -0500, Jeff King wrote: > > > > I do admit that I am tempted to teach index-pack to always NUL-terminate > > > > objects in memory that we feed to fsck, just to be on the safe side. It > > > > doesn't cost much, and could prevent a silly mistake (either in the > > > > future, or one that I missed in my analysis). > > > > > > I think I'm missing a "but.." here. > > > > The "but..."s I have are: > > > > 1) we potentially waste space, and > > I think this can be ignored. It's 1 byte per object, and only while we > keep the object in RAM. Also, we already do it for buffers read from > read_sha1_file, so when you run "git log" every commit buffer we keep in > RAM is already doing this (and has been since basically day one). Fine with me. > > 2) I would like to make really certain, preferably with static analysis, > > that fsck_object() only receives buffers that are NUL terminated, and > > that no call path is missed. > > I know this is not as good as a real static analysis, but I was > concerned about this exact thing about a year ago (I think in relation > to commit parsing for pretty-printing) and traced all of the paths > through which you can get an object; they all end up in the same few > code paths that all xmallocz: unpack_sha1_file for loose objects, > unpack_compressed_entry for pack bases, and patch_delta for deltas. Thank you for sharing the analysis. This is exactly what I was looking for. > Index-pack and unpack-objects are the odd men out here because they are > processing objects that are not actually in the repository yet. I think > the spots Duy pointed out probably cover index-pack. It looks like > builtin/unpack-objects.c:get_data needs the same treatment. I just started working on that. To see the progress, please have a look here: https://github.com/dscho/git/pull/5 Ciao, Dscho -- 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