Re: [PATCH v6 05/15] reftable: ignore remove() return value in stack_test.c
From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2022-01-21 11:49:58
On Thu, Jan 20 2022, Han-Wen Nienhuys via GitGitGadget wrote:
quoted hunk ↗ jump to hunk
From: Han-Wen Nienhuys <redacted> If the cleanup fails, there is nothing we can do. Signed-off-by: Han-Wen Nienhuys <redacted> --- 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?