Skip to content

Commit 45843d8

Browse files
pks-tgitster
authored andcommitted
commit-reach: use size_t to track indices in remove_redundant()
The function `remove_redundant()` gets as input an array of commits as well as the size of that array and then drops redundant commits from that array. It then returns either `-1` in case an error occurred, or the new number of items in the array. The function receives and returns these sizes with a signed integer, which causes several warnings with -Wsign-compare. Fix this issue by consistently using `size_t` to track array indices and splitting up the returned value into a returned error code and a separate out pointer for the new computed size. Note that `get_merge_bases_many()` and related functions still track array sizes as a signed integer. This will be fixed in a subsequent commit. Signed-off-by: Patrick Steinhardt <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 04aeeea commit 45843d8

File tree

1 file changed

+30
-23
lines changed

1 file changed

+30
-23
lines changed

commit-reach.c

Lines changed: 30 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -212,12 +212,13 @@ int get_octopus_merge_bases(struct commit_list *in, struct commit_list **result)
212212
}
213213

214214
static int remove_redundant_no_gen(struct repository *r,
215-
struct commit **array, int cnt)
215+
struct commit **array,
216+
size_t cnt, size_t *dedup_cnt)
216217
{
217218
struct commit **work;
218219
unsigned char *redundant;
219-
int *filled_index;
220-
int i, j, filled;
220+
size_t *filled_index;
221+
size_t i, j, filled;
221222

222223
CALLOC_ARRAY(work, cnt);
223224
redundant = xcalloc(cnt, 1);
@@ -267,20 +268,22 @@ static int remove_redundant_no_gen(struct repository *r,
267268
for (i = filled = 0; i < cnt; i++)
268269
if (!redundant[i])
269270
array[filled++] = work[i];
271+
*dedup_cnt = filled;
270272
free(work);
271273
free(redundant);
272274
free(filled_index);
273-
return filled;
275+
return 0;
274276
}
275277

276278
static int remove_redundant_with_gen(struct repository *r,
277-
struct commit **array, int cnt)
279+
struct commit **array, size_t cnt,
280+
size_t *dedup_cnt)
278281
{
279-
int i, count_non_stale = 0, count_still_independent = cnt;
282+
size_t i, count_non_stale = 0, count_still_independent = cnt;
280283
timestamp_t min_generation = GENERATION_NUMBER_INFINITY;
281284
struct commit **walk_start, **sorted;
282285
size_t walk_start_nr = 0, walk_start_alloc = cnt;
283-
int min_gen_pos = 0;
286+
size_t min_gen_pos = 0;
284287

285288
/*
286289
* Sort the input by generation number, ascending. This allows
@@ -326,12 +329,12 @@ static int remove_redundant_with_gen(struct repository *r,
326329
* terminate early. Otherwise, we will do the same amount of work
327330
* as before.
328331
*/
329-
for (i = walk_start_nr - 1; i >= 0 && count_still_independent > 1; i--) {
332+
for (i = walk_start_nr; i && count_still_independent > 1; i--) {
330333
/* push the STALE bits up to min generation */
331334
struct commit_list *stack = NULL;
332335

333-
commit_list_insert(walk_start[i], &stack);
334-
walk_start[i]->object.flags |= STALE;
336+
commit_list_insert(walk_start[i - 1], &stack);
337+
walk_start[i - 1]->object.flags |= STALE;
335338

336339
while (stack) {
337340
struct commit_list *parents;
@@ -388,10 +391,12 @@ static int remove_redundant_with_gen(struct repository *r,
388391
clear_commit_marks_many(walk_start_nr, walk_start, STALE);
389392
free(walk_start);
390393

391-
return count_non_stale;
394+
*dedup_cnt = count_non_stale;
395+
return 0;
392396
}
393397

394-
static int remove_redundant(struct repository *r, struct commit **array, int cnt)
398+
static int remove_redundant(struct repository *r, struct commit **array,
399+
size_t cnt, size_t *dedup_cnt)
395400
{
396401
/*
397402
* Some commit in the array may be an ancestor of
@@ -401,19 +406,17 @@ static int remove_redundant(struct repository *r, struct commit **array, int cnt
401406
* that number.
402407
*/
403408
if (generation_numbers_enabled(r)) {
404-
int i;
405-
406409
/*
407410
* If we have a single commit with finite generation
408411
* number, then the _with_gen algorithm is preferred.
409412
*/
410-
for (i = 0; i < cnt; i++) {
413+
for (size_t i = 0; i < cnt; i++) {
411414
if (commit_graph_generation(array[i]) < GENERATION_NUMBER_INFINITY)
412-
return remove_redundant_with_gen(r, array, cnt);
415+
return remove_redundant_with_gen(r, array, cnt, dedup_cnt);
413416
}
414417
}
415418

416-
return remove_redundant_no_gen(r, array, cnt);
419+
return remove_redundant_no_gen(r, array, cnt, dedup_cnt);
417420
}
418421

419422
static int get_merge_bases_many_0(struct repository *r,
@@ -425,7 +428,8 @@ static int get_merge_bases_many_0(struct repository *r,
425428
{
426429
struct commit_list *list;
427430
struct commit **rslt;
428-
int cnt, i;
431+
size_t cnt, i;
432+
int ret;
429433

430434
if (merge_bases_many(r, one, n, twos, result) < 0)
431435
return -1;
@@ -452,8 +456,8 @@ static int get_merge_bases_many_0(struct repository *r,
452456
clear_commit_marks(one, all_flags);
453457
clear_commit_marks_many(n, twos, all_flags);
454458

455-
cnt = remove_redundant(r, rslt, cnt);
456-
if (cnt < 0) {
459+
ret = remove_redundant(r, rslt, cnt, &cnt);
460+
if (ret < 0) {
457461
free(rslt);
458462
return -1;
459463
}
@@ -582,7 +586,8 @@ struct commit_list *reduce_heads(struct commit_list *heads)
582586
struct commit_list *p;
583587
struct commit_list *result = NULL, **tail = &result;
584588
struct commit **array;
585-
int num_head, i;
589+
size_t num_head, i;
590+
int ret;
586591

587592
if (!heads)
588593
return NULL;
@@ -603,11 +608,13 @@ struct commit_list *reduce_heads(struct commit_list *heads)
603608
p->item->object.flags &= ~STALE;
604609
}
605610
}
606-
num_head = remove_redundant(the_repository, array, num_head);
607-
if (num_head < 0) {
611+
612+
ret = remove_redundant(the_repository, array, num_head, &num_head);
613+
if (ret < 0) {
608614
free(array);
609615
return NULL;
610616
}
617+
611618
for (i = 0; i < num_head; i++)
612619
tail = &commit_list_insert(array[i], tail)->next;
613620
free(array);

0 commit comments

Comments
 (0)