- Fix the fd leak in error path.
OBS-URL: https://build.opensuse.org/package/show/Base:System/coreutils?expand=0&rev=133
This commit is contained in:
Philipp Thomas
Git OBS Bridge
parent
2791e9e3ec
commit
98b9b66740
@ -10,18 +10,18 @@ Move some declarations "down" to point of initialization.
|
||||
src/tac.c | 17 +++++++----------
|
||||
1 files changed, 7 insertions(+), 10 deletions(-)
|
||||
|
||||
diff --git a/src/tac.c b/src/tac.c
|
||||
index 65ac6a6..c572862 100644
|
||||
--- src/tac.c
|
||||
+++ src/tac.c
|
||||
@@ -426,20 +426,17 @@ copy_to_temp (FILE **g_tmp, char **g_tempfile, int input_fd, char const *file)
|
||||
Index: src/tac.c
|
||||
===================================================================
|
||||
--- src/tac.c.orig 2011-02-19 18:17:03.000000000 +0100
|
||||
+++ src/tac.c 2011-10-17 15:46:27.879485098 +0200
|
||||
@@ -426,20 +426,17 @@ copy_to_temp (FILE **g_tmp, char **g_tem
|
||||
{
|
||||
static char *template = NULL;
|
||||
static char const *tempdir;
|
||||
- char *tempfile;
|
||||
- FILE *tmp;
|
||||
- int fd;
|
||||
|
||||
|
||||
if (template == NULL)
|
||||
{
|
||||
- char const * const Template = "%s/tacXXXXXX";
|
||||
@ -29,7 +29,7 @@ index 65ac6a6..c572862 100644
|
||||
tempdir = getenv ("TMPDIR");
|
||||
if (tempdir == NULL)
|
||||
tempdir = DEFAULT_TMPDIR;
|
||||
|
||||
|
||||
- /* Subtract 2 for `%s' and add 1 for the trailing NUL byte. */
|
||||
- template = xmalloc (strlen (tempdir) + strlen (Template) - 2 + 1);
|
||||
- sprintf (template, Template, tempdir);
|
||||
@ -37,63 +37,14 @@ index 65ac6a6..c572862 100644
|
||||
+ template = xmalloc (strlen (tempdir) + strlen (Template) + 1 + 1);
|
||||
+ stpcpy (stpcpy (stpcpy (template, tempdir), "/"), Template);
|
||||
}
|
||||
|
||||
|
||||
/* FIXME: there's a small window between a successful mkstemp call
|
||||
@@ -451,8 +448,8 @@ copy_to_temp (FILE **g_tmp, char **g_tempfile, int input_fd, char const *file)
|
||||
@@ -451,21 +448,23 @@ copy_to_temp (FILE **g_tmp, char **g_tem
|
||||
FIXME: clean up upon fatal signal. Don't block them, in case
|
||||
$TMPFILE is a remote file system. */
|
||||
|
||||
|
||||
- tempfile = template;
|
||||
- fd = mkstemp (template);
|
||||
+ char *tempfile = template;
|
||||
+ int fd = mkstemp (template);
|
||||
if (fd < 0)
|
||||
{
|
||||
error (0, errno, _("cannot create temporary file in %s"),
|
||||
@@ -460,7 +457,7 @@ copy_to_temp (FILE **g_tmp, char **g_tempfile, int input_fd, char const *file)
|
||||
return false;
|
||||
}
|
||||
|
||||
- tmp = fdopen (fd, (O_BINARY ? "w+b" : "w+"));
|
||||
+ FILE *tmp = fdopen (fd, (O_BINARY ? "w+b" : "w+"));
|
||||
if (! tmp)
|
||||
{
|
||||
error (0, errno, _("cannot open %s for writing"), quote (tempfile));
|
||||
--
|
||||
1.7.7
|
||||
|
||||
|
||||
>From 608f9d9d0daef48c957fe38570e5d0f293c0f1eb Mon Sep 17 00:00:00 2001
|
||||
From: Jim Meyering <meyering@redhat.com>
|
||||
Date: Sun, 16 Oct 2011 12:07:05 +0200
|
||||
Subject: [PATCH 2/3] tac: don't misbehave with multiple non-seekable inputs
|
||||
|
||||
* src/tac.c (copy_to_temp): Do not reuse the template buffer.
|
||||
Instead, scribble only on a freshly-xstrdup'd copy each time.
|
||||
Free that buffer both here, upon failure, and ...
|
||||
(tac_nonseekable): ...free the buffer in caller, upon success.
|
||||
* tests/misc/tac-2-nonseekable: New file.
|
||||
* tests/Makefile.am (TESTS): Add it.
|
||||
* NEWS (Bug fixes): Mention it.
|
||||
Reported by Ambrose Feinstein in http://debbugs.gnu.org/9762.
|
||||
---
|
||||
NEWS | 5 +++++
|
||||
src/tac.c | 19 +++++++++++++++----
|
||||
tests/Makefile.am | 1 +
|
||||
tests/misc/tac-2-nonseekable | 27 +++++++++++++++++++++++++++
|
||||
4 files changed, 48 insertions(+), 4 deletions(-)
|
||||
create mode 100755 tests/misc/tac-2-nonseekable
|
||||
|
||||
diff --git a/src/tac.c b/src/tac.c
|
||||
index c572862..2d8d6ea 100644
|
||||
--- src/tac.c
|
||||
+++ src/tac.c
|
||||
@@ -448,12 +448,13 @@ copy_to_temp (FILE **g_tmp, char **g_tempfile, int input_fd, char const *file)
|
||||
FIXME: clean up upon fatal signal. Don't block them, in case
|
||||
$TMPFILE is a remote file system. */
|
||||
|
||||
- char *tempfile = template;
|
||||
- int fd = mkstemp (template);
|
||||
+ char *tempfile = xstrdup (template);
|
||||
+ int fd = mkstemp (tempfile);
|
||||
if (fd < 0)
|
||||
@ -103,47 +54,48 @@ index c572862..2d8d6ea 100644
|
||||
+ free (tempfile);
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -463,6 +464,7 @@ copy_to_temp (FILE **g_tmp, char **g_tempfile, int input_fd, char const *file)
|
||||
|
||||
- tmp = fdopen (fd, (O_BINARY ? "w+b" : "w+"));
|
||||
+ FILE *tmp = fdopen (fd, (O_BINARY ? "w+b" : "w+"));
|
||||
if (! tmp)
|
||||
{
|
||||
error (0, errno, _("cannot open %s for writing"), quote (tempfile));
|
||||
close (fd);
|
||||
unlink (tempfile);
|
||||
+ free (tempfile);
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -498,6 +500,7 @@ copy_to_temp (FILE **g_tmp, char **g_tempfile, int input_fd, char const *file)
|
||||
|
||||
|
||||
@@ -501,6 +500,7 @@ copy_to_temp (FILE **g_tmp, char **g_tem
|
||||
|
||||
Fail:
|
||||
fclose (tmp);
|
||||
+ free (tempfile);
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -509,8 +512,16 @@ tac_nonseekable (int input_fd, const char *file)
|
||||
|
||||
@@ -512,8 +512,14 @@ tac_nonseekable (int input_fd, const cha
|
||||
{
|
||||
FILE *tmp_stream;
|
||||
char *tmp_file;
|
||||
- return (copy_to_temp (&tmp_stream, &tmp_file, input_fd, file)
|
||||
- && tac_seekable (fileno (tmp_stream), tmp_file));
|
||||
+ if (copy_to_temp (&tmp_stream, &tmp_file, input_fd, file))
|
||||
+ {
|
||||
+ if (tac_seekable (fileno (tmp_stream), tmp_file))
|
||||
+ {
|
||||
+ free (tmp_file);
|
||||
+ return true;
|
||||
+ }
|
||||
+ }
|
||||
+
|
||||
+ return false;
|
||||
+ if (!copy_to_temp (&tmp_stream, &tmp_file, input_fd, file))
|
||||
+ return false;
|
||||
+
|
||||
+ bool ok = tac_seekable (fileno (tmp_stream), tmp_file);
|
||||
+ fclose (tmp_stream);
|
||||
+ free (tmp_file);
|
||||
+ return ok;
|
||||
}
|
||||
|
||||
|
||||
/* Print FILE in reverse, copying it to a temporary
|
||||
diff --git a/tests/Makefile.am b/tests/Makefile.am
|
||||
index 9c9a1b8..5021c18 100644
|
||||
--- tests/Makefile.am
|
||||
+++ tests/Makefile.am
|
||||
@@ -274,6 +274,7 @@ TESTS = \
|
||||
Index: tests/Makefile.am
|
||||
===================================================================
|
||||
--- tests/Makefile.am.orig 2011-10-17 15:40:44.533154336 +0200
|
||||
+++ tests/Makefile.am 2011-10-17 15:40:44.882149592 +0200
|
||||
@@ -270,6 +270,7 @@ TESTS = \
|
||||
misc/sum-sysv \
|
||||
misc/tac \
|
||||
misc/tac-continue \
|
||||
@ -151,11 +103,10 @@ index 9c9a1b8..5021c18 100644
|
||||
misc/tail \
|
||||
misc/tee \
|
||||
misc/tee-dash \
|
||||
diff --git a/tests/misc/tac-2-nonseekable b/tests/misc/tac-2-nonseekable
|
||||
new file mode 100755
|
||||
index 0000000..7b48773
|
||||
--- /dev/null
|
||||
+++ tests/misc/tac-2-nonseekable
|
||||
Index: tests/misc/tac-2-nonseekable
|
||||
===================================================================
|
||||
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
|
||||
+++ tests/misc/tac-2-nonseekable 2011-10-17 15:40:44.883149578 +0200
|
||||
@@ -0,0 +1,27 @@
|
||||
+#!/bin/sh
|
||||
+# ensure that tac works with two or more non-seekable inputs
|
||||
@ -184,40 +135,3 @@ index 0000000..7b48773
|
||||
+compare err /dev/null || fail=1
|
||||
+
|
||||
+Exit $fail
|
||||
--
|
||||
1.7.7
|
||||
|
||||
|
||||
>From 95fa11b63f0a5c983723f02afa7c5b896e5a0a97 Mon Sep 17 00:00:00 2001
|
||||
From: Jim Meyering <meyering@redhat.com>
|
||||
Date: Sun, 16 Oct 2011 12:14:05 +0200
|
||||
Subject: [PATCH 3/3] tac: don't leak a file descriptor for each non-seekable
|
||||
input
|
||||
|
||||
* src/tac.c (tac_nonseekable): Call fclose after each successful
|
||||
call to copy_to_temp.
|
||||
---
|
||||
src/tac.c | 2 ++
|
||||
1 files changed, 2 insertions(+), 0 deletions(-)
|
||||
|
||||
diff --git a/src/tac.c b/src/tac.c
|
||||
index 2d8d6ea..ebafa18 100644
|
||||
--- src/tac.c
|
||||
+++ src/tac.c
|
||||
@@ -516,9 +516,11 @@ tac_nonseekable (int input_fd, const char *file)
|
||||
{
|
||||
if (tac_seekable (fileno (tmp_stream), tmp_file))
|
||||
{
|
||||
+ fclose (tmp_stream);
|
||||
free (tmp_file);
|
||||
return true;
|
||||
}
|
||||
+ fclose (tmp_stream);
|
||||
}
|
||||
|
||||
return false;
|
||||
--
|
||||
1.7.7
|
||||
|
||||
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user