[PATCH 3/3] test-delta: close output descriptor after use

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxx>
---
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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux