[PATCH 3/4] [DbgHelp]: cleanups for module loading (lookups on module name and image

Eric Pouech eric.pouech at wanadoo.fr
Tue Mar 13 11:33:02 CDT 2007


name are two different things)
- split up module_find_by_name in two different functions:
  + reused module_find_by_name which looks upon the module name
  + added module_is_already_loaded which looks upon the image name
- cleanup module loading using these functions (removed extra parameter
  to pe_load_module_from_pcs)

A+
---

 dlls/dbghelp/dbghelp_private.h |    9 ++--
 dlls/dbghelp/elf_module.c      |    2 -
 dlls/dbghelp/module.c          |  101 +++++++++++++++++++++-------------------
 dlls/dbghelp/pe_module.c       |   54 ++++++---------------
 dlls/dbghelp/source.c          |    2 -
 dlls/dbghelp/symbol.c          |    2 -
 6 files changed, 79 insertions(+), 91 deletions(-)

diff --git a/dlls/dbghelp/dbghelp_private.h b/dlls/dbghelp/dbghelp_private.h
index 198c4da..b62b120 100644
--- a/dlls/dbghelp/dbghelp_private.h
+++ b/dlls/dbghelp/dbghelp_private.h
@@ -441,10 +441,13 @@ extern struct module*
                                         enum module_type type);
 extern struct module*
                     module_find_by_name(const struct process* pcs,
-                                        const WCHAR* name, enum module_type type);
+                                        const WCHAR* name);
 extern struct module*
                     module_find_by_nameA(const struct process* pcs,
-                                         const char* name, enum module_type type);
+                                         const char* name);
+extern struct module*
+                    module_is_already_loaded(const struct process* pcs,
+                                             const WCHAR* imgname);
 extern BOOL         module_get_debug(struct module_pair*);
 extern struct module*
                     module_new(struct process* pcs, const WCHAR* name,
@@ -479,7 +482,7 @@ extern struct module*
                                    HANDLE hFile, DWORD base, DWORD size);
 extern struct module*
                     pe_load_module_from_pcs(struct process* pcs, const WCHAR* name,
-                                            const WCHAR* mod_name, DWORD base, DWORD size);
+                                            DWORD base, DWORD size);
 extern BOOL         pe_load_debug_info(const struct process* pcs,
                                        struct module* module);
 /* source.c */
diff --git a/dlls/dbghelp/elf_module.c b/dlls/dbghelp/elf_module.c
index d509db6..62a1ee8 100644
--- a/dlls/dbghelp/elf_module.c
+++ b/dlls/dbghelp/elf_module.c
@@ -1352,7 +1352,7 @@ static BOOL elf_search_and_load_file(str
     static WCHAR        S_libstdcPPW[] = {'l','i','b','s','t','d','c','+','+','\0'};
 
     if (filename == NULL || *filename == '\0') return FALSE;
-    if ((module = module_find_by_name(pcs, filename, DMT_ELF)))
+    if ((module = module_is_already_loaded(pcs, filename)))
     {
         elf_info->module = module;
         module->elf_info->elf_mark = 1;
diff --git a/dlls/dbghelp/module.c b/dlls/dbghelp/module.c
index 3bff6b7..f46a696 100644
--- a/dlls/dbghelp/module.c
+++ b/dlls/dbghelp/module.c
@@ -190,46 +190,50 @@ struct module* module_new(struct process
  *	module_find_by_name
  *
  */
-struct module* module_find_by_name(const struct process* pcs,
-                                   const WCHAR* name, enum module_type type)
+struct module* module_find_by_name(const struct process* pcs, const WCHAR* name)
 {
     struct module*      module;
 
-    if (type == DMT_UNKNOWN)
+    for (module = pcs->lmodules; module; module = module->next)
     {
-        if ((module = module_find_by_name(pcs, name, DMT_PE)) ||
-            (module = module_find_by_name(pcs, name, DMT_ELF)))
-            return module;
-    }
-    else
-    {
-        WCHAR   modname[MAX_PATH];
-
-        for (module = pcs->lmodules; module; module = module->next)
-        {
-            if (type == module->type &&
-                !strcmpiW(name, module->module.LoadedImageName))
-                return module;
-        }
-        module_fill_module(name, modname, sizeof(modname));
-        for (module = pcs->lmodules; module; module = module->next)
-        {
-            if (type == module->type &&
-                !strcmpiW(modname, module->module.ModuleName))
-                return module;
-        }
+        if (!strcmpiW(name, module->module.ModuleName)) return module;
     }
     SetLastError(ERROR_INVALID_NAME);
     return NULL;
 }
 
-struct module* module_find_by_nameA(const struct process* pcs,
-                                    const char* name, enum module_type type)
+struct module* module_find_by_nameA(const struct process* pcs, const char* name)
 {
     WCHAR wname[MAX_PATH];
 
     MultiByteToWideChar(CP_ACP, 0, name, -1, wname, sizeof(wname) / sizeof(WCHAR));
-    return module_find_by_name(pcs, wname, type);
+    return module_find_by_name(pcs, wname);
+}
+
+/***********************************************************************
+ *	module_is_already_loaded
+ *
+ */
+struct module* module_is_already_loaded(const struct process* pcs, const WCHAR* name)
+{
+    struct module*      module;
+    const WCHAR*        filename;
+
+    /* first compare the loaded image name... */
+    for (module = pcs->lmodules; module; module = module->next)
+    {
+        if (!strcmpiW(name, module->module.LoadedImageName))
+            return module;
+    }
+    /* then compare the standard filenames (without the path) ... */
+    filename = get_filename(name, NULL);
+    for (module = pcs->lmodules; module; module = module->next)
+    {
+        if (!strcmpiW(filename, get_filename(module->module.LoadedImageName, NULL)))
+            return module;
+    }
+    SetLastError(ERROR_INVALID_NAME);
+    return NULL;
 }
 
 /***********************************************************************
@@ -511,33 +515,36 @@ DWORD64 WINAPI  SymLoadModuleExW(HANDLE 
     /* this is a Wine extension to the API just to redo the synchronisation */
     if (!wImageName && !hFile) return 0;
 
-    if (module_is_elf_container_loaded(pcs, wImageName, BaseOfDll))
+    /* check if the module is already loaded, or if it's a builtin PE module with
+     * an containing ELF module
+     */
+    if (wImageName)
     {
-        /* force the loading of DLL as builtin */
-        if ((module = pe_load_module_from_pcs(pcs, wImageName, wModuleName,
-                                              BaseOfDll, SizeOfDll)))
-            goto done;
-        WARN("Couldn't locate %s\n", debugstr_w(wImageName));
-        return 0;
+        module = module_is_already_loaded(pcs, wImageName);
+        if (!module && module_is_elf_container_loaded(pcs, wImageName, BaseOfDll))
+        {
+            /* force the loading of DLL as builtin */
+            module = pe_load_module_from_pcs(pcs, wImageName, BaseOfDll, SizeOfDll);
+        }
+    }
+    if (!module)
+    {
+        /* otherwise, try a regular PE module */
+        if (!(module = pe_load_module(pcs, wImageName, hFile, BaseOfDll, SizeOfDll)))
+        {
+            /* and finally and ELF module */
+            if (module_get_type_by_name(wImageName) == DMT_ELF)
+                module = elf_load_module(pcs, wImageName, BaseOfDll);
+        }
     }
-    TRACE("Assuming %s as native DLL\n", debugstr_w(wImageName));
-    if (!(module = pe_load_module(pcs, wImageName, hFile, BaseOfDll, SizeOfDll)))
+    if (!module)
     {
-        if (module_get_type_by_name(wImageName) == DMT_ELF &&
-            (module = elf_load_module(pcs, wImageName, BaseOfDll)))
-            goto done;
-        FIXME("Should have successfully loaded debug information for image %s\n",
-              debugstr_w(wImageName));
-        if ((module = pe_load_module_from_pcs(pcs, wImageName, wModuleName,
-                                              BaseOfDll, SizeOfDll)))
-            goto done;
         WARN("Couldn't locate %s\n", debugstr_w(wImageName));
         return 0;
     }
     module->module.NumSyms = module->ht_symbols.num_elts;
-done:
-    /* by default pe_load_module fills module.ModuleName from a derivation
-     * of ImageName. Overwrite it, if we have better information
+    /* by default module_new fills module.ModuleName from a derivation
+     * of LoadedImageName. Overwrite it, if we have better information
      */
     if (wModuleName)
         module_set_module(module, wModuleName);
diff --git a/dlls/dbghelp/pe_module.c b/dlls/dbghelp/pe_module.c
index 8f75b80..b85c658 100644
--- a/dlls/dbghelp/pe_module.c
+++ b/dlls/dbghelp/pe_module.c
@@ -329,17 +329,13 @@ struct module* pe_load_module(struct pro
     struct module*      module = NULL;
     BOOL                opened = FALSE;
     HANDLE              hMap;
-    void*               mapping;
     WCHAR               loaded_name[MAX_PATH];
 
     loaded_name[0] = '\0';
     if (!hFile)
     {
-        if (!name)
-        {
-            /* FIXME SetLastError */
-            return NULL;
-        }
+
+        assert(name);
 
         if ((hFile = FindExecutableImageExW(name, pcs->search_path, loaded_name, NULL, NULL)) == NULL)
             return NULL;
@@ -348,9 +344,11 @@ struct module* pe_load_module(struct pro
     else if (name) strcpyW(loaded_name, name);
     else if (dbghelp_options & SYMOPT_DEFERRED_LOADS)
         FIXME("Trouble ahead (no module name passed in deferred mode)\n");
-    if (!(module = module_find_by_name(pcs, loaded_name, DMT_PE)) &&
-        (hMap = CreateFileMappingW(hFile, NULL, PAGE_READONLY, 0, 0, NULL)) != NULL)
+
+    if ((hMap = CreateFileMappingW(hFile, NULL, PAGE_READONLY, 0, 0, NULL)) != NULL)
     {
+        void*   mapping;
+
         if ((mapping = MapViewOfFile(hMap, FILE_MAP_READ, 0, 0, 0)) != NULL)
         {
             IMAGE_NT_HEADERS*   nth = RtlImageNtHeader(mapping);
@@ -400,41 +398,21 @@ BOOL pe_load_nt_header(HANDLE hProc, DWO
  *
  */
 struct module* pe_load_module_from_pcs(struct process* pcs, const WCHAR* name,
-                                       const WCHAR* mod_name, DWORD base, DWORD size)
+                                       DWORD base, DWORD size)
 {
-    struct module*      module;
-    const WCHAR*        ptr;
+    struct module*      module = NULL;
 
-    if ((module = module_find_by_name(pcs, name, DMT_PE))) return module;
-    if (mod_name) ptr = mod_name;
-    else
-    {
-        for (ptr = name + strlenW(name) - 1; ptr >= name; ptr--)
-        {
-            if (*ptr == '/' || *ptr == '\\')
-            {
-                ptr++;
-                break;
-            }
-        }
-    }
-    if (ptr && (module = module_find_by_name(pcs, ptr, DMT_PE))) return module;
-    if (base)
+    if (base && pcs->dbg_hdr_addr)
     {
-        if (pcs->dbg_hdr_addr)
-        {
-            IMAGE_NT_HEADERS    nth;
+        IMAGE_NT_HEADERS    nth;
 
-            if (pe_load_nt_header(pcs->handle, base, &nth))
-            {
-                if (!size) size = nth.OptionalHeader.SizeOfImage;
-                module = module_new(pcs, name, DMT_PE, FALSE, base, size,
-                                    nth.FileHeader.TimeDateStamp,
-                                    nth.OptionalHeader.CheckSum);
-            }
-        } else if (size)
+        if (pe_load_nt_header(pcs->handle, base, &nth))
+        {
+            if (!size) size = nth.OptionalHeader.SizeOfImage;
             module = module_new(pcs, name, DMT_PE, FALSE, base, size,
-                                0 /* FIXME */, 0 /* FIXME */);
+                                nth.FileHeader.TimeDateStamp,
+                                nth.OptionalHeader.CheckSum);
+        }
     }
     return module;
 }
diff --git a/dlls/dbghelp/source.c b/dlls/dbghelp/source.c
index 311a263..0939c65 100644
--- a/dlls/dbghelp/source.c
+++ b/dlls/dbghelp/source.c
@@ -133,7 +133,7 @@ BOOL WINAPI SymEnumSourceFiles(HANDLE hP
     {
         if (Mask[0] == '!')
         {
-            pair.requested = module_find_by_nameA(pair.pcs, Mask + 1, DMT_UNKNOWN);
+            pair.requested = module_find_by_nameA(pair.pcs, Mask + 1);
             if (!module_get_debug(&pair)) return FALSE;
         }
         else
diff --git a/dlls/dbghelp/symbol.c b/dlls/dbghelp/symbol.c
index 90f9d8f..d298b50 100644
--- a/dlls/dbghelp/symbol.c
+++ b/dlls/dbghelp/symbol.c
@@ -1174,7 +1174,7 @@ BOOL WINAPI SymFromName(HANDLE hProcess,
         assert(name - Name < sizeof(tmp));
         memcpy(tmp, Name, name - Name);
         tmp[name - Name] = '\0';
-        module = module_find_by_nameA(pcs, tmp, DMT_UNKNOWN);
+        module = module_find_by_nameA(pcs, tmp);
         return find_name(pcs, module, name + 1, Symbol);
     }
     for (module = pcs->lmodules; module; module = module->next)



More information about the wine-patches mailing list