On 12/8/2021 2:26 PM, Taylor Blau wrote: > This series demonstrates and fixes a problem (which has existed since the > inception of MIDX bitmaps) which made it possible to reliably introduce bitmap > corruption into a repository. > > The recipe (which is explained in detail in the first patch) roughly looks like > creating a MIDX with some packs. Then re-generating the MIDX and its bitmap but > under a different object order (e.g., by changing the `--preferred-pack`). > > Because we used to link the new .rev file into place, and because the object > order is not encoded into the MIDX itself, we would inadvertently generate a > .rev file with the same name as the existing one that contains different > contents, and then fail to move it into place. That causes us to read the > .bitmap under the old object order, producing incorrect results. > > This series applies a minimal fix (in the second patch), which is to include the > pack ordering in the MIDX itself. Some small thoughts here: > > - We could alternatively have included the entire .rev file's contents in the > MIDX. But this ends up being kind of awkward (and is also discussed in the > second patch). > > - We could cache the identity of the preferred pack (which we currently > discover by looking up the pack for the object in the 0th position of the > MIDX's pseudo-pack order) by just reading the first value of the new > optional chunk. > > I'm certainly interested in pursuing the latter, but in a different series, > since I'd like to keep this fix as minimal as possible. > > Taylor Blau (2): > t5326: demonstrate bitmap corruption after permutation > midx.c: make changing the preferred pack safe Just chiming in to say that I reviewed an earlier version of this series and the version in this submission looks good to me. Thanks, -Stolee
