Re: [PATCH 4/9] commit-reach: use `size_t` to track indices in `remove_redundant()`

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 24/12/27 11:46AM, Patrick Steinhardt wrote:
> 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 <ps@xxxxxx>
> ---
>  commit-reach.c | 53 ++++++++++++++++++++++++++++++-----------------------
>  1 file changed, 30 insertions(+), 23 deletions(-)
> 
> diff --git a/commit-reach.c b/commit-reach.c
> index 9f8b2457bcc12bebf725a5276d1aec467bb7af05..d7f6f1be75e95cc834d60be719e930a77ad0518f 100644
> --- a/commit-reach.c
> +++ b/commit-reach.c
> @@ -212,12 +212,13 @@ int get_octopus_merge_bases(struct commit_list *in, struct commit_list **result)
>  }
>  
>  static int remove_redundant_no_gen(struct repository *r,
> -				   struct commit **array, int cnt)
> +				   struct commit **array,
> +				   size_t cnt, size_t *dedup_cnt)
>  {
>  	struct commit **work;
>  	unsigned char *redundant;
> -	int *filled_index;
> -	int i, j, filled;
> +	size_t *filled_index;
> +	size_t i, j, filled;
>  
>  	CALLOC_ARRAY(work, cnt);
>  	redundant = xcalloc(cnt, 1);
> @@ -267,20 +268,22 @@ static int remove_redundant_no_gen(struct repository *r,
>  	for (i = filled = 0; i < cnt; i++)
>  		if (!redundant[i])
>  			array[filled++] = work[i];
> +	*dedup_cnt = filled;
>  	free(work);
>  	free(redundant);
>  	free(filled_index);
> -	return filled;
> +	return 0;

Previously the return value indicated either a potential error if its
value was negative or the new count otherwise. Since we now want to make
the count a `size_t` to avoid -Wsign-compare warnings, we split the two
concerns and have a separate pointer used to record the count. This
approach is used for each of the `remove_redundant*()` functions. Seems
sensible to me.

-Justin




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux