On Tue, Jan 12, 2021 at 04:11:59AM -0500, Jeff King wrote: > > - Add an additional guard to check that the offset 'obj_offset()' > > points to a real object. This should be the case with well-behaved > > callers to 'packed_object_info()', but isn't guarenteed. > > > > Other blocks that fill in various other values from the 'struct > > object_info' request handle bad inputs by setting the type to > > 'OBJ_BAD' and jumping to 'out'. Do the same when given a bad offset > > here. > > Also good. I wonder if we need to call error() here, too. The caller > will probably say something like "bad object" or whatever, but the user > will have no clue that it's related to the revindex. > > That would match other parts of the function (e.g., calling into > unpack_entry() can generate lots of descriptive errors about exactly > what went wrong). Indeed, and we have the object's offset and pack name to include, too, so our error can be quite descriptive. > Yep. Again, I'm really happy to see these "should never happen" cases > converted to real errors or even BUG()s. :-). Thanks, Taylor
