[PATCH 3/3] test-delta: close output descriptor after use
- From
Jeff King <peff@peff.net>
- Date
- Jul 24, 2025, 00:03 UTC
- Message-ID
- <20250724000308.GC597294@coredump.intra.peff.net>
- In-Reply-To
- <20250723235929.GB592873@coredump.intra.peff.net>
After we write to the output file, the program exits. This naturally closes the descriptor. But we should do an explicit close for two reasons:
1. It's possible to hit an error on close(), which we should detect
and report via our exit code. 2. Leaking descriptors is a bad practice in general. Even if it isn't
meaningful here, it sets a bad example.It is tempting to write:
if (write_in_full(fd, ...) < 0 || close(fd) < 0)
die_errno(...);But that pattern contains a subtle problem that has resulted in descriptor leaks before. If write_in_full() fails, we'll short-circuit and never call close(), leaking the descriptor.
That's not a problem here, since our error path dies instead of returning up the stack. But since we're trying to set a good example, let's write it out as two separate conditions. As a bonus, that lets us produce a slightly more specific error message.
Signed-off-by: Jeff King <peff@peff.net> --- Another option is that the program should just write to stdout, then we do not have to open or close it at all. ;)
t/helper/test-delta.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/t/helper/test-delta.c b/t/helper/test-delta.c index 7945793078..52ea00c937 100644 --- a/t/helper/test-delta.c +++ b/t/helper/test-delta.c @@ -45,6 +45,8 @@ int cmd__delta(int argc, const char **argv) fd = xopen(argv[4], O_WRONLY|O_CREAT|O_TRUNC, 0666); if (write_in_full(fd, out_buf, out_size) < 0) die_errno("write(%s)", argv[4]); + if (close(fd) < 0) + die_errno("close(%s)", argv[4]); strbuf_release(&from); strbuf_release(&data);
-- 2.50.1.666.gdb1e186d6a