[PATCH v3 resend 2/2] server: Allow creating named pipes using \Device\NamedPipe\ as RootDirectory.
Jacek Caban
jacek at codeweavers.com
Tue Jan 4 15:58:28 CST 2022
Hi Jinoh,
Sorry for the delay. The patch seems a bit risky for code freeze, but it
fixes a regression, so I'm not sure. Anyway, I have a few comments bellow.
On 1/4/22 11:08, Jinoh Kang wrote:
> Separate the named pipe root directory from the named pipe device file.
> Open the root directory instead of the device file if the path ends
> with backslash.
>
> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52105
> Signed-off-by: Jinoh Kang <jinoh.kang.kr at gmail.com>
> ---
> dlls/ntdll/tests/om.c | 2 +-
> dlls/ntdll/tests/pipe.c | 6 +-
> server/named_pipe.c | 217 ++++++++++++++++++++++++++++++++--------
> 3 files changed, 181 insertions(+), 44 deletions(-)
>
> diff --git a/dlls/ntdll/tests/om.c b/dlls/ntdll/tests/om.c
> index 04acf5b2241..b5a32ec1081 100644
> --- a/dlls/ntdll/tests/om.c
> +++ b/dlls/ntdll/tests/om.c
> @@ -1874,7 +1874,7 @@ static void test_query_object(void)
> handle = CreateFileA( "\\\\.\\pipe\\", 0, 0, NULL, OPEN_EXISTING, 0, 0 );
> ok( handle != INVALID_HANDLE_VALUE, "CreateFile failed (%d)\n", GetLastError() );
>
> - test_object_name( handle, L"\\Device\\NamedPipe\\", TRUE );
> + test_object_name( handle, L"\\Device\\NamedPipe\\", FALSE );
> test_object_type( handle, L"File" );
> test_file_info( handle );
>
> diff --git a/dlls/ntdll/tests/pipe.c b/dlls/ntdll/tests/pipe.c
> index f5ca6077605..4b0fa552748 100644
> --- a/dlls/ntdll/tests/pipe.c
> +++ b/dlls/ntdll/tests/pipe.c
> @@ -2509,7 +2509,7 @@ static void test_empty_name(void)
>
> pRtlInitUnicodeString(&name, L"nonexistent_pipe");
> status = wait_pipe(hdirectory, &name, &zero_timeout);
> - todo_wine ok(status == STATUS_ILLEGAL_FUNCTION, "unexpected status for FSCTL_PIPE_WAIT on \\Device\\NamedPipe: %08x\n", status);
> + ok(status == STATUS_ILLEGAL_FUNCTION, "unexpected status for FSCTL_PIPE_WAIT on \\Device\\NamedPipe: %08x\n", status);
>
> name.Buffer = NULL;
> name.Length = 0;
> @@ -2650,11 +2650,11 @@ static void test_empty_name(void)
> timeout.QuadPart = -(LONG64)10000000;
> status = pNtCreateNamedPipeFile(&hpipe, GENERIC_READ|GENERIC_WRITE, &attr, &io, FILE_SHARE_READ|FILE_SHARE_WRITE,
> FILE_CREATE, FILE_PIPE_FULL_DUPLEX, 0, 0, 0, 1, 256, 256, &timeout);
> - todo_wine ok(!status, "unexpected failure from NtCreateNamedPipeFile: %08x\n", status);
> + ok(!status, "unexpected failure from NtCreateNamedPipeFile: %08x\n", status);
>
> handle = CreateFileA("\\\\.\\pipe\\test3\\pipe", GENERIC_READ, FILE_SHARE_READ|FILE_SHARE_WRITE, NULL,
> OPEN_EXISTING, 0, 0 );
> - todo_wine ok(handle != INVALID_HANDLE_VALUE, "Failed to open NamedPipe (%u)\n", GetLastError());
> + ok(handle != INVALID_HANDLE_VALUE, "Failed to open NamedPipe (%u)\n", GetLastError());
>
> CloseHandle(handle);
> CloseHandle(hpipe);
> diff --git a/server/named_pipe.c b/server/named_pipe.c
> index 3e6cf09d4f2..37520789722 100644
> --- a/server/named_pipe.c
> +++ b/server/named_pipe.c
> @@ -103,6 +103,13 @@ struct named_pipe_device_file
> struct named_pipe_device *device; /* named pipe device */
> };
>
> +struct named_pipe_dir
> +{
> + struct object obj; /* object header */
> + struct fd *fd; /* pseudo-fd for ioctls */
> + struct named_pipe_device *device; /* named pipe device */
> +};
This is identical to named_pipe_device_file, maybe we could reuse that
(we'd still use different ops).
> +
> static void named_pipe_dump( struct object *obj, int verbose );
> static unsigned int named_pipe_map_access( struct object *obj, unsigned int access );
> static WCHAR *named_pipe_get_full_name( struct object *obj, data_size_t *ret_len );
> @@ -321,6 +328,57 @@ static const struct fd_ops named_pipe_device_fd_ops =
> default_fd_reselect_async /* reselect_async */
> };
>
> +static void named_pipe_dir_dump( struct object *obj, int verbose );
> +static struct fd *named_pipe_dir_get_fd( struct object *obj );
> +static enum server_fd_type named_pipe_dir_get_fd_type( struct fd *fd );
> +static void named_pipe_dir_ioctl( struct fd *fd, ioctl_code_t code, struct async *async );
> +static WCHAR *named_pipe_dir_get_full_name( struct object *obj, data_size_t *ret_len );
> +static struct object *named_pipe_dir_lookup_name( struct object *obj, struct unicode_str *name,
> + unsigned int attr, struct object *root );
> +static struct object *named_pipe_dir_open_file( struct object *obj, unsigned int access,
> + unsigned int sharing, unsigned int options );
> +static void named_pipe_dir_destroy( struct object *obj );
> +
> +static const struct object_ops named_pipe_dir_ops =
> +{
> + sizeof(struct named_pipe_dir), /* size */
> + &file_type, /* type */
> + named_pipe_dir_dump, /* dump */
> + add_queue, /* add_queue */
> + remove_queue, /* remove_queue */
> + default_fd_signaled, /* signaled */
> + no_satisfied, /* satisfied */
> + no_signal, /* signal */
> + named_pipe_dir_get_fd, /* get_fd */
> + default_map_access, /* map_access */
> + default_get_sd, /* get_sd */
> + default_set_sd, /* set_sd */
> + named_pipe_dir_get_full_name, /* get_full_name */
> + named_pipe_dir_lookup_name, /* lookup_name */
> + no_link_name, /* link_name */
> + NULL, /* unlink_name */
> + named_pipe_dir_open_file, /* open_file */
> + no_kernel_obj_list, /* get_kernel_obj_list */
> + no_close_handle, /* close_handle */
> + named_pipe_dir_destroy /* destroy */
> +};
> +
> +static const struct fd_ops named_pipe_dir_fd_ops =
> +{
> + default_fd_get_poll_events, /* get_poll_events */
> + default_poll_event, /* poll_event */
> + named_pipe_dir_get_fd_type, /* get_fd_type */
> + no_fd_read, /* read */
> + no_fd_write, /* write */
> + no_fd_flush, /* flush */
> + default_fd_get_file_info, /* get_file_info */
> + no_fd_get_volume_info, /* get_volume_info */
> + named_pipe_dir_ioctl, /* ioctl */
> + default_fd_cancel_async, /* cancel_async */
> + default_fd_queue_async, /* queue_async */
> + default_fd_reselect_async /* reselect_async */
> +};
> +
> static void named_pipe_dump( struct object *obj, int verbose )
> {
> fputs( "Named pipe\n", stderr );
> @@ -501,6 +559,15 @@ static struct object *named_pipe_device_lookup_name( struct object *obj, struct
> assert( device->pipes );
>
> if (!name) return NULL; /* open the device itself */
> + if (!name->len && name->str)
> + {
> + /* open the root directory */
> + struct named_pipe_dir *dir;
> + if (!(dir = alloc_object( &named_pipe_dir_ops ))) return NULL;
> + dir->fd = NULL;
> + dir->device = (struct named_pipe_device *)grab_object( obj );
> + return &dir->obj;
> + }
>
> if ((found = find_object( device->pipes, name, attr | OBJ_CASE_INSENSITIVE )))
> name->len = 0;
> @@ -581,6 +648,113 @@ static void named_pipe_device_file_destroy( struct object *obj )
> release_object( file->device );
> }
>
> +static void named_pipe_dir_dump( struct object *obj, int verbose )
> +{
> + struct named_pipe_dir *dir = (struct named_pipe_dir *)obj;
> +
> + fprintf( stderr, "Root directory of named pipe device %p\n", dir->device );
> +}
> +
> +static struct fd *named_pipe_dir_get_fd( struct object *obj )
> +{
> + struct named_pipe_dir *dir = (struct named_pipe_dir *)obj;
> + return (struct fd *)grab_object( dir->fd );
> +}
> +
> +static enum server_fd_type named_pipe_dir_get_fd_type( struct fd *fd )
> +{
> + /* TODO actually implement NtQueryDirectoryFile */
> + return FD_TYPE_DIR;
> +}
> +
> +static void named_pipe_dir_ioctl( struct fd *fd, ioctl_code_t code, struct async *async )
> +{
> + struct named_pipe_dir *dir = get_fd_user( fd );
> +
> + switch(code)
> + {
> + case FSCTL_PIPE_WAIT:
> + {
> + const FILE_PIPE_WAIT_FOR_BUFFER *buffer = get_req_data();
> + data_size_t size = get_req_data_size();
> + struct named_pipe *pipe;
> + struct unicode_str name;
> + timeout_t when;
> +
> + if (size < sizeof(*buffer) ||
> + size < FIELD_OFFSET(FILE_PIPE_WAIT_FOR_BUFFER, Name[buffer->NameLength/sizeof(WCHAR)]))
> + {
> + set_error( STATUS_INVALID_PARAMETER );
> + return;
> + }
> + name.str = buffer->Name;
> + name.len = (buffer->NameLength / sizeof(WCHAR)) * sizeof(WCHAR);
> + if (!(pipe = open_named_object( &dir->obj, &named_pipe_ops, &name, 0 ))) return;
> +
> + if (list_empty( &pipe->listeners ))
> + {
> + queue_async( &pipe->waiters, async );
> + when = buffer->TimeoutSpecified ? buffer->Timeout.QuadPart : pipe->timeout;
> + async_set_timeout( async, when, STATUS_IO_TIMEOUT );
> + set_error( STATUS_PENDING );
> + }
> +
> + release_object( pipe );
> + return;
> + }
> +
> + default:
> + default_fd_ioctl( fd, code, async );
> + }
> +}
> +
> +static WCHAR *named_pipe_dir_get_full_name( struct object *obj, data_size_t *ret_len )
> +{
> + static const WCHAR backslash = '\\';
> + struct named_pipe_dir *dir = (struct named_pipe_dir *)obj;
> + data_size_t len;
> + char *device_name, *ret;
> +
> + device_name = (char *)dir->device->obj.ops->get_full_name( &dir->device->obj, &len );
> + if (!device_name) return NULL;
> +
> + len += sizeof(WCHAR);
> + ret = realloc(device_name, len);
> + if (!ret)
> + {
> + free(device_name);
> + return NULL;
> + }
> +
> + *ret_len = len;
> + memcpy( ret + len - sizeof(WCHAR), &backslash, sizeof(WCHAR) );
> + return (WCHAR *)ret;
> +}
My first impression was that there should be a better way, but it seems
that we indeed something like that. I think that it could be made a bit
nicer by using WCHAR type for device_name and ret. memcpy() can be
replaced with a simple assignment:
ret[len / sizeof(WCHAR) - 1] = '\\';
> +
> +static struct object *named_pipe_dir_lookup_name( struct object *obj, struct unicode_str *name,
> + unsigned int attr, struct object *root )
> +{
> + struct named_pipe_dir *dir = (struct named_pipe_dir *)obj;
> + if (!name || !name->len) return NULL; /* open the directory itself */
> + return dir->device->obj.ops->lookup_name( &dir->device->obj, name, attr, root );
> +}
> +
> +static struct object *named_pipe_dir_open_file( struct object *obj, unsigned int access,
> + unsigned int sharing, unsigned int options )
> +{
> + struct named_pipe_dir *dir = (struct named_pipe_dir *)obj;
> + if (!dir->fd && !(dir->fd = alloc_pseudo_fd( &named_pipe_dir_fd_ops, obj, options ))) return NULL;
Why do you call alloc_pseudo_fd() here instead of
named_pipe_device_lookup_name, where you alloc dir object? Also, it
could use allow_fd_caching().
Thanks,
Jacek
More information about the wine-devel
mailing list