git/pack-bitmap.c, branch v2.24.0

Merge branch 'jk/misc-uninitialized-fixes'

2019-09-30T04:19:30Z

Various fixes to codepaths gcc 9 had trouble following dataflow. * jk/misc-uninitialized-fixes: pack-objects: drop packlist index_pos optimization test-read-cache: drop namelen variable diff-delta: set size out-parameter to 0 for NULL delta bulk-checkin: zero-initialize hashfile_checkpoint pack-objects: use object_id in packlist_alloc() git-am: handle missing "author" when parsing commit

pack-objects: drop packlist index_pos optimization

2019-09-06T18:03:42Z

Once upon a time, the code to add an object to our packing list in pack-objects all lived in a single function. It computed the position within the hash table once, then used it to check if the object was already present, and if not, to add it. Later, in 2834bc27c1 (pack-objects: refactor the packing list, 2013-10-24), this was split into two functions: packlist_find() and packlist_alloc(). We ended up with an "index_pos" variable that gets passed through several functions to make it from one to the other. The resulting code is rather confusing to follow. The "index_pos" variable is sometimes undefined, if we don't yet have a hash table. This works out in practice because in that case packlist_alloc() won't use it at all, since it will have to create/grow the hash table. But it's hard to verify that, and it does cause gcc 9.2.1's -Wmaybe-uninitialized to complain when compiled with "-flto -O3" (rightfully, since we do pass the uninitialized value as a function parameter, even if nobody ends up using it). All of this is to save computing the hash index again when we're inserting into the hash table, which I found doesn't make a measurable difference in the program runtime (which is not surprising, since we're doing all kinds of other heavyweight things for each object). Let's just drop this index_pos variable entirely, simplifying the code (and pleasing the compiler). We might be better still refactoring this custom hash table to use one of our existing implementations (an oidmap, or a kh_oid_map). I stopped short of that here, but this would be the likely first step towards that anyway. Reported-by: Stephan Beyer Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

tag: factor out get_tagged_oid()

2019-09-05T21:10:18Z

Add a function for accessing the ID of the object referenced by a tag safely, i.e. without causing a segfault when encountering a broken tag where ->tagged is NULL. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano

pack-bitmap: convert khash_sha1 maps into kh_oid_map

2019-06-20T17:35:05Z

All of the users of our khash_sha1 maps actually have a "struct object_id". Let's use the more descriptive type. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

khash: drop broken oid_map typedef

2019-06-20T17:21:27Z

Commit 5a8643eff1 (khash: move oid hash table definition, 2019-02-19) added a khash "oid_map" type to match the existing "oid" type, which is a simple set (i.e., just keys, no values). But in setting up the khash_oid_map typedef, it accidentally referred to "kh_oid_t", which is the set type. Nobody noticed the breakage because there are not yet any callers; the type was added just as a match to the existing sha1 types (whose map type confusingly _is_ called khash_sha1, and it has no matching set type). We could easily fix this with s/oid/oid_map/ in the typedef. But let's take this a step further, and just drop the typedef entirely. These typedefs were added by 5a8643eff1 to match the khash_sha1 typedefs. But the actual khash-derived type names are descriptive enough; this is just adding an extra layer of indirection. The khash names do not quite follow our usual style (e.g., they end in "_t"), but since we end up using other khash names (e.g., khiter_t, kh_get_oid()) anyway, just typedef-ing the struct name is not really helping much. And there are already many cases where we use the raw khash type names anyway (e.g., the "set" variant defined just above us does not have such a typedef!). So let's drop this typedef, and the matching oid_pos one (which actually _does_ have a user, but we can easily convert it). We'll leave the khash_sha1 typedef around. The ultimate fate of its callers should be conversion to kh_oid_map_t, so there's no point in going through the noise of changing the names now. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

pack-objects: convert packlist_find() to use object_id

2019-06-20T16:54:58Z

We take a raw hash pointer, but most of our callers have a "struct object_id" already. Let's switch to taking the full struct, which will let us continue removing uses of raw sha1 buffers. There are two callers that do need special attention: - in rebuild_existing_bitmaps(), we need to switch to nth_packed_object_oid(). This incurs an extra hash copy over pointing straight to the mmap'd sha1, but it shouldn't be measurable compared to the rest of the operation. - in can_reuse_delta() we already spent the effort to copy the sha1 into a "struct object_id", but now we just have to do so a little earlier in the function (we can't easily convert that function's callers because they may be pointing at mmap'd REF_DELTA blocks). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

Merge branch 'bc/hash-transition-16'

2019-04-25T07:41:17Z

Conversion from unsigned char[20] to struct object_id continues. * bc/hash-transition-16: (35 commits) gitweb: make hash size independent Git.pm: make hash size independent read-cache: read data in a hash-independent way dir: make untracked cache extension hash size independent builtin/difftool: use parse_oid_hex refspec: make hash size independent archive: convert struct archiver_args to object_id builtin/get-tar-commit-id: make hash size independent get-tar-commit-id: parse comment record hash: add a function to lookup hash algorithm by length remote-curl: make hash size independent http: replace sha1_to_hex http: compute hash of downloaded objects using the_hash_algo http: replace hard-coded constant with the_hash_algo http-walker: replace sha1_to_hex http-push: remove remaining uses of sha1_to_hex http-backend: allow 64-character hex names http-push: convert to use the_hash_algo builtin/pull: make hash-size independent builtin/am: make hash size independent ...

pack-revindex: open index if necessary

2019-04-16T07:58:21Z

We can't create a pack revindex if we haven't actually looked at the index. Normally we would never get as far as creating a revindex without having already been looking in the pack, so this code never bothered to double-check that pack->index_data had been loaded. But with the new multi-pack-index feature, many code paths might not load the individual pack .idx at all (they'd find objects via the midx and then open the .pack, but not its index). This can't yet be triggered in practice, because a bug in the midx code means we accidentally open up the individual .idx files anyway. But in preparation for fixing that, let's have the revindex code check that everything it needs has been loaded. In most cases this will just be a quick noop. But note that this does introduce a possibility of error (if we have to open the index and it's corrupt), so load_pack_revindex() now returns a result code, and callers need to handle the error. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

pack-bitmap: switch hash tables to use struct object_id

2019-04-01T02:57:37Z

Instead of storing unsigned char pointers in the hash tables, switch to storing instances of struct object_id. Update several internal functions and one external function to take pointers to struct object_id. Signed-off-by: brian m. carlson Signed-off-by: Junio C Hamano

pack-bitmap: switch hard-coded constants to the_hash_algo

2019-04-01T02:57:37Z

Switch two hard-coded uses of 20 to references to the_hash_algo. Signed-off-by: brian m. carlson Signed-off-by: Junio C Hamano