Re: [PATCH v2 6/8] syscalls.cc: Expose shallow-pathconv unlink_nt

[Corinna Vinschen <corinna-cygwin@cygwin.com>]

On Jan 20 17:10, Ben Wijen wrote:
> Not having to query file information improves unlink speed.
> ---
>  winsup/cygwin/syscalls.cc | 78 ++++++++++++++++++++++++++-------------
>  1 file changed, 52 insertions(+), 26 deletions(-)
> 
> diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
> index ab0c4c2d6..b5ab6ac5e 100644
> --- a/winsup/cygwin/syscalls.cc
> +++ b/winsup/cygwin/syscalls.cc
> @@ -1272,6 +1272,28 @@ _unlink_ntpc_ (path_conv& pc, bool shareable)
>    return status;
>  }
>  
> +NTSTATUS
> +unlink_nt (const char *ourname, ULONG eflags)
> +{
> +  uint32_t opt = PC_SYM_NOFOLLOW | PC_SKIP_SYM_CHECK | PC_SKIP_FS_CHECK;
> +  if (!(eflags & FILE_NON_DIRECTORY_FILE))
> +    opt &= ~PC_SKIP_FS_CHECK;
> +
> +  path_conv pc (ourname, opt, NULL);
> +  if (pc.error || pc.isspecial ())
> +    return STATUS_CANNOT_DELETE;
> +
> +  OBJECT_ATTRIBUTES attr;
> +  PUNICODE_STRING ntpath = pc.get_nt_native_path ();
> +  InitializeObjectAttributes(&attr, ntpath, 0, NULL, NULL);
> +  NTSTATUS status = _unlink_nt (&attr, eflags);

Sorry again, but I don't see the advantage of not using the intelligence
already collected in path_conv by neglecting to call the unlink
variation not using them.  It's also unclear to me, why the new code
doesn't try_to_bin right away, rather than stomping ahead and falling
back to the current code for each such file.  In an rm -rf on a file
hirarchy used by other users, you could end up with a much slower
operation.

Wouldn't it make more sense to streamline the existing _unlink_nt?  I
don't claim it's the most streamlined way to delete files, but at least
it has documented workarounds for problems encountered on the way,
already.


Corinna