On Fri, Apr 9, 2010 at 11:19 AM, rre...@iol.it <rre...@iol.it> wrote:
> Hi,
>
> there seems to be a bug in the function parse_samefile() in find/parser.c...
> if I am not mistaken, after the call to collect_arg_stat_info(),
> argv[*arg_ptr] becomes NULL. I enclose a little patch to fix this problem.
>
Thanks for spotting that! I've applied your fix as the attached
patch. The ChangeLog entry says "(tiny change)" not because your
work lacks merit, but simply to indicate that it is not necessary for
you to perform a copyright assignment to the FSF in order for me to
apply your patch.
Thanks for your help!
James.
From 54723137491054781667e56533e2e0eb5186964b Mon Sep 17 00:00:00 2001
From: James Youngman <j...@gnu.org>
Date: Fri, 9 Apr 2010 22:20:10 +0100
Subject: [PATCH] Pass filename, rather than NULL, to open in pred_samefile.
To: findutils-patches@gnu.org
* find/parser.c (parse_samefile): Since collect_arg_stat_info
increments *arg_ptr, argv[*arg_ptr] is often NULL. Use filename
instead, as that's the variable in which we store the name of our
file. The purpose of the fd was only to attempt to keep the inode
number stable on systems that don't really have inode numbers, so
this change should have no functional effect on POSIX systems.
* find/parser.c (parse_samefile): Use open_cloexec to open the
reference file, so that we don't leak a file descriptor.
Signed-off-by: James Youngman <j...@gnu.org>
---
ChangeLog | 14 ++++++++++++++
find/parser.c | 11 ++++++-----
2 files changed, 20 insertions(+), 5 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index af42fb0..6fe36f5 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@
+2010-04-09 James Youngman <j...@gnu.org>
+
+ * find/parser.c (parse_samefile): Use open_cloexec to open the
+ reference file, so that we don't leak a file descriptor.
+
+2010-04-09 Roberto Reale <rre...@iol.it> (tiny change)
+
+ * find/parser.c (parse_samefile): Since collect_arg_stat_info
+ increments *arg_ptr, argv[*arg_ptr] is often NULL. Use filename
+ instead, as that's the variable in which we store the name of our
+ file. The purpose of the fd was only to attempt to keep the inode
+ number stable on systems that don't really have inode numbers, so
+ this change should have no functional effect on POSIX systems.
+
2010-04-08 James Youngman <j...@gnu.org>
Move on from 4.5.8.
diff --git a/find/parser.c b/find/parser.c
index f29f58b..fb6c753 100644
--- a/find/parser.c
+++ b/find/parser.c
@@ -43,6 +43,7 @@
#include "error.h"
#include "findutils-version.h"
#include "safe-atoi.h"
+#include "fdleak.h"
#include <fcntl.h>
@@ -2412,9 +2413,9 @@ parse_samefile (const struct parser_table* entry, char **argv, int *arg_ptr)
{
/* Race condition here. The file might become a
* symbolic link in between our call to stat and
- * the call to open.
+ * the call to open_cloexec.
*/
- fd = open (argv[*arg_ptr], openflags);
+ fd = open_cloexec (filename, openflags);
if (fd >= 0)
{
@@ -2423,7 +2424,7 @@ parse_samefile (const struct parser_table* entry, char **argv, int *arg_ptr)
*/
if (0 != fstat (fd, &fst))
{
- fatal_target_file_error (errno, argv[*arg_ptr]);
+ fatal_target_file_error (errno, filename);
}
else
{
@@ -2432,8 +2433,8 @@ parse_samefile (const struct parser_table* entry, char **argv, int *arg_ptr)
* open, fst may contain the stat information for the
* destination of the link, not the link itself.
*/
- if ((*options.xstat) (argv[*arg_ptr], &st))
- fatal_target_file_error (errno, argv[*arg_ptr]);
+ if ((*options.xstat) (filename, &st))
+ fatal_target_file_error (errno, filename);
if ((options.symlink_handling == SYMLINK_NEVER_DEREF)
&& (!options.open_nofollow_available))
--
1.7.0
_______________________________________________
Findutils-patches mailing list
Findutils-patches@gnu.org
http://lists.gnu.org/mailman/listinfo/findutils-patches
