Re: [PATCH 18/20] pack-revindex: remove unused 'find_revindex_position()'
flat view
From: Derrick Stolee <hidden>
Date: 2021-01-11 11:57:44
On 1/8/2021 1:17 PM, Taylor Blau wrote:
quoted hunk ↗ jump to hunk
-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