On Wed, Dec 23, 2015 at 11:13 PM, Paul Eggert <egg...@cs.ucla.edu> wrote:
> James Youngman wrote:
>
>> +static FILE* fopen_cloexec_for_read_only (const char *file_name) {
>> + int fd = open_cloexec (file_name, O_RDONLY);
>> + return (fd < 0) ? NULL : fdopen (fd, "r");
>> +}
>
>
> This should close fd if fdopen fails, no?
Ouch, I leaked an fd in code intended to avoid that.
> Also, the indentation should be fixed to be GNU-like.
Thanks, I fixed both problems in the attached patch. This is a sign
I should stop working for the day, probably; my attention to detail
has fallen off. Thanks for spotting the problems.
James.
From d286cf67ddbad8f5827310f759f862c3fdfe550e Mon Sep 17 00:00:00 2001
From: James Youngman <j...@gnu.org>
Date: Wed, 23 Dec 2015 23:23:05 +0000
Subject: [PATCH] Avoid an fd leak in fopen_cloexec_for_read_only.
To: findutils-patc...@gnu.org
* xargs/xargs.c (fopen_cloexec_for_read_only): when fdopen fails,
close the file descriptor instead of leaking it. Also, use GNU-style
brace positioning. Both problems were noticed by Paul Eggert.
---
xargs/xargs.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/xargs/xargs.c b/xargs/xargs.c
index bf130e0..5f3e760 100644
--- a/xargs/xargs.c
+++ b/xargs/xargs.c
@@ -367,9 +367,24 @@ smaller_of (size_t a, size_t b)
}
-static FILE* fopen_cloexec_for_read_only (const char *file_name) {
+static FILE* fopen_cloexec_for_read_only (const char *file_name)
+{
int fd = open_cloexec (file_name, O_RDONLY);
- return (fd < 0) ? NULL : fdopen (fd, "r");
+ if (fd < 0)
+ {
+ return NULL;
+ }
+ else
+ {
+ FILE *result = fdopen (fd, "r");
+ if (!result)
+ {
+ int saved_errno = errno;
+ close (fd);
+ errno = saved_errno;
+ return NULL;
+ }
+ }
}
--
2.1.4
