-- v12: kernelbase: Implement RegLoadAppKey. ntdll: Implement REG_APP_HIVE flag server: Create \REGISTRY\A hive. ntdll: Rename NtLoadKeyEx parameters. ntdll: Pass filename to server in NtLoadKeyEx.
From: Santino Mazza smazza@codeweavers.com
Pass filename instead of openning a file handle and passing it to wineserver. This will help to simplify future implementations for load_key where we have to keep track of the file to lock it, save it, etc. For example, with application hives. --- dlls/ntdll/unix/registry.c | 18 ++++++++--------- include/wine/server_protocol.h | 5 +++-- server/protocol.def | 2 +- server/registry.c | 37 ++++++++++++++++++++-------------- server/request.h | 1 - server/trace.c | 4 ++-- 6 files changed, 36 insertions(+), 31 deletions(-)
diff --git a/dlls/ntdll/unix/registry.c b/dlls/ntdll/unix/registry.c index 797e32a5bf1..a15260d6713 100644 --- a/dlls/ntdll/unix/registry.c +++ b/dlls/ntdll/unix/registry.c @@ -700,7 +700,6 @@ NTSTATUS WINAPI NtLoadKeyEx( const OBJECT_ATTRIBUTES *attr, OBJECT_ATTRIBUTES *f HANDLE event, ACCESS_MASK access, HANDLE *roothandle, IO_STATUS_BLOCK *iostatus ) { NTSTATUS ret; - HANDLE key; data_size_t len; struct object_attributes *objattr; char *unix_name; @@ -717,30 +716,29 @@ NTSTATUS WINAPI NtLoadKeyEx( const OBJECT_ATTRIBUTES *attr, OBJECT_ATTRIBUTES *f if (iostatus) FIXME("iostatus is not filled\n");
get_redirect( &new_attr, &nt_name ); - if (!(ret = nt_to_unix_file_name( &new_attr, &unix_name, FILE_OPEN ))) - { - ret = open_unix_file( &key, unix_name, GENERIC_READ | SYNCHRONIZE, - &new_attr, 0, 0, FILE_OPEN, 0, NULL, 0 ); - free( unix_name ); - } + ret = nt_to_unix_file_name( &new_attr, &unix_name, FILE_OPEN ); free( nt_name.Buffer );
if (ret) return ret;
- if ((ret = alloc_object_attributes( attr, &objattr, &len ))) return ret; + if ((ret = alloc_object_attributes( attr, &objattr, &len ))) + { + free( unix_name ); + return ret; + } objattr->attributes |= OBJ_OPENIF | OBJ_CASE_INSENSITIVE;
SERVER_START_REQ( load_registry ) { - req->file = wine_server_obj_handle( key ); wine_server_add_data( req, objattr, len ); + wine_server_add_data( req, unix_name, strlen( unix_name ) ); ret = wine_server_call( req ); if (ret == STATUS_OBJECT_NAME_EXISTS) ret = STATUS_SUCCESS; } SERVER_END_REQ;
- NtClose( key ); free( objattr ); + free( unix_name ); return ret; }
diff --git a/include/wine/server_protocol.h b/include/wine/server_protocol.h index fb3168c4a6a..080fe4bb6ec 100644 --- a/include/wine/server_protocol.h +++ b/include/wine/server_protocol.h @@ -2360,8 +2360,9 @@ struct delete_key_value_reply struct load_registry_request { struct request_header __header; - obj_handle_t file; /* VARARG(objattr,object_attributes); */ + /* VARARG(filename,string); */ + char __pad_12[4]; }; struct load_registry_reply { @@ -6324,7 +6325,7 @@ union generic_reply
/* ### protocol_version begin ### */
-#define SERVER_PROTOCOL_VERSION 755 +#define SERVER_PROTOCOL_VERSION 757
/* ### protocol_version end ### */
diff --git a/server/protocol.def b/server/protocol.def index d828d41d1f7..9c2a1af35c4 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -1825,8 +1825,8 @@ struct process_info
/* Load a registry branch from a file */ @REQ(load_registry) - obj_handle_t file; /* file to load from */ VARARG(objattr,object_attributes); /* object attributes */ + VARARG(filename,string); /* file to load name */ @END
diff --git a/server/registry.c b/server/registry.c index 96ba18a0a5a..3a83b31a3b4 100644 --- a/server/registry.c +++ b/server/registry.c @@ -1747,24 +1747,17 @@ static void load_keys( struct key *key, const char *filename, FILE *f, int prefi }
/* load a part of the registry from a file */ -static void load_registry( struct key *key, obj_handle_t handle ) +static void load_registry( struct key *key, const char *filename ) { - struct file *file; - int fd; + FILE *f;
- if (!(file = get_file_obj( current->process, handle, FILE_READ_DATA ))) return; - fd = dup( get_file_unix_fd( file ) ); - release_object( file ); - if (fd != -1) + f = fopen( filename, "r" ); + if (f) { - FILE *f = fdopen( fd, "r" ); - if (f) - { - load_keys( key, NULL, f, -1 ); - fclose( f ); - } - else file_set_error(); + load_keys( key, NULL, f, -1 ); + fclose( f ); } + else file_set_error(); }
/* load one of the initial registry files */ @@ -2280,8 +2273,12 @@ DECL_HANDLER(load_registry) { struct key *key, *parent = NULL; struct unicode_str name; + const char *req_data; + data_size_t req_data_len; + char *filename; const struct security_descriptor *sd; const struct object_attributes *objattr = get_req_object_attributes( &sd, &name, NULL ); + req_data = get_req_data_after_objattr( objattr, &req_data_len );
if (!objattr) return;
@@ -2297,8 +2294,18 @@ DECL_HANDLER(load_registry)
if ((key = create_key( parent, &name, 0, KEY_WOW64_64KEY, 0, sd ))) { - load_registry( key, req->file ); + filename = malloc( req_data_len + 1 ); + if (!filename) { + set_error( STATUS_NO_MEMORY ); + return; + } + memcpy( filename, req_data, req_data_len ); + filename[req_data_len] = 0; + + load_registry( key, filename ); release_object( key ); + + free( filename ); } if (parent) release_object( parent ); } diff --git a/server/request.h b/server/request.h index 9e943cceb3c..d14cede98be 100644 --- a/server/request.h +++ b/server/request.h @@ -1230,7 +1230,6 @@ C_ASSERT( FIELD_OFFSET(struct enum_key_value_reply, namelen) == 16 ); C_ASSERT( sizeof(struct enum_key_value_reply) == 24 ); C_ASSERT( FIELD_OFFSET(struct delete_key_value_request, hkey) == 12 ); C_ASSERT( sizeof(struct delete_key_value_request) == 16 ); -C_ASSERT( FIELD_OFFSET(struct load_registry_request, file) == 12 ); C_ASSERT( sizeof(struct load_registry_request) == 16 ); C_ASSERT( FIELD_OFFSET(struct unload_registry_request, parent) == 12 ); C_ASSERT( FIELD_OFFSET(struct unload_registry_request, attributes) == 16 ); diff --git a/server/trace.c b/server/trace.c index c6a324bb905..5765ab40bd8 100644 --- a/server/trace.c +++ b/server/trace.c @@ -2406,8 +2406,8 @@ static void dump_delete_key_value_request( const struct delete_key_value_request
static void dump_load_registry_request( const struct load_registry_request *req ) { - fprintf( stderr, " file=%04x", req->file ); - dump_varargs_object_attributes( ", objattr=", cur_size ); + dump_varargs_object_attributes( " objattr=", cur_size ); + dump_varargs_string( ", filename=", cur_size ); }
static void dump_unload_registry_request( const struct unload_registry_request *req )
From: Santino Mazza smazza@codeweavers.com
--- dlls/ntdll/unix/registry.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/dlls/ntdll/unix/registry.c b/dlls/ntdll/unix/registry.c index a15260d6713..f434c380834 100644 --- a/dlls/ntdll/unix/registry.c +++ b/dlls/ntdll/unix/registry.c @@ -696,7 +696,7 @@ NTSTATUS WINAPI NtLoadKey2( const OBJECT_ATTRIBUTES *attr, OBJECT_ATTRIBUTES *fi /****************************************************************************** * NtLoadKeyEx (NTDLL.@) */ -NTSTATUS WINAPI NtLoadKeyEx( const OBJECT_ATTRIBUTES *attr, OBJECT_ATTRIBUTES *file, ULONG flags, HANDLE trustkey, +NTSTATUS WINAPI NtLoadKeyEx( const OBJECT_ATTRIBUTES *key_attr, OBJECT_ATTRIBUTES *file_attr, ULONG flags, HANDLE trustkey, HANDLE event, ACCESS_MASK access, HANDLE *roothandle, IO_STATUS_BLOCK *iostatus ) { NTSTATUS ret; @@ -704,9 +704,9 @@ NTSTATUS WINAPI NtLoadKeyEx( const OBJECT_ATTRIBUTES *attr, OBJECT_ATTRIBUTES *f struct object_attributes *objattr; char *unix_name; UNICODE_STRING nt_name; - OBJECT_ATTRIBUTES new_attr = *file; + OBJECT_ATTRIBUTES new_attr = *file_attr;
- TRACE( "(%p,%p,0x%x,%p,%p,0x%x,%p,%p)\n", attr, file, flags, trustkey, event, access, roothandle, iostatus ); + TRACE( "(%p,%p,0x%x,%p,%p,0x%x,%p,%p)\n", key_attr, file_attr, flags, trustkey, event, access, roothandle, iostatus );
if (flags) FIXME( "flags %x not handled\n", flags ); if (trustkey) FIXME("trustkey parameter not supported\n"); @@ -721,7 +721,7 @@ NTSTATUS WINAPI NtLoadKeyEx( const OBJECT_ATTRIBUTES *attr, OBJECT_ATTRIBUTES *f
if (ret) return ret;
- if ((ret = alloc_object_attributes( attr, &objattr, &len ))) + if ((ret = alloc_object_attributes( key_attr, &objattr, &len ))) { free( unix_name ); return ret;
From: Santino Mazza smazza@codeweavers.com
--- dlls/ntdll/tests/reg.c | 2 +- server/registry.c | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/dlls/ntdll/tests/reg.c b/dlls/ntdll/tests/reg.c index c7d9d7e4c7f..b618d879e57 100644 --- a/dlls/ntdll/tests/reg.c +++ b/dlls/ntdll/tests/reg.c @@ -2361,7 +2361,7 @@ static void test_NtRegLoadKeyEx(void) /* Test for roothandle parameter with REG_APP_HIVE */ RtlCreateUnicodeString(&key_pathW, L"\REGISTRY\A\TestKey"); status = pNtLoadKeyEx(&key_attr, &file_attr, REG_APP_HIVE, NULL, NULL, KEY_READ, &key, NULL); - todo_wine ok(status == STATUS_SUCCESS, "got 0x%lx\n", status); + ok(status == STATUS_SUCCESS, "got 0x%lx\n", status); todo_wine ok(key != NULL, "key is null\n"); if (key) pNtClose(key); RtlFreeUnicodeString(&key_pathW); diff --git a/server/registry.c b/server/registry.c index 3a83b31a3b4..2aeb93141d9 100644 --- a/server/registry.c +++ b/server/registry.c @@ -1846,10 +1846,13 @@ void init_registry(void) 'C','u','r','r','e','n','t','V','e','r','s','i','o','n','\', 'P','e','r','f','l','i','b','\', '0','0','9'}; + static const WCHAR application[] = {'A'}; + static const struct unicode_str root_name = { REGISTRY, sizeof(REGISTRY) }; static const struct unicode_str HKLM_name = { HKLM, sizeof(HKLM) }; static const struct unicode_str HKU_name = { HKU_default, sizeof(HKU_default) }; static const struct unicode_str perflib_name = { perflib, sizeof(perflib) }; + static const struct unicode_str application_name = { application, sizeof(application) };
WCHAR *current_user_path; struct unicode_str current_user_str; @@ -1927,6 +1930,11 @@ void init_registry(void) release_object( key ); }
+ /* create application hive */ + if (!(key = create_key_recursive( root_key, &application_name, current_time ))) + fatal_error( "could not create \REGISTRY\A registry key\n" ); + release_object( key ); + release_object( hklm ); release_object( hkcu );
From: Santino Mazza smazza@codeweavers.com
--- dlls/ntdll/tests/reg.c | 4 +-- dlls/ntdll/unix/registry.c | 12 +++++-- include/wine/server_protocol.h | 8 +++-- server/protocol.def | 4 +++ server/registry.c | 66 +++++++++++++++++++++++++++------- server/request.h | 6 +++- server/trace.c | 11 ++++-- 7 files changed, 89 insertions(+), 22 deletions(-)
diff --git a/dlls/ntdll/tests/reg.c b/dlls/ntdll/tests/reg.c index b618d879e57..a328d8e3cda 100644 --- a/dlls/ntdll/tests/reg.c +++ b/dlls/ntdll/tests/reg.c @@ -2345,7 +2345,7 @@ static void test_NtRegLoadKeyEx(void) InitializeObjectAttributes(&file_attr, &hivefile_pathW, 0, NULL, NULL); key_attr.ObjectName = &key_pathW; status = pNtLoadKeyEx(&key_attr, &file_attr, 0, NULL, NULL, KEY_READ, &key, NULL); - todo_wine ok(status == STATUS_INVALID_PARAMETER_7 || broken(status == STATUS_INVALID_PARAMETER_6) /* win7 */, "got 0x%lx\n", status); + ok(status == STATUS_INVALID_PARAMETER_7 || broken(status == STATUS_INVALID_PARAMETER_6) /* win7 */, "got 0x%lx\n", status); if (status == STATUS_INVALID_PARAMETER_6) { win_skip("NtLoadKeyEx has a different order of parameters in this windows version\n"); @@ -2362,7 +2362,7 @@ static void test_NtRegLoadKeyEx(void) RtlCreateUnicodeString(&key_pathW, L"\REGISTRY\A\TestKey"); status = pNtLoadKeyEx(&key_attr, &file_attr, REG_APP_HIVE, NULL, NULL, KEY_READ, &key, NULL); ok(status == STATUS_SUCCESS, "got 0x%lx\n", status); - todo_wine ok(key != NULL, "key is null\n"); + ok(key != NULL, "key is null\n"); if (key) pNtClose(key); RtlFreeUnicodeString(&key_pathW);
diff --git a/dlls/ntdll/unix/registry.c b/dlls/ntdll/unix/registry.c index f434c380834..df9915a4849 100644 --- a/dlls/ntdll/unix/registry.c +++ b/dlls/ntdll/unix/registry.c @@ -700,6 +700,7 @@ NTSTATUS WINAPI NtLoadKeyEx( const OBJECT_ATTRIBUTES *key_attr, OBJECT_ATTRIBUTE HANDLE event, ACCESS_MASK access, HANDLE *roothandle, IO_STATUS_BLOCK *iostatus ) { NTSTATUS ret; + HANDLE key; data_size_t len; struct object_attributes *objattr; char *unix_name; @@ -708,13 +709,13 @@ NTSTATUS WINAPI NtLoadKeyEx( const OBJECT_ATTRIBUTES *key_attr, OBJECT_ATTRIBUTE
TRACE( "(%p,%p,0x%x,%p,%p,0x%x,%p,%p)\n", key_attr, file_attr, flags, trustkey, event, access, roothandle, iostatus );
- if (flags) FIXME( "flags %x not handled\n", flags ); + if (flags && (flags & ~REG_APP_HIVE)) FIXME( "flags %x not handled\n", flags ); if (trustkey) FIXME("trustkey parameter not supported\n"); if (event) FIXME("event parameter not supported\n"); - if (access) FIXME("access parameter not supported\n"); - if (roothandle) FIXME("roothandle is not filled\n"); if (iostatus) FIXME("iostatus is not filled\n");
+ if (roothandle && !(flags & REG_APP_HIVE)) return STATUS_INVALID_PARAMETER_7; + get_redirect( &new_attr, &nt_name ); ret = nt_to_unix_file_name( &new_attr, &unix_name, FILE_OPEN ); free( nt_name.Buffer ); @@ -730,13 +731,18 @@ NTSTATUS WINAPI NtLoadKeyEx( const OBJECT_ATTRIBUTES *key_attr, OBJECT_ATTRIBUTE
SERVER_START_REQ( load_registry ) { + req->flags = flags; + req->access = access; wine_server_add_data( req, objattr, len ); wine_server_add_data( req, unix_name, strlen( unix_name ) ); ret = wine_server_call( req ); + key = wine_server_ptr_handle( reply->hkey ); if (ret == STATUS_OBJECT_NAME_EXISTS) ret = STATUS_SUCCESS; } SERVER_END_REQ;
+ if (roothandle) *roothandle = key; + else NtClose( key ); free( objattr ); free( unix_name ); return ret; diff --git a/include/wine/server_protocol.h b/include/wine/server_protocol.h index 080fe4bb6ec..36fdd241499 100644 --- a/include/wine/server_protocol.h +++ b/include/wine/server_protocol.h @@ -2360,13 +2360,17 @@ struct delete_key_value_reply struct load_registry_request { struct request_header __header; + unsigned int flags; + unsigned int access; /* VARARG(objattr,object_attributes); */ /* VARARG(filename,string); */ - char __pad_12[4]; + char __pad_20[4]; }; struct load_registry_reply { struct reply_header __header; + obj_handle_t hkey; + char __pad_12[4]; };
@@ -6325,7 +6329,7 @@ union generic_reply
/* ### protocol_version begin ### */
-#define SERVER_PROTOCOL_VERSION 757 +#define SERVER_PROTOCOL_VERSION 758
/* ### protocol_version end ### */
diff --git a/server/protocol.def b/server/protocol.def index 9c2a1af35c4..4e0c4b905db 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -1825,8 +1825,12 @@ struct process_info
/* Load a registry branch from a file */ @REQ(load_registry) + unsigned int flags; /* flags */ + unsigned int access; /* wanted access rights */ VARARG(objattr,object_attributes); /* object attributes */ VARARG(filename,string); /* file to load name */ +@REPLY + obj_handle_t hkey; /* handle to root key */ @END
diff --git a/server/registry.c b/server/registry.c index 2aeb93141d9..a4020a301aa 100644 --- a/server/registry.c +++ b/server/registry.c @@ -90,6 +90,7 @@ struct key unsigned int flags; /* flags */ timeout_t modif; /* last modification time */ struct list notify_list; /* list of notifications */ + FILE *file; /* loaded file */ };
/* key flags */ @@ -637,15 +638,6 @@ static void key_unlink_name( struct object *obj, struct object_name *name ) } }
-/* close the notification associated with a handle */ -static int key_close_handle( struct object *obj, struct process *process, obj_handle_t handle ) -{ - struct key * key = (struct key *) obj; - struct notify *notify = find_notify( key, process, handle ); - if (notify) do_notification( key, notify, 1 ); - return 1; /* ok to close */ -} - static void key_destroy( struct object *obj ) { int i; @@ -699,6 +691,7 @@ static struct key *create_key_object( struct object *parent, const struct unicod key->last_value = -1; key->values = NULL; key->modif = modif; + key->file = NULL; list_init( &key->notify_list );
if (options & REG_OPTION_CREATE_LINK) key->flags |= KEY_SYMLINK; @@ -1760,6 +1753,30 @@ static void load_registry( struct key *key, const char *filename ) else file_set_error(); }
+static void load_app_registry( struct key *key, const char *filename ) +{ + WCHAR applicationhive_fullpath[12] = {'\', 'R', 'E', 'G', 'I', 'S', 'T', 'R', 'Y', '\', 'A'}; + WCHAR *key_fullpath; + data_size_t key_fullpath_size; + + /* check if we are loading in \REGISTRY\A */ + key_fullpath = key_get_full_name( (struct object*)key, &key_fullpath_size ); + if (key_fullpath_size < (sizeof(applicationhive_fullpath) - 2 ) || + memcmp( key_fullpath, applicationhive_fullpath, sizeof(applicationhive_fullpath) - 2 )) + { + set_error( STATUS_PRIVILEGE_NOT_HELD ); + } + free( key_fullpath ); + + if (get_error()) return; + + load_registry( key, filename ); + + key->file = fopen( filename, "r+" ); + if (!key->file) + file_set_error(); +} + /* load one of the initial registry files */ static int load_init_registry_from_file( const char *filename, struct key *key ) { @@ -2120,6 +2137,21 @@ void flush_registry(void) if (fchdir( server_dir_fd ) == -1) fatal_error( "chdir to server dir: %s\n", strerror( errno )); }
+/* close the notification associated with a handle */ +static int key_close_handle( struct object *obj, struct process *process, obj_handle_t handle ) +{ + struct key * key = (struct key *) obj; + struct notify *notify = find_notify( key, process, handle ); + if (notify) do_notification( key, notify, 1 ); + if (key->file) + { + save_all_subkeys( key, key->file ); + if (fclose( key->file )) file_set_error(); + delete_key( key, 1 ); + } + return 1; /* ok to close */ +} + /* determine if the thread is wow64 (32-bit client running on 64-bit prefix) */ static int is_wow64_thread( struct thread *thread ) { @@ -2290,7 +2322,7 @@ DECL_HANDLER(load_registry)
if (!objattr) return;
- if (!thread_single_check_privilege( current, SeRestorePrivilege )) + if (!(req->flags & REG_APP_HIVE) && !thread_single_check_privilege( current, SeRestorePrivilege )) { set_error( STATUS_PRIVILEGE_NOT_HELD ); return; @@ -2310,9 +2342,19 @@ DECL_HANDLER(load_registry) memcpy( filename, req_data, req_data_len ); filename[req_data_len] = 0;
- load_registry( key, filename ); - release_object( key ); + if (req->flags & REG_APP_HIVE) + { + load_app_registry( key, filename ); + if (!get_error()) + reply->hkey = alloc_handle( current->process, key, req->access, objattr->attributes ); + } + else + load_registry( key, filename );
+ if (get_error()) + delete_key(key, 0); + + release_object( key ); free( filename ); } if (parent) release_object( parent ); diff --git a/server/request.h b/server/request.h index d14cede98be..faf66287b60 100644 --- a/server/request.h +++ b/server/request.h @@ -1230,7 +1230,11 @@ C_ASSERT( FIELD_OFFSET(struct enum_key_value_reply, namelen) == 16 ); C_ASSERT( sizeof(struct enum_key_value_reply) == 24 ); C_ASSERT( FIELD_OFFSET(struct delete_key_value_request, hkey) == 12 ); C_ASSERT( sizeof(struct delete_key_value_request) == 16 ); -C_ASSERT( sizeof(struct load_registry_request) == 16 ); +C_ASSERT( FIELD_OFFSET(struct load_registry_request, flags) == 12 ); +C_ASSERT( FIELD_OFFSET(struct load_registry_request, access) == 16 ); +C_ASSERT( sizeof(struct load_registry_request) == 24 ); +C_ASSERT( FIELD_OFFSET(struct load_registry_reply, hkey) == 8 ); +C_ASSERT( sizeof(struct load_registry_reply) == 16 ); C_ASSERT( FIELD_OFFSET(struct unload_registry_request, parent) == 12 ); C_ASSERT( FIELD_OFFSET(struct unload_registry_request, attributes) == 16 ); C_ASSERT( sizeof(struct unload_registry_request) == 24 ); diff --git a/server/trace.c b/server/trace.c index 5765ab40bd8..97435dc7493 100644 --- a/server/trace.c +++ b/server/trace.c @@ -2406,10 +2406,17 @@ static void dump_delete_key_value_request( const struct delete_key_value_request
static void dump_load_registry_request( const struct load_registry_request *req ) { - dump_varargs_object_attributes( " objattr=", cur_size ); + fprintf( stderr, " flags=%08x", req->flags ); + fprintf( stderr, ", access=%08x", req->access ); + dump_varargs_object_attributes( ", objattr=", cur_size ); dump_varargs_string( ", filename=", cur_size ); }
+static void dump_load_registry_reply( const struct load_registry_reply *req ) +{ + fprintf( stderr, " hkey=%04x", req->hkey ); +} + static void dump_unload_registry_request( const struct unload_registry_request *req ) { fprintf( stderr, " parent=%04x", req->parent ); @@ -4869,7 +4876,7 @@ static const dump_func reply_dumpers[REQ_NB_REQUESTS] = { (dump_func)dump_get_key_value_reply, (dump_func)dump_enum_key_value_reply, NULL, - NULL, + (dump_func)dump_load_registry_reply, NULL, NULL, NULL,
From: Santino Mazza smazza@codeweavers.com
--- dlls/advapi32/tests/registry.c | 6 +-- dlls/kernelbase/registry.c | 71 ++++++++++++++++++++++++++++------ 2 files changed, 62 insertions(+), 15 deletions(-)
diff --git a/dlls/advapi32/tests/registry.c b/dlls/advapi32/tests/registry.c index 55a8074f1b7..e2024ec8899 100644 --- a/dlls/advapi32/tests/registry.c +++ b/dlls/advapi32/tests/registry.c @@ -1652,7 +1652,7 @@ static void test_reg_load_app_key(void) ok(appkey != NULL, "got a null key\n");
ret = RegSetValueExA(appkey, "testkey", 0, REG_BINARY, test_data, sizeof(test_data)); - todo_wine ok(ret == ERROR_SUCCESS, "couldn't set key value %lx\n", ret); + ok(ret == ERROR_SUCCESS, "couldn't set key value %lx\n", ret); RegCloseKey(appkey);
wait_file_available(hivefilepath); @@ -1665,9 +1665,9 @@ static void test_reg_load_app_key(void) size = sizeof(test_data); memset(output, 0xff, sizeof(output)); ret = RegGetValueA(appkey, NULL, "testkey", RRF_RT_REG_BINARY, NULL, output, &size); - todo_wine ok(ret == ERROR_SUCCESS, "expected ERROR_SUCCESS, got %ld\n", ret); + ok(ret == ERROR_SUCCESS, "expected ERROR_SUCCESS, got %ld\n", ret); ok(size == sizeof(test_data), "size doesn't match %ld != %ld\n", size, (DWORD)sizeof(test_data)); - todo_wine ok(!memcmp(test_data, output, sizeof(test_data)), "output is not what expected\n"); + ok(!memcmp(test_data, output, sizeof(test_data)), "output is not what expected\n");
RegCloseKey(appkey);
diff --git a/dlls/kernelbase/registry.c b/dlls/kernelbase/registry.c index 91462d80e06..ac4dd97776a 100644 --- a/dlls/kernelbase/registry.c +++ b/dlls/kernelbase/registry.c @@ -3080,35 +3080,82 @@ cleanup: return ret; }
+static void generate_string_uuid(WCHAR *out, DWORD out_size) +{ + UUID uuid; + ULONG *ptr = (ULONG*)&uuid; + LARGE_INTEGER ft; + ULONG seed; + int i; + + NtQuerySystemTime(&ft); + seed = ft.LowPart; + for (i = 0; i < sizeof(UUID)/sizeof(ULONG); i++, ptr++) + *ptr = RtlUniform(&seed); + + uuid.Data3 &= 0x0fff; + uuid.Data3 |= (4 << 12); + uuid.Data4[0] &= 0x3f; + uuid.Data4[0] |= 0x80; + + swprintf(out, out_size, L"{%08X-%04X-%04X-%02X%02X-%02X%02X%02X%02X%02X%02X}", uuid.Data1, + uuid.Data2, uuid.Data3, uuid.Data4[0], uuid.Data4[1], uuid.Data4[2], uuid.Data4[3], + uuid.Data4[4], uuid.Data4[5], uuid.Data4[6], uuid.Data4[7]); +}
/****************************************************************************** * RegLoadAppKeyA (kernelbase.@) * */ -LSTATUS WINAPI RegLoadAppKeyA(const char *file, HKEY *result, REGSAM sam, DWORD options, DWORD reserved) +LSTATUS WINAPI RegLoadAppKeyA(const char *filename, HKEY *result, REGSAM sam, DWORD options, DWORD reserved) { - FIXME("%s %p %lu %lu %lu: stub\n", wine_dbgstr_a(file), result, sam, options, reserved); - - if (!file || reserved) - return ERROR_INVALID_PARAMETER; + UNICODE_STRING filenameW; + LSTATUS status; + TRACE("%s %p %lu %lu %lu\n", wine_dbgstr_a(filename), result, sam, options, reserved);
- *result = (HKEY)0xdeadbeef; - return ERROR_SUCCESS; + RtlCreateUnicodeStringFromAsciiz(&filenameW, filename); + status = RegLoadAppKeyW(filenameW.Buffer, result, sam, options, reserved); + RtlFreeUnicodeString(&filenameW); + return status; }
/****************************************************************************** * RegLoadAppKeyW (kernelbase.@) * */ -LSTATUS WINAPI RegLoadAppKeyW(const WCHAR *file, HKEY *result, REGSAM sam, DWORD options, DWORD reserved) +LSTATUS WINAPI RegLoadAppKeyW(const WCHAR *filename, HKEY *result, REGSAM sam, DWORD options, DWORD reserved) { - FIXME("%s %p %lu %lu %lu: stub\n", wine_dbgstr_w(file), result, sam, options, reserved); + NTSTATUS status; + WCHAR application_root[13] = L"\REGISTRY\A\"; + WCHAR rootguid_str[39]; + WCHAR *destkey_path_tmp; + UNICODE_STRING destkey_path, filenameW; + OBJECT_ATTRIBUTES destkey, file;
- if (!file || reserved) + TRACE("%s %p %lu %lu %lu\n", wine_dbgstr_w(filename), result, sam, options, reserved); + + if (!filename || reserved) return ERROR_INVALID_PARAMETER;
- *result = (HKEY)0xdeadbeef; - return ERROR_SUCCESS; + InitializeObjectAttributes(&destkey, &destkey_path, 0, 0, 0); + RtlDosPathNameToNtPathName_U(filename, &filenameW, NULL, NULL); + InitializeObjectAttributes(&file, &filenameW, 0, 0, 0); + + generate_string_uuid(rootguid_str, sizeof(rootguid_str)); + + destkey_path_tmp = heap_alloc_zero(sizeof(application_root) + sizeof(rootguid_str)); + wcscat(destkey_path_tmp, application_root); + wcscat(destkey_path_tmp, rootguid_str); + + RtlCreateUnicodeString(&destkey_path, destkey_path_tmp); + heap_free(destkey_path_tmp); + + status = NtLoadKeyEx(&destkey, &file, REG_APP_HIVE, 0, 0, sam, (HANDLE *)result, 0); + + RtlFreeUnicodeString(&destkey_path); + RtlFreeUnicodeString(&filenameW); + + return RtlNtStatusToDosError(status); }
Huw Davies (@huw) commented about server/registry.c:
if ((key = create_key( parent, &name, 0, KEY_WOW64_64KEY, 0, sd ))) {
load_registry( key, req->file );
filename = malloc( req_data_len + 1 );if (!filename) {set_error( STATUS_NO_MEMORY );return;}memcpy( filename, req_data, req_data_len );filename[req_data_len] = 0;load_registry( key, filename ); release_object( key );free( filename );
You're leaking `key` and potentially `parent` in the `malloc()` failure path. It would also be conventional to free `filename` before releasing `key` (i.e. in the opposite order to which they were acquired. Something like:
```suggestion:-11+0 filename = malloc( req_data_len + 1 ); if (filename) { memcpy( filename, req_data, req_data_len ); filename[req_data_len] = 0; load_registry( key, filename ); free( filename ); } else set_error( STATUS_NO_MEMORY ); release_object( key ); ```
Huw Davies (@huw) commented about server/registry.c:
else file_set_error();}
+static void load_app_registry( struct key *key, const char *filename ) +{
- WCHAR applicationhive_fullpath[12] = {'\', 'R', 'E', 'G', 'I', 'S', 'T', 'R', 'Y', '\', 'A'};
Leave this array unsized and remove the `- 2`s from the `sizeof()`s below.
Huw Davies (@huw) commented about server/registry.c:
else file_set_error();}
+static void load_app_registry( struct key *key, const char *filename ) +{
- WCHAR applicationhive_fullpath[12] = {'\', 'R', 'E', 'G', 'I', 'S', 'T', 'R', 'Y', '\', 'A'};
- WCHAR *key_fullpath;
- data_size_t key_fullpath_size;
- /* check if we are loading in \REGISTRY\A */
- key_fullpath = key_get_full_name( (struct object*)key, &key_fullpath_size );
```suggestion:-0+0 key_fullpath = key_get_full_name( &key->obj, &key_fullpath_size ); ```
Huw Davies (@huw) commented about server/registry.c:
memcpy( filename, req_data, req_data_len ); filename[req_data_len] = 0;
load_registry( key, filename );release_object( key );
if (req->flags & REG_APP_HIVE){load_app_registry( key, filename );if (!get_error())reply->hkey = alloc_handle( current->process, key, req->access, objattr->attributes );}elseload_registry( key, filename );if (get_error())delete_key(key, 0);
This is changing the non `REG_APP_HIVE` path too.
Huw Davies (@huw) commented about server/registry.c:
memcpy( filename, req_data, req_data_len ); filename[req_data_len] = 0;
load_registry( key, filename );release_object( key );
if (req->flags & REG_APP_HIVE){load_app_registry( key, filename );if (!get_error())
It may be cleaner to have `load_app_registry()` return the error.
On Fri Sep 23 10:07:31 2022 +0000, Huw Davies wrote:
Leave this array unsized and remove the `- 2`s from the `sizeof()`s below.
Also make it `static const`
On Fri Sep 23 10:07:32 2022 +0000, Huw Davies wrote:
This is changing the non `REG_APP_HIVE` path too.
You mean the path of the key? I already check that inside `load_app_registry` function.
On Fri Sep 23 11:59:57 2022 +0000, Santino Mazza wrote:
You mean the path of the key? I already check that inside `load_app_registry` function.
I mean that `if (get_error()) delete_key(key, 0)` is now being executed in the non-`REG_APP_HIVE` case - it didn't used to be.
On Fri Sep 23 12:03:09 2022 +0000, Huw Davies wrote:
I mean that `if (get_error()) delete_key(key, 0)` is now being executed in the non-`REG_APP_HIVE` case - it didn't used to be.
Oh yeah, that's expected, if the load fails it should delete the key.
On Fri Sep 23 10:07:32 2022 +0000, Huw Davies wrote:
It may be cleaner to have `load_app_registry()` return the error.
mmm not sure, I think I'll have to make a bigger refactor to actually get the code cleaner by returning the error. For example when I use `if (get_error()) delete_key(key, 0)`, I will have to repeat code for the load_app_registry case or modify load_registry and the functions it calls to also return the error. I think another option could be to use `set_error` and also return the error.
On Fri Sep 23 12:05:20 2022 +0000, Santino Mazza wrote:
Oh yeah, that's expected, if the load fails it should delete the key.
If it needs to happen in the non-`REG_APP_HIVE` case then that bit needs to be a separate commit, probably a separate MR, with a test.
On Fri Sep 23 12:11:08 2022 +0000, Santino Mazza wrote:
mmm not sure, I think I'll have to make a bigger refactor to actually get the code cleaner by returning the error. For example when I use `if (get_error()) delete_key(key, 0)`, I will have to repeat code for the load_app_registry case or modify load_registry and the functions it calls to also return the error. I think another option could be to use `set_error` and also return the error.
I've been reading other requests implementations, and I think it will be better to pass load_app_registry a pointer to `reply->hkey` and do all the stuff in there, so we don't have to return an error and also don't have to make use of get_error.
-
Huw Davies (＠huw) -
Santino Mazza
-
Santino Mazza (＠tati1454)