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

Reply via email to