On Mon, Mar 20, 2023 at 04:02:40PM -0400, Taylor Blau wrote: > So b5007211b6 got rid of the above convention and replaced it by reading > individual fields of that structure with a `read_u8()` helper that reads > from the region of memory pointed to by `->map`, and updates the > `->map_pos` pointer accordingly. > > But this forces callers to be intimately aware of `bitmap_git->map` and > `bitmap_git->map_pos`. Instead, teach `read_u8()` to take a `struct > bitmap_index *` directly, and avoid having callers deal with the > internals themselves. OK. I always felt like this read_u8() and read_be32() were trying to match get_be32(), etc, just with an auto-incrementing "pos" pointer. And this patch makes them a lot less generic. But that is probably OK. They are static-local to the bitmap file, and we have not found another caller who wanted them in the intervening years. Arguably they could be given more descriptive names, like read_bitmap_u8() or something, but again, being static-local to the file, the generic names are fine. -Peff
