On Thu, Jan 20 2022, Han-Wen Nienhuys via GitGitGadget wrote:
> From: Han-Wen Nienhuys <hanwen@xxxxxxxxxx>
>
> If the cleanup fails, there is nothing we can do.
>
> Signed-off-by: Han-Wen Nienhuys <hanwen@xxxxxxxxxx>
> ---
> reftable/stack_test.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/reftable/stack_test.c b/reftable/stack_test.c
> index e84f50d27ff..19fe4e20085 100644
> --- a/reftable/stack_test.c
> +++ b/reftable/stack_test.c
> @@ -90,7 +90,7 @@ static void test_read_file(void)
> EXPECT(0 == strcmp(want[i], names[i]));
> }
> free_names(names);
> - remove(fn);
> + (void) remove(fn);
> }
>
> static void test_parse_names(void)
Well, if we fail here due to a permission error or other I/O weirdness
surely it's better to:
if (remove(fn) < 0)
die_errno("unable to remove '%s'", fn);
Otherwise we're just silently sweeping that under the rug, and likely
having the "rm -rf" we'll shortly do in test-lib.sh catch it at a
distance.
Also why are we using remove() here at all? Shouldn't this just be
unlink()? Or per the feedback above unlink_or_warn() or
remove_or_warn()? I.e. looking at the context we just open()'d this
"fn", so we're not unsure if it's a directory or a file, are we?
