On Tue, Aug 20, 2024 at 04:19:13PM -0700, Junio C Hamano wrote: > Patrick Steinhardt <ps@xxxxxx> writes: > > > When writing the MIDX file we first create the `struct hashfile` used to > > write the trailer hash, and then afterwards we verify whether we can > > actually write the MIDX in the first place. When we decide that we > > can't, this leads to a memory leak because we never free the hash file > > contents. > > > > We could fix this by freeing the hashfile on the exit path. There is a > > better option though: we can simply move the checks for the error > > condition earlier. As there is no early exit between creating the > > hashfile and finalizing it anymore this is sufficient to fix the memory > > leak. > > The above is a good explanation why "are we dropping everything" > block was moved up, but does not explain why the other "if there is > no objects" block has to move (it is however easy to see that the > latter block can be moved without any bad side effect). > > In any case, the struct hashfile hashfd() gives us is now associated > with the struct chunkfile cf immediately after it is instanciated, > and there is no early exit while the chunkfile is in use, which is > great. There basically is no reason why the other block needs to move. It just felt more natural to keep massaging of parameters closly together before we go off and actually do "the thing". I'll document this accordingly. Patrick
