Patrick Steinhardt <ps@xxxxxx> writes: >> + if (revs->do_not_die_on_missing_objects) >> + oidset_init(&revs->missing_objects, 0); >> + > > While we're initializing the new oidset, we never clear it. We should > probably call `oidset_clear()` in `release_revisions()`. And if we > unconditionally initialized the oidset here then we can also call > `oiadset_clear()` unconditionally there, which should be preferable > given that `oidset_init()` does not allocate memory when no initial size > was given. Yup, I used the conditional one to match the above, but initializing unused oidset is cheap and frees us from having to worry about mistakes. I like your idea much better. >> + >> + /* Missing objects to be tracked without failing traversal. */ >> + struct oidset missing_objects; > > As far as I can see we only use this set to track missing commits, but > none of the other objects. The name thus feels a bit misleading to me, > as a reader might rightfully assume that it contains _all_ missing > objects after the revwalk. Should we rename it to `missing_commits` to > clarify? Again, very good suggestion. Thanks.
