[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: minor mkdir bug fixes
From: |
Jim Meyering |
Subject: |
Re: minor mkdir bug fixes |
Date: |
Wed, 15 Jun 2005 11:45:03 +0200 |
Thanks for all of your cleanup work and for catching the true/false bug!
Paul Eggert <address@hidden> wrote:
> I found some more glitches in the mkdir code. Occasionally multiple
> diagnostics might be generated where one would do. The diagnostics
I spotted another problem.
install -d could still create directories in the wrong place, albeit only
under contrived conditions. That was due to a typo in one of my changes.
I've just fixed that and added to the existing tests/install/basic-1
test script so it would detect the resulting failure.
> could report the error number, but didn't. In one unlikely case in
> 'install', the code would install the file into the wrong place.
If the bug is not too hard to test for, would you please create a
test case for it, so it'll be less likely to be reintroduced later?
These corner case bugs are hard enough to find.
If we don't record/document them via test cases,
we can't even hope to converge on correctness.
> Also, there's no need for make_dir_parents to invoke 'stat' each time
> through the loop. It can just rely on information gleaned from the
> other system calls. In theory even the first "stat" could go but I'm
> not sure that would improve performance on the average, since "mkdir
> -p /some/existing/file" must be a common case.
Finally, there was a new small bug in mkdir-p.c.
Patch and test case below.
2005-06-15 Jim Meyering <address@hidden>
* lib/mkdir-p.c (make_dir_parents): Don't let a failed chdir($PWD)
stop us from restricting permissions of just-created absolute-named
directories.
* tests/mkdir/p-3: Add a test for just-fixed bug in mkdir-p.c.
* src/install.c (main): Fix my typo: s/argv[optind]/file[i]/.
* tests/install/basic-1: Ensure that each `-d'-specified directory
is created. Ensure that rel-named dirs are not created when
chdir($PWD) fails.
Index: lib/mkdir-p.c
===================================================================
RCS file: /fetish/cu/lib/mkdir-p.c,v
retrieving revision 1.7
retrieving revision 1.8
diff -u -p -u -r1.7 -r1.8
--- lib/mkdir-p.c 14 Jun 2005 23:56:49 -0000 1.7
+++ lib/mkdir-p.c 15 Jun 2005 08:31:44 -0000 1.8
@@ -319,7 +319,8 @@ make_dir_parents (char const *arg,
for (; leading_dirs != NULL; leading_dirs = leading_dirs->next)
{
leading_dirs->dirname_end[0] = '\0';
- if (cwd_problem || chmod (full_dir, parent_mode) != 0)
+ if ((cwd_problem && *full_dir != '/')
+ || chmod (full_dir, parent_mode) != 0)
{
error (0, (cwd_problem ? 0 : errno),
_("cannot change permissions of %s"), quote (full_dir));
Index: tests/mkdir/p-3
===================================================================
RCS file: /fetish/cu/tests/mkdir/p-3,v
retrieving revision 1.3
retrieving revision 1.4
diff -u -p -u -r1.3 -r1.4
--- tests/mkdir/p-3 14 Jun 2005 09:00:00 -0000 1.3
+++ tests/mkdir/p-3 15 Jun 2005 08:32:11 -0000 1.4
@@ -18,6 +18,7 @@ mkdir -p $tmp || framework_failure=1
cd $tmp || framework_failure=1
mkdir no-access || framework_failure=1
mkdir no-acce2s || framework_failure=1
+mkdir no-acce3s || framework_failure=1
if test $framework_failure = 1; then
echo "$0: failure in testing framework" 1>&2
@@ -26,16 +27,25 @@ fi
p=$pwd/$tmp
(cd no-access; chmod 0 . && mkdir -p $p/a/b u/v) 2> /dev/null && fail=1
+test -d $p/a/b || fail=1
# Same as above, but with a following *absolute* name, it should succeed
-(cd no-acce2s; chmod 0 . && mkdir -p $p/a/b $p/z) || fail=1
-
-test -d $p/a/b || fail=1
-b=`ls $p/a|tr -d '\n'`
+(cd no-acce2s; chmod 0 . && mkdir -p $p/b/b $p/z) || fail=1
test -d $p/z || fail=1
+b=`ls $p/a|tr -d '\n'`
# With coreutils-5.3.0, this would fail with $b=bu.
test "x$b" = xb || fail=1
+# Ensure that the re_protect code is run on absolute names, even
+# after failure to return to the initial working directory.
+# This is actually a test of the underlying mkdir-p.c code.
+# The part in question cannot be tested via mkdir(1) because that
+# program cannot create leading directories that lack u=wx permissions,
+# so we have to test with install (aka ginstall in the build directory).
+(cd no-acce3s; chmod 0 . && ginstall -m 0 -d $p/c/b $p/y/z) || fail=1
+p=`ls -ld $p/y|sed 's/ .*//'`
+case $p in d---------);; *) fail=1;; esac
+
exit $fail
Index: src/install.c
===================================================================
RCS file: /fetish/cu/src/install.c,v
retrieving revision 1.184
retrieving revision 1.185
diff -u -p -u -r1.184 -r1.185
--- src/install.c 14 Jun 2005 23:55:24 -0000 1.184
+++ src/install.c 15 Jun 2005 08:54:21 -0000 1.185
@@ -360,7 +360,7 @@ main (int argc, char **argv)
int cwd_errno = 0;
for (i = 0; i < n_files; i++)
{
- if (cwd_errno != 0 && IS_RELATIVE_FILE_NAME (argv[optind]))
+ if (cwd_errno != 0 && IS_RELATIVE_FILE_NAME (file[i]))
{
error (0, cwd_errno, _("cannot return to working directory"));
ok = false;
Index: tests/install/basic-1
===================================================================
RCS file: /fetish/cu/tests/install/basic-1,v
retrieving revision 1.15
retrieving revision 1.17
diff -u -p -r1.15 -r1.17
--- tests/install/basic-1 21 Apr 2005 00:30:13 -0000 1.15
+++ tests/install/basic-1 15 Jun 2005 09:31:16 -0000 1.17
@@ -9,12 +9,12 @@ dir=dir
file=file
pwd=`pwd`
-tmp=inst-basic.$$
-trap 'status=$?; cd $pwd; rm -rf $tmp && exit $status' 0
-trap 'exit $?' 1 2 13 15
+t0=`echo "$0"|sed 's,.*/,,'`.tmp; tmp=$t0/$$
+trap 'status=$?; cd $pwd; chmod -R u+rwx $t0; rm -rf $t0 && exit $status' 0
+trap '(exit $?); exit $?' 1 2 13 15
framework_failure=0
-mkdir $tmp || framework_failure=1
+mkdir -p $tmp || framework_failure=1
cd $tmp || framework_failure=1
rm -rf $dir $file || framework_failure=1
@@ -36,7 +36,7 @@ test -f $dir/$file || fail=1
# Make sure strip works.
dd=dd$EXEEXT
dd2=dd2$EXEEXT
-cp ../../../src/$dd . || fail=1
+cp $pwd/../../src/$dd . || fail=1
cp $dd $dd2 || fail=1
strip $dd2 || \
@@ -63,6 +63,26 @@ test "$1" = -r-xr-xr-x || fail=1
# These failed in coreutils CVS from 2004-06-25 to 2004-08-11.
ginstall -d . || fail=1
ginstall -d newdir || fail=1
+test -d newdir || fail=1
ginstall -d newdir1 newdir2 newdir3 || fail=1
+test -d newdir1 || fail=1
+test -d newdir2 || fail=1
+test -d newdir3 || fail=1
+
+# This fails because mkdir-p.c's make_dir_parents fails to return to its
+# initial working directory ($abs) after creating the first argument, and
+# hence cannot do anything meaningful with the following relative-named dirs.
+abs=$pwd/$tmp
+mkdir sub && cd sub
+chmod 0 .; ginstall -d $abs/xx/yy rel/sub1 rel/sub2 2> /dev/null && fail=1
+chmod 755 $abs/sub
+
+# Ensure that the first argument-dir has been created.
+test -d $abs/xx/yy || fail=1
+
+# Make sure that the `rel' directory was not created...
+test -d $abs/sub/rel && fail=1
+# and make sure it was not created in the wrong place.
+test -d $abs/xx/rel && fail=1
(exit $fail); exit $fail