Re: [Patch] unlink

2004-10-30 Thread Christopher Faylor
On Fri, Oct 29, 2004 at 06:01:51PM -0400, Pierre A. Humblet wrote:
Here is a patch that should allow unlink() to handle
nul etc.. on local disks.

It's a cut and paste of Corinna's open on NT and the
existing CreateFile.
 
It works on normal files. I haven't tested with the
special names because I forgot how to create them !
Feedback welcome.

X This should NOT be applied in 1.5.12 XX

Pierre

2004-10-29  Pierre Humblet [EMAIL PROTECTED]

   * syscalls.cc (nt_delete): New function.
   (unlink): Call nt_delete instead of CreateFile and remove
   unreachable code.

Corinna suggested something similar to me a couple of months ago but I
wanted to wait for things to settle down somewhat after the original
use of NtCreateFile.

On reflection, however, wouldn't it be a little easier just to prepend
the path being deleted with a: \\.\ so that rm nul would eventually
translate to DeleteFile(\\.\c:\foo\null) (I'm not using true C
backslash quoting here)?  I don't know if that would work on Windows 9x,
though.

cgf


Re: [Patch] cygcheck: Don't use keyeprint if GetLastError is irrelevant.

2004-10-30 Thread Bas van Gompel
Op Fri, 29 Oct 2004 11:22:38 -0400 schreef Christopher Faylor
in [EMAIL PROTECTED]:
:  On Fri, Oct 29, 2004 at 06:31:11AM +0200, Bas van Gompel wrote:
:  Following (trivial, once more, I hope) patch cleans up some of the
:  (IMO) inappropriate ``keyeprint'' usage in cygcheck. It (keyeprint)
:  should not be used when GetLastError does not apply, I think. Also the
:  format ending in ``failed'' can cause strange messages like ``NULL
:  pointer for file failed''.
:
:   If malloc failed, it is not inconceivable that there is a system error.

Ok.

:  Since the point of keyeprint is to print error messages, reverting to
:  using raw puts is a step backwards.  If it is really known that there is
:  not a remote possibility that GetLastError will be useful, then an
:  option to keyeprint should be added.

I thought so too, at first. I'll admit my solution wasn't pretty
either. Maybe a new function sh/could be added to print messages on
stderr, but without the ``failed'' suffix and the LastError output.
(This could then be called from keyeprint as well.)

:  I'd rather regularize error output
:  throughout cygcheck (which may be a bigger job than your current assignment
:  status will allow) than sprinkle fputs's, and fprints's around the code.

I hope a step-by-step approach will work...
[...]

:  While doing this I caught a typo in get_dword.

I'll start by trying to fix that one (and some more).
(The ones in track_down don't look like they can ever really
get triggered.)


ChangeLog-entry:

2004-10-28  Bas van Gompel  [EMAIL PROTECTED]

* cygcheck.cc (get_dword): Fix errormessage.
(cygwin_info): Ditto.
(track_down): Ditto.
(check_keys): Ditto.


--- src/winsup/utils-keye-usage-p0/cygcheck.cc  27 Oct 2004 01:28:07 -  1.58
+++ src/winsup/utils-keye-usage-p0/cygcheck.cc  30 Oct 2004 03:16:07 -
@@ -276,7 +276,7 @@ get_dword (HANDLE fh, int offset)
 
   if (SetFilePointer (fh, offset, 0, FILE_BEGIN) == INVALID_SET_FILE_POINTER
GetLastError () != NO_ERROR)
-keyeprint (get_word: SetFilePointer());
+keyeprint (get_dword: SetFilePointer());
 
   if (!ReadFile (fh, rv, 4, (DWORD *) r, 0))
 keyeprint (get_dword: Readfile());
@@ -359,7 +359,7 @@ cygwin_info (HANDLE h)
   buf_start = buf = (char *) calloc (1, size + 1);
   if (buf == NULL)
 {
-  keyeprint (cygwin_info: malloc());
+  keyeprint (cygwin_info: calloc());
   return;
 }
 
@@ -537,13 +537,13 @@ track_down (char *file, char *suffix, in
 {
   if (file == NULL)
 {
-  keyeprint (track_down: malloc());
+  keyeprint (track_down: NULL passed for file);
   return;
 }
 
   if (suffix == NULL)
 {
-  keyeprint (track_down: malloc());
+  keyeprint (track_down: NULL passed for suffix);
   return;
 }
 
@@ -1271,7 +1271,7 @@ check_keys ()
  OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
 
   if (h == INVALID_HANDLE_VALUE || h == NULL)
-return (keyeprint (check_key: Opening CONIN$));
+return (keyeprint (check_keys: Opening CONIN$));
 
   DWORD mode;
 
@@ -1281,7 +1281,7 @@ check_keys ()
 {
   mode = ~ENABLE_PROCESSED_INPUT;
   if (!SetConsoleMode (h, mode))
-   keyeprint (check_keys: GetConsoleMode());
+   keyeprint (check_keys: SetConsoleMode());
 }
 
   fputs (\nThis key check works only in a console window,, stderr);
@@ -1300,7 +1300,7 @@ check_keys ()
 {
   prev_in = in;
   if (!ReadConsoleInput (h, in, 1, mode))
-   keyeprint (ReadConsoleInput);
+   keyeprint (check_keys: ReadConsoleInput());
 
   if (!memcmp (in, prev_in, sizeof in))
continue;


L8r,

Buzz.
-- 
  ) |  | ---/ ---/  Yes, this | This message consists of true | I do not
--  |  |   //   really is |   and false bits entirely.| mail for
  ) |  |  //a 72 by 4 +---+ any1 but
--  \--| /--- /---  .sigfile. |   |perl -pe s.u(z)\1.as.| me. 4^re


Re: [Patch] cygcheck: Don't use keyeprint if GetLastError is irrelevant.

2004-10-30 Thread Christopher Faylor
On Sat, Oct 30, 2004 at 11:32:27PM +0200, Bas van Gompel wrote:
2004-10-28  Bas van Gompel  [EMAIL PROTECTED]

   * cygcheck.cc (get_dword): Fix errormessage.
   (cygwin_info): Ditto.
   (track_down): Ditto.
   (check_keys): Ditto.

Go ahead and check these in.

Thanks,
cgf


Re: [Patch] unlink

2004-10-30 Thread Pierre A. Humblet
At 01:39 PM 10/30/2004 -0400, you wrote:
On Fri, Oct 29, 2004 at 06:01:51PM -0400, Pierre A. Humblet wrote:
Here is a patch that should allow unlink() to handle
nul etc.. on local disks.

It's a cut and paste of Corinna's open on NT and the
existing CreateFile.
 
It works on normal files. I haven't tested with the
special names because I forgot how to create them !
Feedback welcome.

X This should NOT be applied in 1.5.12 XX

Pierre

2004-10-29  Pierre Humblet [EMAIL PROTECTED]

  * syscalls.cc (nt_delete): New function.
  (unlink): Call nt_delete instead of CreateFile and remove
  unreachable code.

Corinna suggested something similar to me a couple of months ago but I
wanted to wait for things to settle down somewhat after the original
use of NtCreateFile.

On reflection, however, wouldn't it be a little easier just to prepend
the path being deleted with a: \\.\ so that rm nul would eventually
translate to DeleteFile(\\.\c:\foo\null) (I'm not using true C
backslash quoting here)?  I don't know if that would work on Windows 9x,
though.

That would work on NT, but then one would need to check if the input
path didn't already have the form //./xx, worry about exceeding max 
pathlength, etc... The patch cleanly handles all of that, is symmetrical
to file creation, and is very efficient. I cleaned it up a little and
tested readonly hard links and other weird cases.
It's good to go but I still wouldn't include it in 1.5.12 if it's
coming out this weekend.

Pierre

2004-10-31  Pierre Humblet [EMAIL PROTECTED]

* syscalls.cc (nt_delete): New function.
(unlink): Call nt_delete instead of CreateFile and remove
unreachable code.

Index: syscalls.cc
===
RCS file: /cvs/src/src/winsup/cygwin/syscalls.cc,v
retrieving revision 1.349
diff -u -p -b -r1.349 syscalls.cc
--- syscalls.cc 28 Oct 2004 01:46:01 -  1.349
+++ syscalls.cc 31 Oct 2004 02:10:49 -
@@ -39,6 +39,8 @@ details. */
 #include wininet.h
 #include lmcons.h /* for UNLEN */
 #include rpc.h
+#include ntdef.h
+#include ntdll.h

 #undef fstat
 #undef lstat
@@ -127,6 +129,30 @@ dup2 (int oldfd, int newfd)
   return cygheap-fdtab.dup2 (oldfd, newfd);
 }

+static HANDLE
+nt_delete (path_conv  pc)
+{
+  WCHAR wpath[CYG_MAX_PATH + 10];
+  UNICODE_STRING upath = {0, sizeof (wpath), wpath};
+  pc.get_nt_native_path (upath);
+
+  HANDLE x;
+  OBJECT_ATTRIBUTES attr;
+  IO_STATUS_BLOCK io;
+  NTSTATUS status;
+
+  InitializeObjectAttributes (attr, upath, OBJ_CASE_INSENSITIVE, NULL, NULL);
+  status = NtCreateFile (x, DELETE, attr, io, NULL, FILE_ATTRIBUTE_NORMAL,
+ FILE_SHARE_READ, FILE_OPEN, FILE_DELETE_ON_CLOSE, NULL, 0);
+  if (!NT_SUCCESS (status))
+{
+  __seterrno_from_win_error (RtlNtStatusToDosError (status));
+  return INVALID_HANDLE_VALUE;
+}
+  else
+return x;
+}
+
 extern C int
 unlink (const char *ourname)
 {
@@ -192,30 +218,17 @@ unlink (const char *ourname)
  Microsoft KB 837665 describes this problem as a bug in 2K3, but I have
  reproduced it on shares on Samba 2.2.8, Samba 3.0.2, NT4SP6, XP64SP1 and
  2K3 and in all cases, DeleteFile works, delete on close does not. */
-  if (!win32_name.isremote ()  wincap.has_delete_on_close ())
+  if (!win32_name.isremote ()  wincap.is_winnt ())
 {
-  HANDLE h;
-  h = CreateFile (win32_name, 0, FILE_SHARE_READ, sec_none_nih,
- OPEN_EXISTING, FILE_FLAG_DELETE_ON_CLOSE, 0);
+  HANDLE h = nt_delete (win32_name);
   if (h != INVALID_HANDLE_VALUE)
{
- if (wincap.has_hard_links ()  setattrs)
+ if (setattrs  wincap.has_hard_links ())
(void) SetFileAttributes (win32_name, (DWORD) win32_name);
  BOOL res = CloseHandle (h);
- syscall_printf (%d = CloseHandle (%p), res, h);
- if (GetFileAttributes (win32_name) == INVALID_FILE_ATTRIBUTES
- || !win32_name.isremote ())
-   {
- syscall_printf (CreateFile (FILE_FLAG_DELETE_ON_CLOSE) succeeded);
+ syscall_printf (%p = nt_delete (). %d = CloseHandle (), h, res);
  goto ok;
}
- else
-   {
- syscall_printf (CreateFile (FILE_FLAG_DELETE_ON_CLOSE) failed);
- if (setattrs)
-   SetFileAttributes (win32_name, (DWORD) win32_name  
~(FILE_ATTRIBUTE_READONLY | FILE_ATTRIBUTE_SYSTEM));
-   }
-   }
 }

   /* Try a delete with attributes reset */


Re: [Patch] unlink

2004-10-30 Thread Christopher Faylor
On Sat, Oct 30, 2004 at 10:30:54PM -0400, Pierre A. Humblet wrote:
At 01:39 PM 10/30/2004 -0400, you wrote:
On Fri, Oct 29, 2004 at 06:01:51PM -0400, Pierre A. Humblet wrote:
Here is a patch that should allow unlink() to handle
nul etc.. on local disks.

It's a cut and paste of Corinna's open on NT and the
existing CreateFile.
 
It works on normal files. I haven't tested with the
special names because I forgot how to create them !
Feedback welcome.

X This should NOT be applied in 1.5.12 XX

Pierre

2004-10-29  Pierre Humblet [EMAIL PROTECTED]

 * syscalls.cc (nt_delete): New function.
 (unlink): Call nt_delete instead of CreateFile and remove
 unreachable code.

Corinna suggested something similar to me a couple of months ago but I
wanted to wait for things to settle down somewhat after the original
use of NtCreateFile.

On reflection, however, wouldn't it be a little easier just to prepend
the path being deleted with a: \\.\ so that rm nul would eventually
translate to DeleteFile(\\.\c:\foo\null) (I'm not using true C
backslash quoting here)?  I don't know if that would work on Windows 9x,
though.

That would work on NT, but then one would need to check if the input
path didn't already have the form //./xx, worry about exceeding max 
pathlength, etc...

Other than being able to delete special filenames is there any other
benefit to using NtCreateFile to delete files?

If path length was an issue we could use '//?/' instead since the length
restriction is a lot larger there.  So, it would be something like:

  char *path;
  char newpath[strlen (win32_name) + 4] = ?\;
  if  (win32_name[0] != '\\')
  path = strcat (newpath, win32_name);
  else
  path = win32_name;

and then you'd use path throughout from then on.

We could use a technique like this for other things, like rename, to enable
it to manipulate accidentally-created special files.

OTOH, I just had an idea about how to use //?/ on NT so that we could
have longer path names.  I have to mull it over a little to see if it
would work or not, though.  I have an airplane trip tomorrow that would
be perfect for this kind of mulling.

cgf