Re: [PATCH v3 4/5] pack-bitmap.c: using error() instead of silently returning -1
- From
Taylor Blau <me@ttaylorr.com>
- Date
- Jun 14, 2022, 01:15 UTC
- Message-ID
- <YqfhR/PV6HhGyhT5@nand.local>
- In-Reply-To
- <72da3b584490467c2492578a8125cbcfe05aad9a.1655018322.git.dyroneteng@gmail.com>
On Sun, Jun 12, 2022 at 03:44:19PM +0800, Teng Long wrote:
Show 6 quoted lines
> @@ -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().
Show 6 quoted lines
> @@ -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?
Show 7 quoted lines
> 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