Re: [PATCH 18/20] pack-revindex: remove unused 'find_revindex_position()'
- From
Derrick Stolee <stolee@gmail.com>
- Date
- Jan 11, 2021, 11:57 UTC
- Message-ID
- <624d0642-b6c9-7c76-aeb6-d7e18b0aad1f@gmail.com>
- In-Reply-To
- <d60411d524656f4680ac578765b2a8704325a060.1610129796.git.me@ttaylorr.com>
On 1/8/2021 1:17 PM, Taylor Blau wrote:
Show 42 quoted lines
> -int find_revindex_position(struct packed_git *p, off_t ofs)
> +int offset_to_pack_pos(struct packed_git *p, off_t ofs, uint32_t *pos)
> {
> int lo = 0;
> - int hi = p->num_objects + 1;
> - const struct revindex_entry *revindex = p->revindex;
> + int hi;
> + const struct revindex_entry *revindex;
> +
> + if (load_pack_revindex(p) < 0)
> + return -1;
> +
> + hi = p->num_objects + 1;
> + revindex = p->revindex;
>
> do {
> const unsigned mi = lo + (hi - lo) / 2;
> if (revindex[mi].offset == ofs) {
> - return mi;
> + *pos = mi;
> + return 0;
> } else if (ofs < revindex[mi].offset)
> hi = mi;
> else
> @@ -189,20 +196,6 @@ int find_revindex_position(struct packed_git *p, off_t ofs)
> return -1;
> }
>
> -int offset_to_pack_pos(struct packed_git *p, off_t ofs, uint32_t *pos)
> -{
> - int ret;
> -
> - if (load_pack_revindex(p) < 0)
> - return -1;
> -
> - ret = find_revindex_position(p, ofs);
> - if (ret < 0)
> - return -1;
> - *pos = ret;
> - return 0;
> -}
> -Not that this is new to the current patch, but this patch made me wonder if we should initialize *pos = -1 in the case of a failure to find the position? A correct caller should not use the value if they are checking for the fail-to-find case properly. But, I could see someone making a mistake and having trouble diagnosing the problem because their position variable was initialized to zero or a previous successful case.
Thanks, -Stolee