[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Fix preserve_mode when destination directory partially exist
From: |
Jim Meyering |
Subject: |
Re: [PATCH] Fix preserve_mode when destination directory partially exists |
Date: |
Mon, 07 Jan 2008 12:26:51 +0100 |
Jan Blunck <address@hidden> wrote:
> On Fri, Jan 04, Paul Eggert wrote:
>
>> Jan Blunck <address@hidden> writes:
>>
>> > I found a bug with cp -p --parents when the destination partially exists
>> > and
>> > the filesystem isn't mounted with acls.
>> >
>> > $ mkdir -p a/b/c a/b/d e
>> > $ touch a/b/c/foo a/b/d/foo
>> > $ cp -p --parent a/b/c e
>> > $ cp -p --parent a/b/d e
>> > $ ls -ld e/a
>> > d--------- 3 jblunck suse 4096 1970-01-01 01:00 e/a
>>
>> I can't reproduce the problem on my Debian stable host, using
>> coreutils 6.9.91:
>>
>> $ mkdir -p a/b/c a/b/d e
>> $ touch a/b/c/foo a/b/d/foo
>> $ cp -p --parent a/b/c e
>> cp: omitting directory `a/b/c'
>
> Oh, my testcase should read as follows
> $ cp -p --parent a/b/c/foo e
> $ cp -p --parent a/b/d/foo e
Ah. Now I see.
Thanks for the report and patch.
I've added a test based on the above and applied your patch:
2008-01-07 Jim Meyering <address@hidden>
* NEWS: Mention the cp bug fix.
2008-01-07 Jan Blunck <address@hidden>
cp --parents: don't use uninitialized memory when restoring permissions
* src/cp.c (make_dir_parents_private): Always stat each source
directory, in case its permissions are required in re_protect,
when setting permissions of a just-created destination directory.
2008-01-07 Jim Meyering <address@hidden>
cp: add a test for today's bug fix.
* tests/cp/parent-perm: New script. Test today's change.
Based on reproducer from Jan Blunck.
* tests/cp/Makefile.am (TESTS): Add parent-perm.
>From db58094e11adb527c31b3510d3430123a4048686 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Mon, 7 Jan 2008 11:57:27 +0100
Subject: [PATCH] cp: add a test for today's bug fix.
* tests/cp/parent-perm: New script. Test today's change.
Based on reproducer from Jan Blunck.
* tests/cp/Makefile.am (TESTS): Add parent-perm.
---
ChangeLog | 7 +++++++
tests/cp/Makefile.am | 3 ++-
tests/cp/parent-perm | 38 ++++++++++++++++++++++++++++++++++++++
3 files changed, 47 insertions(+), 1 deletions(-)
create mode 100755 tests/cp/parent-perm
diff --git a/ChangeLog b/ChangeLog
index 85dc1c1..29ae607 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2008-01-07 Jim Meyering <address@hidden>
+
+ cp: add a test for today's bug fix.
+ * tests/cp/parent-perm: New script. Test today's change.
+ Based on reproducer from Jan Blunck.
+ * tests/cp/Makefile.am (TESTS): Add parent-perm.
+
2008-01-06 Jim Meyering <address@hidden>
touch: add a test for today's change.
diff --git a/tests/cp/Makefile.am b/tests/cp/Makefile.am
index 4af269c..e2f96c9 100644
--- a/tests/cp/Makefile.am
+++ b/tests/cp/Makefile.am
@@ -1,6 +1,6 @@
# Make coreutils tests for cp. -*-Makefile-*-
-# Copyright (C) 1997-2001, 2003, 2005-2007 Free Software Foundation, Inc.
+# Copyright (C) 1997-2001, 2003, 2005-2008 Free Software Foundation, Inc.
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -16,6 +16,7 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
TESTS = \
+ parent-perm \
abuse \
proc-zero-len \
thru-dangling \
diff --git a/tests/cp/parent-perm b/tests/cp/parent-perm
new file mode 100755
index 0000000..1c7a222
--- /dev/null
+++ b/tests/cp/parent-perm
@@ -0,0 +1,38 @@
+#!/bin/sh
+# Ensure that cp --parents works properly with a preexisting dest. directory
+
+# Copyright (C) 2008 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+if test "$VERBOSE" = yes; then
+ set -x
+ cp --version
+fi
+
+. $srcdir/../envvar-check
+. $srcdir/../test-lib.sh
+
+mkdir -p a/b/c a/b/d e || framework_failure
+touch a/b/c/foo a/b/d/foo || framework_failure
+cp -p --parent a/b/c/foo e || framework_failure
+
+fail=0
+cp -p --parent a/b/d/foo e || fail=1
+
+# Ensure that permissions on just-created directory, e/a/,
+# are the same as those on original, a/.
+test $(stat --printf %A a) = $(stat --printf %A e/a) || fail=1
+
+(exit $fail); exit $fail
--
1.5.4.rc2.61.g6ed4e
>From a54e8bb8a547c2ee9147865e2eb42eece69e8072 Mon Sep 17 00:00:00 2001
From: Jan Blunck <address@hidden>
Date: Mon, 7 Jan 2008 12:13:42 +0100
Subject: [PATCH] cp --parents: don't use uninitialized memory when restoring
permissions
* src/cp.c (make_dir_parents_private): Always stat each source
directory, in case its permissions are required in re_protect,
when setting permissions of a just-created destination directory.
---
ChangeLog | 7 +++++++
src/cp.c | 24 ++++++++++++------------
2 files changed, 19 insertions(+), 12 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 29ae607..435ef43 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2008-01-07 Jan Blunck <address@hidden>
+
+ cp --parents: don't use uninitialized memory when restoring permissions
+ * src/cp.c (make_dir_parents_private): Always stat each source
+ directory, in case its permissions are required in re_protect,
+ when setting permissions of a just-created destination directory.
+
2008-01-07 Jim Meyering <address@hidden>
cp: add a test for today's bug fix.
diff --git a/src/cp.c b/src/cp.c
index 99e1f73..be3701f 100644
--- a/src/cp.c
+++ b/src/cp.c
@@ -403,6 +403,7 @@ make_dir_parents_private (char const *const_dir, size_t
src_offset,
slash++;
while ((slash = strchr (slash, '/')))
{
+ int src_errno;
/* Add this directory to the list of directories whose modes need
fixing later. */
struct dir_attr *new = xmalloc (sizeof *new);
@@ -412,12 +413,22 @@ make_dir_parents_private (char const *const_dir, size_t
src_offset,
*attr_list = new;
*slash = '\0';
+ src_errno = (stat (src, &new->st) != 0
+ ? errno
+ : S_ISDIR (new->st.st_mode)
+ ? 0
+ : ENOTDIR);
+ if (src_errno)
+ {
+ error (0, src_errno, _("failed to get attributes of %s"),
+ quote (src));
+ return false;
+ }
if (stat (dir, &stats) != 0)
{
mode_t src_mode;
mode_t omitted_permissions;
mode_t mkdir_mode;
- int src_errno;
/* This component does not exist. We must set
*new_dst and new->st.st_mode inside this loop because,
@@ -425,17 +436,6 @@ make_dir_parents_private (char const *const_dir, size_t
src_offset,
make_dir_parents_private creates only e_dir/../a if
./b already exists. */
*new_dst = true;
- src_errno = (stat (src, &new->st) != 0
- ? errno
- : S_ISDIR (new->st.st_mode)
- ? 0
- : ENOTDIR);
- if (src_errno)
- {
- error (0, src_errno, _("failed to get attributes of %s"),
- quote (src));
- return false;
- }
src_mode = new->st.st_mode;
/* If the ownership or special mode bits might change,
--
1.5.4.rc2.61.g6ed4e
>From ab1c9b54b173312c132c9452eff6080a5b4cf6e7 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Mon, 7 Jan 2008 12:17:52 +0100
Subject: [PATCH] NEWS: Mention the cp bug fix.
---
ChangeLog | 4 ++++
NEWS | 4 ++++
2 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 435ef43..676f33e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2008-01-07 Jim Meyering <address@hidden>
+
+ * NEWS: Mention the cp bug fix.
+
2008-01-07 Jan Blunck <address@hidden>
cp --parents: don't use uninitialized memory when restoring permissions
diff --git a/NEWS b/NEWS
index 542e5f2..ca3bbc8 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,10 @@ GNU coreutils NEWS -*-
outline -*-
** Bug fixes
+ cp --parents no longer uses uninitialized memory when restoring the
+ permissions of a just-created destination directory.
+ [bug introduced in coreutils-6.9.90]
+
tr's case conversion would fail in a locale with differing numbers
of lower case and upper case characters. E.g., this would fail:
env LC_CTYPE=en_US.iso88591 tr '[:upper:]' '[:lower:]'
--
1.5.4.rc2.61.g6ed4e