[PATCH 0/8] MR6709: Code refactor reduced variable inner scope
Micro-optimization works only in Debug build, therefore, it is more correct to name this code refactoring, in my GCC with -O2 flag it does not affect count and decrease cost per CPU clock cycle instructions generated in any way. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6709
From: Semenov Herman (Семенов Герман)<GermanAizek(a)yandex.ru> --- server/atom.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/server/atom.c b/server/atom.c index ff0799f5880..b69eac6d465 100644 --- a/server/atom.c +++ b/server/atom.c @@ -166,14 +166,13 @@ static atom_t add_atom_entry( struct atom_table *table, struct atom_entry *entry /* dump an atom table */ static void atom_table_dump( struct object *obj, int verbose ) { - int i; struct atom_table *table = (struct atom_table *)obj; assert( obj->ops == &atom_table_ops ); fprintf( stderr, "Atom table size=%d entries=%d\n", table->last + 1, table->entries_count ); if (!verbose) return; - for (i = 0; i <= table->last; i++) + for (int i = 0; i <= table->last; i++) { struct atom_entry *entry = table->handles[i]; if (!entry) continue; @@ -187,12 +186,11 @@ static void atom_table_dump( struct object *obj, int verbose ) /* destroy the atom table */ static void atom_table_destroy( struct object *obj ) { - int i; struct atom_table *table = (struct atom_table *)obj; assert( obj->ops == &atom_table_ops ); if (table->handles) { - for (i = 0; i <= table->last; i++) free( table->handles[i] ); + for (int i = 0; i <= table->last; i++) free( table->handles[i] ); free( table->handles ); } free( table->entries ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6709
From: Semenov Herman (Семенов Герман)<GermanAizek(a)yandex.ru> --- programs/wusa/main.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/programs/wusa/main.c b/programs/wusa/main.c index ef7302847cc..4584f4e3a89 100644 --- a/programs/wusa/main.c +++ b/programs/wusa/main.c @@ -451,17 +451,14 @@ static void strbuf_free(struct strbuf *buf) static BOOL strbuf_append(struct strbuf *buf, const WCHAR *str, DWORD len) { - DWORD new_len; - WCHAR *new_buf; - if (!buf->buf) return FALSE; if (!str) return TRUE; if (len == ~0U) len = lstrlenW(str); if (buf->pos + len + 1 > buf->len) { - new_len = max(buf->pos + len + 1, buf->len * 2); - new_buf = realloc(buf->buf, new_len * sizeof(WCHAR)); + DWORD new_len = max(buf->pos + len + 1, buf->len * 2); + WCHAR *new_buf = realloc(buf->buf, new_len * sizeof(WCHAR)); if (!new_buf) { strbuf_free(buf); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6709
From: Semenov Herman (Семенов Герман)<GermanAizek(a)yandex.ru> --- programs/msidb/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/msidb/main.c b/programs/msidb/main.c index 8570b1f9f3d..05ad4e8b7a3 100644 --- a/programs/msidb/main.c +++ b/programs/msidb/main.c @@ -445,7 +445,6 @@ static int import_tables( struct msidb_state *state ) LIST_FOR_EACH_ENTRY( data, &state->table_list, struct msidb_listentry, entry ) { WCHAR *table_name = data->name; - WCHAR table_path[MAX_PATH]; WCHAR *ext; /* permit specifying tables with wildcards ('Feature*') */ @@ -487,6 +486,7 @@ static int import_tables( struct msidb_state *state ) if ((ext = PathFindExtensionW( table_name )) == NULL || lstrcmpW( ext, L".idt" ) != 0) { /* truncate to 8 characters */ + WCHAR table_path[MAX_PATH]; swprintf( table_path, ARRAY_SIZE(table_path), L"%.8s.idt", table_name ); table_name = table_path; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6709
From: Semenov Herman (Семенов Герман)<GermanAizek(a)yandex.ru> --- dlls/wsdapi/soap.c | 8 +++----- dlls/wsdapi/xml.c | 10 +++------- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/dlls/wsdapi/soap.c b/dlls/wsdapi/soap.c index a2753c943a7..f114d290976 100644 --- a/dlls/wsdapi/soap.c +++ b/dlls/wsdapi/soap.c @@ -361,7 +361,7 @@ static LPWSTR ulonglong_to_string(void *parent, ULONGLONG value) static WSDXML_ATTRIBUTE *add_attribute(IWSDXMLContext *xml_context, WSDXML_ELEMENT *parent, LPCWSTR ns_uri, LPCWSTR name) { - WSDXML_ATTRIBUTE *attribute, *cur_attrib; + WSDXML_ATTRIBUTE *attribute; WSDXML_NAME *name_obj = NULL; if (ns_uri == NULL) @@ -400,7 +400,7 @@ static WSDXML_ATTRIBUTE *add_attribute(IWSDXMLContext *xml_context, WSDXML_ELEME else { /* Find the last attribute and add this as the next one */ - cur_attrib = parent->FirstAttribute; + WSDXML_ATTRIBUTE *cur_attrib = parent->FirstAttribute; while (cur_attrib->Next != NULL) { @@ -1497,8 +1497,6 @@ static WSDXML_ELEMENT *find_element(WSDXML_ELEMENT *parent, LPCWSTR name, LPCWST static void remove_element(WSDXML_ELEMENT *element) { - WSDXML_NODE *cur; - if (element == NULL) return; @@ -1506,7 +1504,7 @@ static void remove_element(WSDXML_ELEMENT *element) element->Node.Parent->FirstChild = element->Node.Next; else { - cur = element->Node.Parent->FirstChild; + WSDXML_NODE *cur = element->Node.Parent->FirstChild; while (cur != NULL) { diff --git a/dlls/wsdapi/xml.c b/dlls/wsdapi/xml.c index d92582718dd..56b878cf2e1 100644 --- a/dlls/wsdapi/xml.c +++ b/dlls/wsdapi/xml.c @@ -162,8 +162,6 @@ HRESULT WINAPI WSDXMLAddSibling(WSDXML_ELEMENT *pFirst, WSDXML_ELEMENT *pSecond) HRESULT WINAPI WSDXMLBuildAnyForSingleElement(WSDXML_NAME *pElementName, LPCWSTR pszText, WSDXML_ELEMENT **ppAny) { - WSDXML_TEXT *child; - TRACE("(%p, %s, %p)\n", pElementName, debugstr_w(pszText), ppAny); if ((pElementName == NULL) || ((pszText != NULL) && (lstrlenW(pszText) > WSD_MAX_TEXT_LENGTH))) @@ -195,7 +193,7 @@ HRESULT WINAPI WSDXMLBuildAnyForSingleElement(WSDXML_NAME *pElementName, LPCWSTR if (pszText != NULL) { - child = WSDAllocateLinkedMemory(*ppAny, sizeof(WSDXML_TEXT)); + WSDXML_TEXT *child = WSDAllocateLinkedMemory(*ppAny, sizeof(WSDXML_TEXT)); if (child == NULL) { @@ -324,7 +322,6 @@ static WSDXML_NAME *add_name(WSDXML_NAMESPACE *ns, LPCWSTR name) { WSDXML_NAME *names; WSDXML_NAME *newName; - int i; names = WSDAllocateLinkedMemory(ns, sizeof(WSDXML_NAME) * (ns->NamesCount + 1)); @@ -338,7 +335,7 @@ static WSDXML_NAME *add_name(WSDXML_NAMESPACE *ns, LPCWSTR name) /* Copy the existing names array over to the new allocation */ memcpy(names, ns->Names, sizeof(WSDXML_NAME) * ns->NamesCount); - for (i = 0; i < ns->NamesCount; i++) + for (int i = 0; i < ns->NamesCount; i++) { /* Attach the local name memory to the new names allocation */ WSDAttachLinkedMemory(names, names[i].LocalName); @@ -380,11 +377,10 @@ static BOOL is_prefix_unique(struct list *namespaces, LPCWSTR prefix) static LPWSTR generate_namespace_prefix(IWSDXMLContextImpl *impl, void *parentMemoryBlock, LPCWSTR uri) { - WCHAR suggestedPrefix[7]; - /* Find a unique prefix */ while (impl->nextUnknownPrefix < 1000) { + WCHAR suggestedPrefix[7]; wsprintfW(suggestedPrefix, L"un%d", impl->nextUnknownPrefix++); /* For the unlikely event where somebody has explicitly created a prefix called 'unX', check it is unique */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6709
From: Semenov Herman (Семенов Герман)<GermanAizek(a)yandex.ru> --- dlls/winhttp/net.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/dlls/winhttp/net.c b/dlls/winhttp/net.c index e46f2023fae..fb32a2d6e30 100644 --- a/dlls/winhttp/net.c +++ b/dlls/winhttp/net.c @@ -472,8 +472,6 @@ static DWORD send_ssl_chunk( struct netconn *conn, const void *msg, size_t size, DWORD netconn_send( struct netconn *conn, const void *msg, size_t len, int *sent, WSAOVERLAPPED *ovr ) { - DWORD err; - if (ovr && !conn->port) { if (!(conn->port = CreateIoCompletionPort( (HANDLE)(SOCKET)conn->socket, NULL, (ULONG_PTR)conn->socket, 0 ))) @@ -505,7 +503,7 @@ DWORD netconn_send( struct netconn *conn, const void *msg, size_t len, int *sent if ((*sent = sock_send( conn->socket, msg, len, ovr )) < 0) { - err = WSAGetLastError(); + DWORD err = WSAGetLastError(); *sent = (err == WSA_IO_PENDING) ? len : 0; return err; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6709
From: Semenov Herman (Семенов Герман)<GermanAizek(a)yandex.ru> --- dlls/wbemprox/query.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dlls/wbemprox/query.c b/dlls/wbemprox/query.c index a6599334f58..e8abb2a96a8 100644 --- a/dlls/wbemprox/query.c +++ b/dlls/wbemprox/query.c @@ -536,7 +536,7 @@ static WCHAR *build_canonical_path( const WCHAR *relpath ) { BSTR server, namespace; WCHAR *ret; - UINT len, i; + UINT len; if (!(server = build_servername())) return NULL; if (!(namespace = build_namespace())) @@ -549,7 +549,7 @@ static WCHAR *build_canonical_path( const WCHAR *relpath ) if ((ret = malloc( len * sizeof(WCHAR ) ))) { len = swprintf( ret, len, L"\\\\%s\\%s:", server, namespace ); - for (i = 0; i < lstrlenW( relpath ); i ++) + for (int i = 0; i < lstrlenW( relpath ); i ++) { if (relpath[i] == '\'') ret[len++] = '"'; else ret[len++] = relpath[i]; @@ -820,7 +820,7 @@ static BOOL is_system_prop( const WCHAR *name ) static BSTR build_proplist( const struct table *table, UINT row, UINT count, UINT *len ) { - UINT i, j, offset; + UINT i, j; BSTR *values, ret = NULL; if (!(values = malloc( count * sizeof(BSTR) ))) return NULL; @@ -838,7 +838,7 @@ static BSTR build_proplist( const struct table *table, UINT row, UINT count, UIN } if ((ret = SysAllocStringLen( NULL, *len ))) { - offset = j = 0; + UINT offset = j = 0; for (i = 0; i < table->num_cols; i++) { if (table->columns[i].type & COL_FLAG_KEY) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6709
From: Semenov Herman (Семенов Герман)<GermanAizek(a)yandex.ru> --- dlls/ntdll/unix/file.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c index 790f0f5515f..d25200b6a66 100644 --- a/dlls/ntdll/unix/file.c +++ b/dlls/ntdll/unix/file.c @@ -1624,12 +1624,11 @@ static inline ULONG get_file_attributes( const struct stat *st ) static int parse_samba_dos_attrib_data( char *data, int len ) { char *end; - int val; if (len > 2 && data[0] == '0' && data[1] == 'x') { data[len] = 0; - val = strtol( data, &end, 16 ); + const int val = strtol( data, &end, 16 ); if (!*end) return val & XATTR_ATTRIBS_MASK; } else @@ -2176,15 +2175,14 @@ static unsigned int get_drives_info( struct file_identity info[MAX_DOS_DRIVES] ) mutex_lock( &cache_mutex ); if (now != last_update) { - char *buffer, *p; + char *buffer; struct stat st; - unsigned int i; if (asprintf( &buffer, "%s/dosdevices/a:", config_dir ) != -1) { - p = buffer + strlen(buffer) - 2; + char *p = buffer + strlen(buffer) - 2; - for (i = nb_drives = 0; i < MAX_DOS_DRIVES; i++) + for (unsigned int i = nb_drives = 0; i < MAX_DOS_DRIVES; i++) { *p = 'a' + i; if (!stat( buffer, &st )) @@ -2637,11 +2635,12 @@ static unsigned int get_cached_dir_data( HANDLE handle, struct dir_data **data_r const UNICODE_STRING *mask ) { unsigned int i; - int entry = -1, free_entries[16]; + int entry = -1; unsigned int status; SERVER_START_REQ( get_directory_cache_entry ) { + int free_entries[16]; req->handle = wine_server_obj_handle( handle ); wine_server_set_reply( req, free_entries, sizeof(free_entries) ); if (!(status = wine_server_call( req ))) entry = reply->entry; @@ -3661,7 +3660,7 @@ static NTSTATUS nt_to_unix_file_name_no_root( const UNICODE_STRING *nameW, char NTSTATUS nt_to_unix_file_name( const OBJECT_ATTRIBUTES *attr, char **name_ret, UINT disposition ) { enum server_fd_type type; - int old_cwd, root_fd, needs_close; + int root_fd, needs_close; const WCHAR *name; char *unix_name; int name_len, unix_len; @@ -3689,6 +3688,7 @@ NTSTATUS nt_to_unix_file_name( const OBJECT_ATTRIBUTES *attr, char **name_ret, U else { mutex_lock( &dir_mutex ); + int old_cwd; if ((old_cwd = open( ".", O_RDONLY )) != -1 && fchdir( root_fd ) != -1) { status = lookup_unix_name( name, name_len, &unix_name, unix_len, 1, disposition, FALSE ); @@ -6267,7 +6267,6 @@ NTSTATUS WINAPI NtFsControlFile( HANDLE handle, HANDLE event, PIO_APC_ROUTINE ap NTSTATUS WINAPI NtFlushBuffersFile( HANDLE handle, IO_STATUS_BLOCK *io ) { NTSTATUS ret; - HANDLE wait_handle; enum server_fd_type type; int fd, needs_close; @@ -6289,6 +6288,7 @@ NTSTATUS WINAPI NtFlushBuffersFile( HANDLE handle, IO_STATUS_BLOCK *io ) } else if (ret != STATUS_ACCESS_DENIED) { + HANDLE wait_handle; struct async_irp *async; if (!(async = (struct async_irp *)alloc_fileio( sizeof(*async), irp_completion, handle ))) @@ -7243,11 +7243,10 @@ NTSTATUS WINAPI NtQueryObject( HANDLE handle, OBJECT_INFORMATION_CLASS info_clas case ObjectTypesInformation: { OBJECT_TYPES_INFORMATION *types = ptr; - OBJECT_TYPE_INFORMATION *p; struct object_type_info *buffer; /* assume at most 32 types, with an average 16-char name */ UINT size = 32 * (sizeof(struct object_type_info) + 16 * sizeof(WCHAR)); - UINT i, count, pos, total, align = sizeof(DWORD_PTR) - 1; + UINT count, align = sizeof(DWORD_PTR) - 1; buffer = malloc( size ); SERVER_START_REQ( get_object_types ) @@ -7260,9 +7259,9 @@ NTSTATUS WINAPI NtQueryObject( HANDLE handle, OBJECT_INFORMATION_CLASS info_clas if (!status) { if (len >= sizeof(*types)) types->NumberOfTypes = count; - total = (sizeof(*types) + align) & ~align; - p = (OBJECT_TYPE_INFORMATION *)((char *)ptr + total); - for (i = pos = 0; i < count; i++) + UINT total = (sizeof(*types) + align) & ~align; + OBJECT_TYPE_INFORMATION *p = (OBJECT_TYPE_INFORMATION *)((char *)ptr + total); + for (UINT i = 0, pos = 0; i < count; i++) { struct object_type_info *info = (struct object_type_info *)((char *)buffer + pos); pos += sizeof(*info) + ((info->name_len + 3) & ~3); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6709
From: Semenov Herman (Семенов Герман)<GermanAizek(a)yandex.ru> --- dlls/d3dcompiler_43/compiler.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/dlls/d3dcompiler_43/compiler.c b/dlls/d3dcompiler_43/compiler.c index 6fffcfb82ab..0013cdafc13 100644 --- a/dlls/d3dcompiler_43/compiler.c +++ b/dlls/d3dcompiler_43/compiler.c @@ -79,7 +79,7 @@ static HRESULT WINAPI d3dcompiler_include_from_file_open(ID3DInclude *iface, D3D const char *filename, const void *parent_data, const void **data, UINT *bytes) { struct d3dcompiler_include_from_file *include = impl_from_ID3DInclude(iface); - char *fullpath, *buffer = NULL, current_dir[MAX_PATH + 1]; + char *fullpath, *buffer = NULL; const char *initial_dir; SIZE_T size; HANDLE file; @@ -93,6 +93,7 @@ static HRESULT WINAPI d3dcompiler_include_from_file_open(ID3DInclude *iface, D3D } else { + char current_dir[MAX_PATH + 1]; len = GetCurrentDirectoryA(MAX_PATH, current_dir); current_dir[len] = '\\'; len++; @@ -289,7 +290,6 @@ static HRESULT assemble_shader(const char *preproc_shader, ID3DBlob **shader_blo uint32_t *res, size; ID3DBlob *buffer; HRESULT hr; - char *pos; shader = SlAssembleShader(preproc_shader, &messages); @@ -313,7 +313,7 @@ static HRESULT assemble_shader(const char *preproc_shader, ID3DBlob **shader_blo if (shader) SlDeleteShader(shader); return hr; } - pos = ID3D10Blob_GetBufferPointer(buffer); + char *pos = ID3D10Blob_GetBufferPointer(buffer); if (preproc_messages) { CopyMemory(pos, preproc_messages, strlen(preproc_messages) + 1); @@ -362,9 +362,7 @@ HRESULT WINAPI D3DAssemble(const void *data, SIZE_T datasize, const char *filena const D3D_SHADER_MACRO *defines, ID3DInclude *include, UINT flags, ID3DBlob **shader, ID3DBlob **error_messages) { - unsigned int preproc_size; ID3DBlob *preproc_shader; - char *preproc_terminated; HRESULT hr; TRACE("data %p, datasize %Iu, filename %s, defines %p, include %p, sflags %#x, " @@ -382,7 +380,8 @@ HRESULT WINAPI D3DAssemble(const void *data, SIZE_T datasize, const char *filena hr = preprocess_shader(data, datasize, filename, defines, include, &preproc_shader, error_messages); if (SUCCEEDED(hr)) { - preproc_size = ID3D10Blob_GetBufferSize(preproc_shader); + unsigned int preproc_size = ID3D10Blob_GetBufferSize(preproc_shader); + char *preproc_terminated; if ((preproc_terminated = malloc(preproc_size + 1))) { memcpy(preproc_terminated, ID3D10Blob_GetBufferPointer(preproc_shader), preproc_size); @@ -461,7 +460,6 @@ HRESULT WINAPI D3DCompile2(const void *data, SIZE_T data_size, const char *filen struct vkd3d_shader_compile_info compile_info; struct vkd3d_shader_compile_option *option; struct vkd3d_shader_code byte_code; - const D3D_SHADER_MACRO *macro; char *messages; HRESULT hr; int ret; @@ -516,7 +514,7 @@ HRESULT WINAPI D3DCompile2(const void *data, SIZE_T data_size, const char *filen preprocess_info.macro_count = 0; if (macros) { - for (macro = macros; macro->Name; ++macro) + for (const D3D_SHADER_MACRO *macro = macros; macro->Name; ++macro) ++preprocess_info.macro_count; } preprocess_info.pfn_open_include = open_include; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6709
Matteo Bruni (@Mystral) commented about dlls/d3dcompiler_43/compiler.c:
if (shader) SlDeleteShader(shader); return hr; } - pos = ID3D10Blob_GetBufferPointer(buffer); + char *pos = ID3D10Blob_GetBufferPointer(buffer);
We don't allow variable definitions in the middle of a code block. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6709#note_85682
In general, I don't see the point of this kind of NOP changes with questionable style benefits. I can only speak for myself (and thus for the DLLs I maintain) but I'm not interested in accepting this. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6709#note_85683
Agreed. Please refrain from doing cosmetic changes in other people's code. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6709#note_85690
This merge request was closed by Alexandre Julliard. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6709
participants (4)
-
Alexandre Julliard (＠julliard) -
Herman Semenov (＠GermanAizek)
-
Matteo Bruni (＠Mystral)
-
Semenov Herman (Семенов Герман)