On Wed, Jan 18, 2023 at 07:05:32PM +0100, René Scharfe wrote: > > 2. The point of this code is to find malformed input to hash-object. > > We're probably better off feeding the buffer to fsck_commit(), etc. > > It does more thorough checks, and these days it does not need an > > object struct at all. > > I like the second one, as long as it won't check too much. c879daa237 > (Make hash-object more robust against malformed objects, 2011-02-05) added > the checks that are now in object-file.c and intended to only validate the > internal structure of objects, not relations between. It gave the example > to allow adding a commit before its tree, which should be allowed. And > IIUC fsck_object() fits that bill. Yes, I think it will do what the right thing here. And having just written up a quick series, the only tests which needed changes were ones with syntactic problems. :) I'll send it out in a few minutes. > > Either of which would naturally fix the leak for tags. I'm not sure > > there actually is a leak for commits, as commit structs don't store any > > strings themselves. > > parse_commit_buffer() allocates the list of parents. Yes, but it does so with lookup_commit(), so the resulting commit objects are themselves reachable from the usual obj_hash, and thus not leaked. > Hmm, and it looks them up. Doesn't this violate the goal to allow > dangling references? No, because lookup_commit() is just about creating an in-process struct. It doesn't look at the object database at all (though it would complain if we had seen the same oid in-process as another type). -Peff