Re: [PATCH v3 4/5] pack-bitmap.c: using error() instead of silently returning -1

[Taylor Blau <me@xxxxxxxxxxxx>]

On Sun, Jun 12, 2022 at 03:44:19PM +0800, Teng Long wrote:
> @@ -323,7 +323,7 @@ static int open_midx_bitmap_1(struct bitmap_index *bitmap_git,
>
>  	if (fstat(fd, &st)) {
>  		close(fd);
> -		return -1;
> +		return error(_("cannot stat bitmap file"));

Since we are handling an error from fstat here, the errno variable
contains useful information that we should include in the error via
error_errno().

> @@ -361,7 +361,7 @@ static int open_midx_bitmap_1(struct bitmap_index *bitmap_git,
>  	bitmap_git->map_pos = 0;
>  	bitmap_git->map = NULL;
>  	bitmap_git->midx = NULL;
> -	return -1;
> +	return error("cannot open midx bitmap file");

The other error strings are marked for translation, but this one is not.
Was that intentional, or just a typo / oversight?

>  static int open_pack_bitmap_1(struct bitmap_index *bitmap_git, struct packed_git *packfile)
> @@ -382,7 +382,7 @@ static int open_pack_bitmap_1(struct bitmap_index *bitmap_git, struct packed_git
>
>  	if (fstat(fd, &st)) {
>  		close(fd);
> -		return -1;
> +		return error(_("cannot stat bitmap file"));

Same note here about using error_errno() instead of just error().

Thanks,
Taylor