This is the mail archive of the cygwin-patches mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [1.7] rename/renameat error


On Thu, Sep 24, 2009 at 09:31:45PM -0600, Eric Blake wrote:
>-----BEGIN PGP SIGNED MESSAGE-----
>Hash: SHA1
>
>According to Christopher Faylor on 9/23/2009 10:41 AM:
>>> Also less risky would be to make changes locally in mkdir, link, and
>>> rename for now.
>
>Done - this patch narrows the scope of the changes to just the interfaces
>in question.  I've also tested that it made it through the coreutils
>testsuite without any regressions.
>
>> 
>> I'm not clear if this is a regression or not.  If it isn't a regression,
>> I'd opt for leaving it until 1.7.2.
>
>Now that I'm not touching path.cc, these are all much more self-contained,
>and make cygwin more consistent with Linux.  For example:
>
>touch a
>ln -s c b
>link a b/
>
>should fail because b/ is not an existing directory, but without this
>patch, it succeeds and creates the regular file c as a link to a.
>
>2009-09-24  Eric Blake  <ebb9@byu.net>
>
>	* syscalls.cc (link): Delete obsolete comment.  Reject directories
>	and missing source up front.
>	(rename): Use correct errno for trailing '.'.  Allow trailing
>	slash to newpath iff oldpath is directory.
>	* dir.cc (mkdir): Reject dangling symlink with trailing slash.
>	* fhandler_disk_file.cc (fhandler_disk_file::link): Reject
>	trailing slash.
>	* fhandler.cc (fhandler_base::link): Match Linux errno.
>
>- --
>Don't work too hard, make some time for fun as well!
>
>Eric Blake             ebb9@byu.net
>-----BEGIN PGP SIGNATURE-----
>Version: GnuPG v1.4.9 (Cygwin)
>Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
>Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
>
>iEYEARECAAYFAkq8OaEACgkQ84KuGfSFAYAiLACghYLCFaIGmFR4AuzKAmBuQcg/
>kFoAoJcX+ufE6YUq7S1AeVRvHtyZ30wc
>=4otJ
>-----END PGP SIGNATURE-----

>diff --git a/winsup/cygwin/dir.cc b/winsup/cygwin/dir.cc
>index 2b9125f..b7c31e4 100644
>--- a/winsup/cygwin/dir.cc
>+++ b/winsup/cygwin/dir.cc
>@@ -1,6 +1,7 @@
> /* dir.cc: Posix directory-related routines
>
>-   Copyright 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2006, 2007 Red Hat, Inc.
>+   Copyright 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2006, 2007,
>+   2009 Red Hat, Inc.
>
> This file is part of Cygwin.
>
>@@ -21,6 +22,7 @@ details. */
> #include "dtable.h"
> #include "cygheap.h"
> #include "cygtls.h"
>+#include "tls_pbuf.h"
>
> extern "C" int
> dirfd (DIR *dir)
>@@ -273,11 +275,26 @@ mkdir (const char *dir, mode_t mode)
> {
>   int res = -1;
>   fhandler_base *fh = NULL;
>+  size_t dlen;
>+  char *newbuf;
>+  tmp_pathbuf tp;
>
>   myfault efault;
>   if (efault.faulted (EFAULT))
>     return -1;
>
>+  /* POSIX says mkdir("symlink-to-missing/") should create the
>+     directory "missing", but Linux rejects it with EEXIST.  Copy
>+     Linux behavior for now.  */
>+
>+  dlen = strlen (dir);
>+  if (isdirsep (dir[dlen - 1]))

Couldn't this index negatively if dir is zero length?

>+    {
>+      stpcpy (newbuf = tp.c_get (), dir);

Since stpcpy returns a pointer to the end of the buffer couldn't we use that
and do pointer arithmetic rather than index arithmetic?

>+      while (dlen > 0 && isdirsep (dir[dlen - 1]))
>+        newbuf[--dlen] = '\0';
>+      dir = newbuf;
>+    }
>   if (!(fh = build_fh_name (dir, NULL, PC_SYM_NOFOLLOW)))
>     goto done;   /* errno already set */;
>
>diff --git a/winsup/cygwin/fhandler.cc b/winsup/cygwin/fhandler.cc
>index b52d7c2..44311ca 100644
>--- a/winsup/cygwin/fhandler.cc
>+++ b/winsup/cygwin/fhandler.cc
>@@ -1541,7 +1541,7 @@ fhandler_base::ftruncate (_off64_t length, bool allow_truncate)
> int
> fhandler_base::link (const char *newpath)
> {
>-  set_errno (EINVAL);
>+  set_errno (EPERM);
>   return -1;
> }
>
>diff --git a/winsup/cygwin/fhandler_disk_file.cc b/winsup/cygwin/fhandler_disk_file.cc
>index 214be47..4c4b559 100644
>--- a/winsup/cygwin/fhandler_disk_file.cc
>+++ b/winsup/cygwin/fhandler_disk_file.cc
>@@ -1186,6 +1186,7 @@ fhandler_disk_file::ftruncate (_off64_t length, bool allow_truncate)
> int
> fhandler_disk_file::link (const char *newpath)
> {
>+  size_t nlen = strlen (newpath);
>   path_conv newpc (newpath, PC_SYM_NOFOLLOW | PC_POSIX, stat_suffixes);
>   if (newpc.error)
>     {
>@@ -1200,7 +1201,13 @@ fhandler_disk_file::link (const char *newpath)
>       return -1;
>     }
>
>-  char new_buf[strlen (newpath) + 5];
>+  if (isdirsep (newpath[nlen - 1]) || has_dot_last_component (newpath, false))

Same observation:  Couldn't newpath be zero length?

cgf


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]