Re: [PATCH v2 2/6] pack-bitmap-write.c: write lookup table extension
Taylor Blau <me@ttaylorr.com> wrote:
Show 6 quoted lines
> We should probably leave this as a pointer to an off_t, since that is a
> more appropriate type for keeping track of an offset within a file (and
> indeed it is the return type of hashfile_total()).
>
> But since it's platform-dependent, we should make sure to cast it to a
> uint64_t before writing it as part of the lookup table.
Hmm, will make the necessary changes.
Show 5 quoted lines
> That at least removes a side-effect from the implementation of
> write_selected_commits_v1() and brings the creation of the
> commit_positions array closer to where it's being used, while still
> maintaining the constant-time lookups. So that may be a good
> alternative, but I'm curious of your thoughts.
> I think the construction of table and table_inv could definitely benefit
> from a comment here indicating what they're used for and what they
> contain (e.g., "table maps abc to xyz").
Yeah, true. Will add comments.
Thanks for the other suggestions also :)
Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com>