[PATCH v3 resend 1/2] ntdll/tests: Add more tests for \Device\NamedPipe\ root directory.
Signed-off-by: Jinoh Kang jinoh.kang.kr@gmail.com ---
Notes: v2 -> v3: move some tests from om.c to pipe.c
dlls/ntdll/tests/om.c | 9 ++++ dlls/ntdll/tests/pipe.c | 93 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 101 insertions(+), 1 deletion(-)
diff --git a/dlls/ntdll/tests/om.c b/dlls/ntdll/tests/om.c index f872d6709a1..04acf5b2241 100644 --- a/dlls/ntdll/tests/om.c +++ b/dlls/ntdll/tests/om.c @@ -1871,6 +1871,15 @@ static void test_query_object(void)
pNtClose( handle );
+ 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_type( handle, L"File" ); + test_file_info( handle ); + + pNtClose( handle ); + RtlInitUnicodeString( &path, L"\REGISTRY\Machine" ); status = pNtCreateKey( &handle, KEY_READ, &attr, 0, 0, 0, 0 ); ok( status == STATUS_SUCCESS, "NtCreateKey failed status %x\n", status ); diff --git a/dlls/ntdll/tests/pipe.c b/dlls/ntdll/tests/pipe.c index 6940e0020b7..f5ca6077605 100644 --- a/dlls/ntdll/tests/pipe.c +++ b/dlls/ntdll/tests/pipe.c @@ -51,6 +51,13 @@ typedef struct { ULONG NamedPipeEnd; } FILE_PIPE_LOCAL_INFORMATION;
+typedef struct _FILE_PIPE_WAIT_FOR_BUFFER { + LARGE_INTEGER Timeout; + ULONG NameLength; + BOOLEAN TimeoutSpecified; + WCHAR Name[1]; +} FILE_PIPE_WAIT_FOR_BUFFER, *PFILE_PIPE_WAIT_FOR_BUFFER; + #ifndef FILE_SYNCHRONOUS_IO_ALERT #define FILE_SYNCHRONOUS_IO_ALERT 0x10 #endif @@ -62,6 +69,10 @@ typedef struct { #ifndef FSCTL_PIPE_LISTEN #define FSCTL_PIPE_LISTEN CTL_CODE(FILE_DEVICE_NAMED_PIPE, 2, METHOD_BUFFERED, FILE_ANY_ACCESS) #endif + +#ifndef FSCTL_PIPE_WAIT +#define FSCTL_PIPE_WAIT CTL_CODE(FILE_DEVICE_NAMED_PIPE, 6, METHOD_BUFFERED, FILE_ANY_ACCESS) +#endif #endif
static NTSTATUS (WINAPI *pNtFsControlFile) (HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, PVOID apc_context, PIO_STATUS_BLOCK io, ULONG code, PVOID in_buffer, ULONG in_size, PVOID out_buffer, ULONG out_size); @@ -177,6 +188,51 @@ static NTSTATUS listen_pipe(HANDLE hPipe, HANDLE hEvent, PIO_STATUS_BLOCK iosb, return pNtFsControlFile(hPipe, hEvent, use_apc ? &ioapc: NULL, use_apc ? &dummy: NULL, iosb, FSCTL_PIPE_LISTEN, 0, 0, 0, 0); }
+static NTSTATUS wait_pipe(HANDLE handle, PUNICODE_STRING name, const LARGE_INTEGER* timeout) +{ + HANDLE event; + NTSTATUS status; + OBJECT_ATTRIBUTES attr; + IO_STATUS_BLOCK iosb; + FILE_PIPE_WAIT_FOR_BUFFER *pipe_wait; + ULONG pipe_wait_size; + + pipe_wait_size = offsetof(FILE_PIPE_WAIT_FOR_BUFFER, Name[0]) + name->Length; + pipe_wait = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, pipe_wait_size); + if (!pipe_wait) return STATUS_NO_MEMORY; + + pipe_wait->TimeoutSpecified = !!timeout; + pipe_wait->NameLength = name->Length; + if (timeout) pipe_wait->Timeout = *timeout; + memcpy(pipe_wait->Name, name->Buffer, name->Length); + + InitializeObjectAttributes(&attr, NULL, 0, 0, NULL); + status = NtCreateEvent(&event, GENERIC_ALL, &attr, NotificationEvent, FALSE); + if (status != STATUS_SUCCESS) + { + ok(0, "NtCreateEvent failure: %08x\n", status); + HeapFree(GetProcessHeap(), 0, pipe_wait); + return status; + } + + memset(&iosb, 0, sizeof(iosb)); + iosb.Status = STATUS_PENDING; + status = pNtFsControlFile(handle, event, NULL, NULL, &iosb, FSCTL_PIPE_WAIT, + pipe_wait, pipe_wait_size, NULL, 0); + if (status == STATUS_PENDING && iosb.Status == STATUS_PENDING) + { + WaitForSingleObject(event, INFINITE); + } + if (SUCCEEDED(status) && iosb.Status != STATUS_PENDING) + { + status = iosb.Status; + } + + NtClose(event); + HeapFree(GetProcessHeap(), 0, pipe_wait); + return status; +} + static void test_create_invalid(void) { IO_STATUS_BLOCK iosb; @@ -2420,6 +2476,7 @@ static void test_security_info(void)
static void test_empty_name(void) { + static const LARGE_INTEGER zero_timeout = {{ 0 }}; HANDLE hdirectory, hpipe, hpipe2, hwrite, hwrite2, handle; OBJECT_TYPE_INFORMATION *type_info; OBJECT_NAME_INFORMATION *name_info; @@ -2450,6 +2507,10 @@ static void test_empty_name(void) FILE_SHARE_READ|FILE_SHARE_WRITE, FILE_OPEN, 0, NULL, 0 ); ok(!status, "Got unexpected status %#x.\n", status);
+ 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); + name.Buffer = NULL; name.Length = 0; name.MaximumLength = 0; @@ -2462,6 +2523,17 @@ static void test_empty_name(void) todo_wine ok(status == STATUS_OBJECT_NAME_INVALID, "Got unexpected status %#x.\n", status); if (!status) CloseHandle(hpipe); + + pRtlInitUnicodeString(&name, L"test3\pipe"); + attr.RootDirectory = hdirectory; + attr.ObjectName = &name; + 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); + ok(status == STATUS_OBJECT_NAME_INVALID, "unexpected status from NtCreateNamedPipeFile: %08x\n", status); + if (!status) + CloseHandle(hpipe); + CloseHandle(hdirectory);
pRtlInitUnicodeString(&name, L"\Device\NamedPipe\"); @@ -2475,6 +2547,10 @@ static void test_empty_name(void) FILE_SHARE_READ|FILE_SHARE_WRITE, FILE_OPEN, 0, NULL, 0 ); ok(!status, "Got unexpected status %#x.\n", status);
+ pRtlInitUnicodeString(&name, L"nonexistent_pipe"); + status = wait_pipe(hdirectory, &name, &zero_timeout); + ok(status == STATUS_OBJECT_NAME_NOT_FOUND, "unexpected status for FSCTL_PIPE_WAIT on \Device\NamedPipe\: %08x\n", status); + name.Buffer = NULL; name.Length = 0; name.MaximumLength = 0; @@ -2565,9 +2641,24 @@ static void test_empty_name(void)
CloseHandle(hwrite); CloseHandle(hpipe); - CloseHandle(hdirectory); CloseHandle(hpipe2); CloseHandle(hwrite2); + + pRtlInitUnicodeString(&name, L"test3\pipe"); + attr.RootDirectory = hdirectory; + attr.ObjectName = &name; + 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); + + 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()); + + CloseHandle(handle); + CloseHandle(hpipe); + CloseHandle(hdirectory); }
START_TEST(pipe)
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@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 */ +}; + 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; +} + +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; + return grab_object( obj ); +} + +static void named_pipe_dir_destroy( struct object *obj ) +{ + struct named_pipe_dir *file = (struct named_pipe_dir*)obj; + assert( obj->ops == &named_pipe_dir_ops ); + if (file->fd) release_object( file->fd ); + if (file->device) release_object( file->device ); +} + static void pipe_end_flush( struct fd *fd, struct async *async ) { struct pipe_end *pipe_end = get_fd_user( fd ); @@ -1284,14 +1458,13 @@ static struct pipe_end *create_pipe_client( struct named_pipe *pipe, data_size_t
static int named_pipe_link_name( struct object *obj, struct object_name *name, struct object *parent ) { - struct named_pipe_device *dev = (struct named_pipe_device *)parent; - + if (parent->ops == &named_pipe_dir_ops) parent = &((struct named_pipe_dir *)parent)->device->obj; if (parent->ops != &named_pipe_device_ops) { set_error( STATUS_OBJECT_NAME_INVALID ); return 0; } - namespace_add( dev->pipes, name ); + namespace_add( ((struct named_pipe_device *)parent)->pipes, name ); name->parent = grab_object( parent ); return 1; } @@ -1334,43 +1507,7 @@ static struct object *named_pipe_open_file( struct object *obj, unsigned int acc
static void named_pipe_device_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) { - struct named_pipe_device *device = 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( &device->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 ); - } + set_error( STATUS_ILLEGAL_FUNCTION ); }
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@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
On 1/5/22 06:58, Jacek Caban wrote:
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).
It's certainly possible to reuse the struct, but I suppose making the two structs separate would help check for potential object type confusion.
+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] = '\';
I actually borrowed that part from server/object.c. Puzzled I was at first, but I guessed there must be some legit reason as to why things were done that way, presumably for compatibility with older compilers. (Apparently GCC 4.x compilation problems on Bugzilla were recently acknowledged as legitimate and fixed).
If this is not (or no longer) the case, perhaps we can modify the old code too?
+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?
named_pipe_device_lookup_name lacks the options parameter.
Also, it could use allow_fd_caching().
Thanks!
Thanks,
Jacek
Hi Jinoh,
On 1/4/22 11:07, Jinoh Kang wrote:
- memset(&iosb, 0, sizeof(iosb));
- iosb.Status = STATUS_PENDING;
- status = pNtFsControlFile(handle, event, NULL, NULL, &iosb, FSCTL_PIPE_WAIT,
pipe_wait, pipe_wait_size, NULL, 0);- if (status == STATUS_PENDING && iosb.Status == STATUS_PENDING)
- {
WaitForSingleObject(event, INFINITE);- }
- if (SUCCEEDED(status) && iosb.Status != STATUS_PENDING)
- {
status = iosb.Status;- }
SUCCEEDED() macro is meant for HRESULT. It's not really needed here, I think that those two branches can be simplified to:
if (status == STATUS_SUCCESS)
{
WaitForSingleObject(event, INFINITE); status = iosb.Status;
}
Thanks,
Jacek
-
Jacek Caban -
Jinoh Kang