This is the mail archive of the cygwin-developers@cygwin.com 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]

Re: current repository: stat() doesn't set inode number correctly for magic devices


On Mon, Oct 29, 2001 at 03:02:29PM -0500, Jonathan Kamens wrote:
>With the current repository, if you call stat() on a device, e.g.,
>"/dev/null", the returned stat structure will have st_ino set to 0.
>If you open() the same device and then call fstat() on it, the
>returned stat structure will have st_ino correctly set to a non-zero
>value.  This is demonstrated by the program below.
>
>The problem is that the namehash field of the fhandler structure is
>only set when open() is called.  It is not set when the various
>constructors for magic devices, e.g.,
>fhandler_dev_null::fhandler_dev_null, are called.
>
>This bug causes "echo foo > /dev/null" to fail in bash, because bash
>stats a file before opening it, fstats it afterwards, and then
>compares the inode numbers from before and after to confirm that it
>hasn't been swapped out from under it.

I don't have any problems typing echo foo >/dev/null.

I wonder why this is different for you.  Maybe you're doing something
with noclobber or something.

>When I was thinking about how this might be fixed, I wanted to avoid
>adding separate calls to hash_path_name in every one of the
>constructors for magic devices, so I thought that the correct place to
>fix this would be in build_fhandler_from_name.  Unfortunately, it
>can't be done there because "namehash" is a private field and there's
>no public setter for it.  I don't know which is the most appropriate
>fix -- adding separate calls to hash_path_name to set the hash in
>every one of the device constructors; adding a public setter for the
>namehash field so that it can be set from buildhandler_from_name;
>making namehash a public field so it can be set directly; or some
>other solution I haven't considered.
>
>This probably needs to be fixed before 1.3.4 is released (sorry!).

Yep, it does.  But you've analyzed it far enough for a simple fix
to be feasible.  The change below seems to do the right thing.

I think it is even the most correct fix since namehash *should* be
changed when the filename changes, so putting the namehash setting
in set_name makes sense.

Thanks for the analysis, Jonathan.

cgf

Index: fhandler.cc
===================================================================
RCS file: /cvs/uberbaum/winsup/cygwin/fhandler.cc,v
retrieving revision 1.99
diff -u -p -r1.99 fhandler.cc
--- fhandler.cc 2001/10/24 04:16:45     1.99
+++ fhandler.cc 2001/10/29 20:26:55
@@ -189,6 +189,7 @@ fhandler_base::set_name (const char *uni
       system_printf ("fatal error. strdup failed");
       exit (ENOMEM);
     }
+  namehash = hash_path_name (0, win32_path_name);
 }
 
 void
@@ -411,7 +412,6 @@ fhandler_base::open (path_conv *, int fl
       && !allow_ntsec && allow_ntea)
     set_file_attribute (has_acls (), get_win32_name (), mode);
 
-  namehash = hash_path_name (0, get_win32_name ());
   set_io_handle (x);
   int bin;
   int fmode;


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