reftable/basics: adjust `hash_size()` to return `uint32_t`

The `hash_size()` function returns the number of bytes used by the hash function. Weirdly enough though, it returns a signed integer for its size even though the size obviously cannot ever be negative. The only case where it could be negative is if the function returned an error when asked for an unknown hash, but we assert(3p) instead. Adjust the type of `hash_size()` to be `uint32_t` and adapt all places that use signed integers for the hash size to follow suit. This also allows us to get rid of a couple asserts that we had which verified that the size was indeed positive, which further stresses the point that this refactoring makes sense. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Jan 20, 2025 at 17:17 UTC 57adf71b93efa9f9b4db5147e9fa1235f0a1d5ba
9 files changed +44 -50
reftable/basics.c
+1 -1
@@ -272,7 +272,7 @@ size_t common_prefix_size(struct reftable_buf *a, struct reftable_buf *b)
272 return p;
273 }
274
275 -int hash_size(enum reftable_hash id)
275 +uint32_t hash_size(enum reftable_hash id)
276 {
277 if (!id)
278 return REFTABLE_HASH_SIZE_SHA1;
reftable/basics.h
+1 -1
@@ -171,7 +171,7 @@ static inline void *reftable_alloc_grow(void *p, size_t nelem, size_t elsize,
171 /* Find the longest shared prefix size of `a` and `b` */
172 size_t common_prefix_size(struct reftable_buf *a, struct reftable_buf *b);
173
174 -int hash_size(enum reftable_hash id);
174 +uint32_t hash_size(enum reftable_hash id);
175
176 /*
177 * Format IDs that identify the hash function used by a reftable. Note that
reftable/block.c
+2 -2
@@ -72,7 +72,7 @@ static int block_writer_register_restart(struct block_writer *w, int n,
72 }
73
74 int block_writer_init(struct block_writer *bw, uint8_t typ, uint8_t *block,
75 - uint32_t block_size, uint32_t header_off, int hash_size)
75 + uint32_t block_size, uint32_t header_off, uint32_t hash_size)
76 {
77 bw->block = block;
78 bw->hash_size = hash_size;
@@ -214,7 +214,7 @@ int block_writer_finish(struct block_writer *w)
214
215 int block_reader_init(struct block_reader *br, struct reftable_block *block,
216 uint32_t header_off, uint32_t table_block_size,
217 - int hash_size)
217 + uint32_t hash_size)
218 {
219 uint32_t full_block_size = table_block_size;
220 uint8_t typ = block->data[header_off];
reftable/block.h
+5 -5
@@ -30,7 +30,7 @@ struct block_writer {
30
31 /* How often to restart keys. */
32 uint16_t restart_interval;
33 - int hash_size;
33 + uint32_t hash_size;
34
35 /* Offset of next uint8_t to write. */
36 uint32_t next;
@@ -48,7 +48,7 @@ struct block_writer {
48 * initializes the blockwriter to write `typ` entries, using `block` as temporary
49 * storage. `block` is not owned by the block_writer. */
50 int block_writer_init(struct block_writer *bw, uint8_t typ, uint8_t *block,
51 - uint32_t block_size, uint32_t header_off, int hash_size);
51 + uint32_t block_size, uint32_t header_off, uint32_t hash_size);
52
53 /* returns the block type (eg. 'r' for ref records. */
54 uint8_t block_writer_type(struct block_writer *bw);
@@ -72,7 +72,7 @@ struct block_reader {
72
73 /* the memory block */
74 struct reftable_block block;
75 - int hash_size;
75 + uint32_t hash_size;
76
77 /* Uncompressed data for log entries. */
78 z_stream *zstream;
@@ -92,7 +92,7 @@ struct block_reader {
92 /* initializes a block reader. */
93 int block_reader_init(struct block_reader *br, struct reftable_block *bl,
94 uint32_t header_off, uint32_t table_block_size,
95 - int hash_size);
95 + uint32_t hash_size);
96
97 void block_reader_release(struct block_reader *br);
98
@@ -108,7 +108,7 @@ struct block_iter {
108 uint32_t next_off;
109 const unsigned char *block;
110 size_t block_len;
111 - int hash_size;
111 + uint32_t hash_size;
112
113 /* key for last entry we read. */
114 struct reftable_buf last_key;
reftable/reader.c
+1 -1
@@ -750,7 +750,7 @@ static int reftable_reader_refs_for_unindexed(struct reftable_reader *r,
750 struct table_iter *ti;
751 struct filtering_ref_iterator *filter = NULL;
752 struct filtering_ref_iterator empty = FILTERING_REF_ITERATOR_INIT;
753 - int oid_len = hash_size(r->hash_id);
753 + uint32_t oid_len = hash_size(r->hash_id);
754 int err;
755
756 REFTABLE_ALLOC_ARRAY(ti, 1);
reftable/record.c
+23 -29
@@ -229,7 +229,7 @@ static int reftable_ref_record_key(const void *r, struct reftable_buf *dest)
229 }
230
231 static int reftable_ref_record_copy_from(void *rec, const void *src_rec,
232 - int hash_size)
232 + uint32_t hash_size)
233 {
234 struct reftable_ref_record *ref = rec;
235 const struct reftable_ref_record *src = src_rec;
@@ -237,8 +237,6 @@ static int reftable_ref_record_copy_from(void *rec, const void *src_rec,
237 size_t refname_cap = 0;
238 int err;
239
240 - assert(hash_size > 0);
241 -
240 SWAP(refname, ref->refname);
241 SWAP(refname_cap, ref->refname_cap);
242 reftable_ref_record_release(ref);
@@ -319,13 +317,12 @@ static uint8_t reftable_ref_record_val_type(const void *rec)
317 }
318
319 static int reftable_ref_record_encode(const void *rec, struct string_view s,
322 - int hash_size)
320 + uint32_t hash_size)
321 {
322 const struct reftable_ref_record *r =
323 (const struct reftable_ref_record *)rec;
324 struct string_view start = s;
325 int n = put_var_int(&s, r->update_index);
328 - assert(hash_size > 0);
326 if (n < 0)
327 return -1;
328 string_view_consume(&s, n);
@@ -365,7 +362,7 @@ static int reftable_ref_record_encode(const void *rec, struct string_view s,
362
363 static int reftable_ref_record_decode(void *rec, struct reftable_buf key,
364 uint8_t val_type, struct string_view in,
368 - int hash_size, struct reftable_buf *scratch)
365 + uint32_t hash_size, struct reftable_buf *scratch)
366 {
367 struct reftable_ref_record *r = rec;
368 struct string_view start = in;
@@ -374,8 +371,6 @@ static int reftable_ref_record_decode(void *rec, struct reftable_buf key,
371 size_t refname_cap = 0;
372 int n, err;
373
377 - assert(hash_size > 0);
378 -
374 n = get_var_int(&update_index, &in);
375 if (n < 0)
376 return n;
@@ -451,7 +446,7 @@ static int reftable_ref_record_is_deletion_void(const void *p)
446 }
447
448 static int reftable_ref_record_equal_void(const void *a,
454 - const void *b, int hash_size)
449 + const void *b, uint32_t hash_size)
450 {
451 struct reftable_ref_record *ra = (struct reftable_ref_record *) a;
452 struct reftable_ref_record *rb = (struct reftable_ref_record *) b;
@@ -495,7 +490,7 @@ static void reftable_obj_record_release(void *rec)
490 }
491
492 static int reftable_obj_record_copy_from(void *rec, const void *src_rec,
498 - int hash_size UNUSED)
493 + uint32_t hash_size UNUSED)
494 {
495 struct reftable_obj_record *obj = rec;
496 const struct reftable_obj_record *src = src_rec;
@@ -527,7 +522,7 @@ static uint8_t reftable_obj_record_val_type(const void *rec)
522 }
523
524 static int reftable_obj_record_encode(const void *rec, struct string_view s,
530 - int hash_size UNUSED)
525 + uint32_t hash_size UNUSED)
526 {
527 const struct reftable_obj_record *r = rec;
528 struct string_view start = s;
@@ -562,7 +557,7 @@ static int reftable_obj_record_encode(const void *rec, struct string_view s,
557
558 static int reftable_obj_record_decode(void *rec, struct reftable_buf key,
559 uint8_t val_type, struct string_view in,
565 - int hash_size UNUSED,
560 + uint32_t hash_size UNUSED,
561 struct reftable_buf *scratch UNUSED)
562 {
563 struct string_view start = in;
@@ -626,7 +621,7 @@ static int not_a_deletion(const void *p UNUSED)
621 }
622
623 static int reftable_obj_record_equal_void(const void *a, const void *b,
629 - int hash_size UNUSED)
624 + uint32_t hash_size UNUSED)
625 {
626 struct reftable_obj_record *ra = (struct reftable_obj_record *) a;
627 struct reftable_obj_record *rb = (struct reftable_obj_record *) b;
@@ -701,7 +696,7 @@ static int reftable_log_record_key(const void *r, struct reftable_buf *dest)
696 }
697
698 static int reftable_log_record_copy_from(void *rec, const void *src_rec,
704 - int hash_size)
699 + uint32_t hash_size)
700 {
701 struct reftable_log_record *dst = rec;
702 const struct reftable_log_record *src =
@@ -782,7 +777,7 @@ static uint8_t reftable_log_record_val_type(const void *rec)
777 }
778
779 static int reftable_log_record_encode(const void *rec, struct string_view s,
785 - int hash_size)
780 + uint32_t hash_size)
781 {
782 const struct reftable_log_record *r = rec;
783 struct string_view start = s;
@@ -830,7 +825,7 @@ static int reftable_log_record_encode(const void *rec, struct string_view s,
825
826 static int reftable_log_record_decode(void *rec, struct reftable_buf key,
827 uint8_t val_type, struct string_view in,
833 - int hash_size, struct reftable_buf *scratch)
828 + uint32_t hash_size, struct reftable_buf *scratch)
829 {
830 struct string_view start = in;
831 struct reftable_log_record *r = rec;
@@ -978,7 +973,7 @@ static int null_streq(const char *a, const char *b)
973 }
974
975 static int reftable_log_record_equal_void(const void *a,
981 - const void *b, int hash_size)
976 + const void *b, uint32_t hash_size)
977 {
978 return reftable_log_record_equal((struct reftable_log_record *) a,
979 (struct reftable_log_record *) b,
@@ -1002,7 +997,7 @@ static int reftable_log_record_cmp_void(const void *_a, const void *_b)
997 }
998
999 int reftable_log_record_equal(const struct reftable_log_record *a,
1005 - const struct reftable_log_record *b, int hash_size)
1000 + const struct reftable_log_record *b, uint32_t hash_size)
1001 {
1002 if (!(null_streq(a->refname, b->refname) &&
1003 a->update_index == b->update_index &&
@@ -1056,7 +1051,7 @@ static int reftable_index_record_key(const void *r, struct reftable_buf *dest)
1051 }
1052
1053 static int reftable_index_record_copy_from(void *rec, const void *src_rec,
1059 - int hash_size UNUSED)
1054 + uint32_t hash_size UNUSED)
1055 {
1056 struct reftable_index_record *dst = rec;
1057 const struct reftable_index_record *src = src_rec;
@@ -1083,7 +1078,7 @@ static uint8_t reftable_index_record_val_type(const void *rec UNUSED)
1078 }
1079
1080 static int reftable_index_record_encode(const void *rec, struct string_view out,
1086 - int hash_size UNUSED)
1081 + uint32_t hash_size UNUSED)
1082 {
1083 const struct reftable_index_record *r =
1084 (const struct reftable_index_record *)rec;
@@ -1101,7 +1096,7 @@ static int reftable_index_record_encode(const void *rec, struct string_view out,
1096 static int reftable_index_record_decode(void *rec, struct reftable_buf key,
1097 uint8_t val_type UNUSED,
1098 struct string_view in,
1104 - int hash_size UNUSED,
1099 + uint32_t hash_size UNUSED,
1100 struct reftable_buf *scratch UNUSED)
1101 {
1102 struct string_view start = in;
@@ -1122,7 +1117,7 @@ static int reftable_index_record_decode(void *rec, struct reftable_buf key,
1117 }
1118
1119 static int reftable_index_record_equal(const void *a, const void *b,
1125 - int hash_size UNUSED)
1120 + uint32_t hash_size UNUSED)
1121 {
1122 struct reftable_index_record *ia = (struct reftable_index_record *) a;
1123 struct reftable_index_record *ib = (struct reftable_index_record *) b;
@@ -1156,14 +1151,14 @@ int reftable_record_key(struct reftable_record *rec, struct reftable_buf *dest)
1151 }
1152
1153 int reftable_record_encode(struct reftable_record *rec, struct string_view dest,
1159 - int hash_size)
1154 + uint32_t hash_size)
1155 {
1156 return reftable_record_vtable(rec)->encode(reftable_record_data(rec),
1157 dest, hash_size);
1158 }
1159
1160 int reftable_record_copy_from(struct reftable_record *rec,
1166 - struct reftable_record *src, int hash_size)
1161 + struct reftable_record *src, uint32_t hash_size)
1162 {
1163 assert(src->type == rec->type);
1164
@@ -1178,7 +1173,7 @@ uint8_t reftable_record_val_type(struct reftable_record *rec)
1173 }
1174
1175 int reftable_record_decode(struct reftable_record *rec, struct reftable_buf key,
1181 - uint8_t extra, struct string_view src, int hash_size,
1176 + uint8_t extra, struct string_view src, uint32_t hash_size,
1177 struct reftable_buf *scratch)
1178 {
1179 return reftable_record_vtable(rec)->decode(reftable_record_data(rec),
@@ -1205,7 +1200,7 @@ int reftable_record_cmp(struct reftable_record *a, struct reftable_record *b)
1200 reftable_record_data(a), reftable_record_data(b));
1201 }
1202
1208 -int reftable_record_equal(struct reftable_record *a, struct reftable_record *b, int hash_size)
1203 +int reftable_record_equal(struct reftable_record *a, struct reftable_record *b, uint32_t hash_size)
1204 {
1205 if (a->type != b->type)
1206 return 0;
@@ -1213,7 +1208,7 @@ int reftable_record_equal(struct reftable_record *a, struct reftable_record *b,
1208 reftable_record_data(a), reftable_record_data(b), hash_size);
1209 }
1210
1216 -static int hash_equal(const unsigned char *a, const unsigned char *b, int hash_size)
1211 +static int hash_equal(const unsigned char *a, const unsigned char *b, uint32_t hash_size)
1212 {
1213 if (a && b)
1214 return !memcmp(a, b, hash_size);
@@ -1222,9 +1217,8 @@ static int hash_equal(const unsigned char *a, const unsigned char *b, int hash_s
1217 }
1218
1219 int reftable_ref_record_equal(const struct reftable_ref_record *a,
1225 - const struct reftable_ref_record *b, int hash_size)
1220 + const struct reftable_ref_record *b, uint32_t hash_size)
1221 {
1227 - assert(hash_size > 0);
1222 if (!null_streq(a->refname, b->refname))
1223 return 0;
1224
reftable/record.h
+8 -8
@@ -47,18 +47,18 @@ struct reftable_record_vtable {
47 /* The record type of ('r' for ref). */
48 uint8_t type;
49
50 - int (*copy_from)(void *dest, const void *src, int hash_size);
50 + int (*copy_from)(void *dest, const void *src, uint32_t hash_size);
51
52 /* a value of [0..7], indicating record subvariants (eg. ref vs. symref
53 * vs ref deletion) */
54 uint8_t (*val_type)(const void *rec);
55
56 /* encodes rec into dest, returning how much space was used. */
57 - int (*encode)(const void *rec, struct string_view dest, int hash_size);
57 + int (*encode)(const void *rec, struct string_view dest, uint32_t hash_size);
58
59 /* decode data from `src` into the record. */
60 int (*decode)(void *rec, struct reftable_buf key, uint8_t extra,
61 - struct string_view src, int hash_size,
61 + struct string_view src, uint32_t hash_size,
62 struct reftable_buf *scratch);
63
64 /* deallocate and null the record. */
@@ -68,7 +68,7 @@ struct reftable_record_vtable {
68 int (*is_deletion)(const void *rec);
69
70 /* Are two records equal? This assumes they have the same type. Returns 0 for non-equal. */
71 - int (*equal)(const void *a, const void *b, int hash_size);
71 + int (*equal)(const void *a, const void *b, uint32_t hash_size);
72
73 /*
74 * Compare keys of two records with each other. The records must have
@@ -135,16 +135,16 @@ void reftable_record_init(struct reftable_record *rec, uint8_t typ);
135
136 /* see struct record_vtable */
137 int reftable_record_cmp(struct reftable_record *a, struct reftable_record *b);
138 -int reftable_record_equal(struct reftable_record *a, struct reftable_record *b, int hash_size);
138 +int reftable_record_equal(struct reftable_record *a, struct reftable_record *b, uint32_t hash_size);
139 int reftable_record_key(struct reftable_record *rec, struct reftable_buf *dest);
140 int reftable_record_copy_from(struct reftable_record *rec,
141 - struct reftable_record *src, int hash_size);
141 + struct reftable_record *src, uint32_t hash_size);
142 uint8_t reftable_record_val_type(struct reftable_record *rec);
143 int reftable_record_encode(struct reftable_record *rec, struct string_view dest,
144 - int hash_size);
144 + uint32_t hash_size);
145 int reftable_record_decode(struct reftable_record *rec, struct reftable_buf key,
146 uint8_t extra, struct string_view src,
147 - int hash_size, struct reftable_buf *scratch);
147 + uint32_t hash_size, struct reftable_buf *scratch);
148 int reftable_record_is_deletion(struct reftable_record *rec);
149
150 static inline uint8_t reftable_record_type(struct reftable_record *rec)
reftable/reftable-record.h
+2 -2
@@ -65,7 +65,7 @@ void reftable_ref_record_release(struct reftable_ref_record *ref);
65
66 /* returns whether two reftable_ref_records are the same. Useful for testing. */
67 int reftable_ref_record_equal(const struct reftable_ref_record *a,
68 - const struct reftable_ref_record *b, int hash_size);
68 + const struct reftable_ref_record *b, uint32_t hash_size);
69
70 /* reftable_log_record holds a reflog entry */
71 struct reftable_log_record {
@@ -105,6 +105,6 @@ void reftable_log_record_release(struct reftable_log_record *log);
105
106 /* returns whether two records are equal. Useful for testing. */
107 int reftable_log_record_equal(const struct reftable_log_record *a,
108 - const struct reftable_log_record *b, int hash_size);
108 + const struct reftable_log_record *b, uint32_t hash_size);
109
110 #endif
t/unit-tests/t-reftable-record.c
+1 -1
@@ -76,7 +76,7 @@ static void t_varint_overflow(void)
76
77 static void set_hash(uint8_t *h, int j)
78 {
79 - for (int i = 0; i < hash_size(REFTABLE_HASH_SHA1); i++)
79 + for (size_t i = 0; i < hash_size(REFTABLE_HASH_SHA1); i++)
80 h[i] = (j >> i) & 0xff;
81 }
82