Skip to content

Commit cc00e5c

Browse files
peffgitster
authored andcommittedAug 29, 2018
convert hashmap comparison functions to oideq()
The comparison functions used for hashmaps don't care about strict ordering; they only want to compare entries for equality. Let's use the oideq() function instead, which can potentially be better optimized. Note that unlike the previous patches mass-converting calls like "!oidcmp()", this patch could actually provide an improvement even with the current implementation. Those comparison functions are passed around as function pointers, so at compile-time the compiler cannot realize that the caller (which is in another file completely) will treat the return value as a boolean. Note that this does change the return values in quite a subtle way (it's still an int, but now the sign bit is irrelevant for ordering). Because of their funny hashmap-specific signature, it's unlikely that any of these static functions would be reused for more generic ordering. But to be double-sure, let's stop using "cmp" in their names. Calling them "eq" doesn't quite work either, because the hashmap convention is actually _inverted_. "0" means "same", and non-zero means "different". So I've called them "neq" by convention here. Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 67947c3 commit cc00e5c

File tree

3 files changed

+13
-13
lines changed

3 files changed

+13
-13
lines changed
 

‎builtin/describe.c

+3-3
Original file line numberDiff line numberDiff line change
@@ -62,15 +62,15 @@ static const char *prio_names[] = {
6262
N_("head"), N_("lightweight"), N_("annotated"),
6363
};
6464

65-
static int commit_name_cmp(const void *unused_cmp_data,
65+
static int commit_name_neq(const void *unused_cmp_data,
6666
const void *entry,
6767
const void *entry_or_key,
6868
const void *peeled)
6969
{
7070
const struct commit_name *cn1 = entry;
7171
const struct commit_name *cn2 = entry_or_key;
7272

73-
return oidcmp(&cn1->peeled, peeled ? peeled : &cn2->peeled);
73+
return !oideq(&cn1->peeled, peeled ? peeled : &cn2->peeled);
7474
}
7575

7676
static inline struct commit_name *find_commit_name(const struct object_id *peeled)
@@ -596,7 +596,7 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
596596
return cmd_name_rev(args.argc, args.argv, prefix);
597597
}
598598

599-
hashmap_init(&names, commit_name_cmp, NULL, 0);
599+
hashmap_init(&names, commit_name_neq, NULL, 0);
600600
for_each_rawref(get_name, NULL);
601601
if (!hashmap_get_size(&names) && !always)
602602
die(_("No names found, cannot describe anything."));

‎oidmap.c

+6-6
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
#include "cache.h"
22
#include "oidmap.h"
33

4-
static int cmpfn(const void *hashmap_cmp_fn_data,
5-
const void *entry, const void *entry_or_key,
6-
const void *keydata)
4+
static int oidmap_neq(const void *hashmap_cmp_fn_data,
5+
const void *entry, const void *entry_or_key,
6+
const void *keydata)
77
{
88
const struct oidmap_entry *entry_ = entry;
99
if (keydata)
10-
return oidcmp(&entry_->oid, (const struct object_id *) keydata);
11-
return oidcmp(&entry_->oid,
10+
return !oideq(&entry_->oid, (const struct object_id *) keydata);
11+
return !oideq(&entry_->oid,
1212
&((const struct oidmap_entry *) entry_or_key)->oid);
1313
}
1414

@@ -21,7 +21,7 @@ static int hash(const struct object_id *oid)
2121

2222
void oidmap_init(struct oidmap *map, size_t initial_size)
2323
{
24-
hashmap_init(&map->map, cmpfn, NULL, initial_size);
24+
hashmap_init(&map->map, oidmap_neq, NULL, initial_size);
2525
}
2626

2727
void oidmap_free(struct oidmap *map, int free_entries)

‎patch-ids.c

+4-4
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,14 @@ int commit_patch_id(struct commit *commit, struct diff_options *options,
2828
/*
2929
* When we cannot load the full patch-id for both commits for whatever
3030
* reason, the function returns -1 (i.e. return error(...)). Despite
31-
* the "cmp" in the name of this function, the caller only cares about
31+
* the "neq" in the name of this function, the caller only cares about
3232
* the return value being zero (a and b are equivalent) or non-zero (a
3333
* and b are different), and returning non-zero would keep both in the
3434
* result, even if they actually were equivalent, in order to err on
3535
* the side of safety. The actual value being negative does not have
3636
* any significance; only that it is non-zero matters.
3737
*/
38-
static int patch_id_cmp(const void *cmpfn_data,
38+
static int patch_id_neq(const void *cmpfn_data,
3939
const void *entry,
4040
const void *entry_or_key,
4141
const void *unused_keydata)
@@ -53,7 +53,7 @@ static int patch_id_cmp(const void *cmpfn_data,
5353
commit_patch_id(b->commit, opt, &b->patch_id, 0))
5454
return error("Could not get patch ID for %s",
5555
oid_to_hex(&b->commit->object.oid));
56-
return oidcmp(&a->patch_id, &b->patch_id);
56+
return !oideq(&a->patch_id, &b->patch_id);
5757
}
5858

5959
int init_patch_ids(struct patch_ids *ids)
@@ -63,7 +63,7 @@ int init_patch_ids(struct patch_ids *ids)
6363
ids->diffopts.detect_rename = 0;
6464
ids->diffopts.flags.recursive = 1;
6565
diff_setup_done(&ids->diffopts);
66-
hashmap_init(&ids->patches, patch_id_cmp, &ids->diffopts, 256);
66+
hashmap_init(&ids->patches, patch_id_neq, &ids->diffopts, 256);
6767
return 0;
6868
}
6969

0 commit comments

Comments
 (0)
Please sign in to comment.