Fix stub DllGetVersion implementation to read Dll version and put it in correct structure. Implement GetDllVersion by using DllGetVersion function and return version
-- v20: cabinet/tests: add test for GetDllVersion
From: Tobias G��rgens tobi.goergens@gmail.com
--- dlls/cabinet/cabinet.h | 9 +++++ dlls/cabinet/cabinet.spec | 2 +- dlls/cabinet/cabinet_main.c | 2 +- dlls/cabinet/tests/Makefile.in | 3 +- dlls/cabinet/tests/version.c | 67 ++++++++++++++++++++++++++++++++++ 5 files changed, 80 insertions(+), 3 deletions(-) create mode 100644 dlls/cabinet/tests/version.c
diff --git a/dlls/cabinet/cabinet.h b/dlls/cabinet/cabinet.h index 6193622cb11..707744e16cc 100644 --- a/dlls/cabinet/cabinet.h +++ b/dlls/cabinet/cabinet.h @@ -160,6 +160,15 @@ struct QTMstate { struct QTMmodelsym m80sym[0x40 + 1], mC0sym[0x40 + 1]; };
+/*version information used in cabinet.dll*/ +typedef struct _CABINETDLLVERSIONINFO { + DWORD cbStruct; + DWORD dwReserved1; + DWORD dwReserved2; + DWORD dwFileVersionMS; + DWORD dwFileVersionLS; +} CABINETDLLVERSIONINFO, *PCABINETDLLVERSIONINFO; + /* LZX stuff */
/* some constants defined by the LZX specification */ diff --git a/dlls/cabinet/cabinet.spec b/dlls/cabinet/cabinet.spec index 96127cfc429..7cf709de60e 100644 --- a/dlls/cabinet/cabinet.spec +++ b/dlls/cabinet/cabinet.spec @@ -1,5 +1,5 @@ 1 stub GetDllVersion -2 stdcall -private DllGetVersion (ptr) +2 stdcall -private DllGetVersion (ptr) cabinet_dll_get_version 3 stdcall Extract(ptr str) 4 stub DeleteExtractedFiles 10 cdecl FCICreate(ptr ptr ptr ptr ptr ptr ptr ptr ptr ptr ptr ptr ptr) diff --git a/dlls/cabinet/cabinet_main.c b/dlls/cabinet/cabinet_main.c index f95eca93c52..4187cf0140f 100644 --- a/dlls/cabinet/cabinet_main.c +++ b/dlls/cabinet/cabinet_main.c @@ -52,7 +52,7 @@ WINE_DEFAULT_DEBUG_CHANNEL(cabinet); * NOTES * Supposedly returns version from IE6SP1RP1 */ -HRESULT WINAPI DllGetVersion (DLLVERSIONINFO *pdvi) +HRESULT WINAPI cabinet_dll_get_version (DLLVERSIONINFO *pdvi) { WARN("hmmm... not right version number "5.1.1106.1"?\n");
diff --git a/dlls/cabinet/tests/Makefile.in b/dlls/cabinet/tests/Makefile.in index f301617473d..fc9fff75c7b 100644 --- a/dlls/cabinet/tests/Makefile.in +++ b/dlls/cabinet/tests/Makefile.in @@ -3,4 +3,5 @@ IMPORTS = cabinet
C_SRCS = \ extract.c \ - fdi.c + fdi.c \ + version.c diff --git a/dlls/cabinet/tests/version.c b/dlls/cabinet/tests/version.c new file mode 100644 index 00000000000..733ad451d31 --- /dev/null +++ b/dlls/cabinet/tests/version.c @@ -0,0 +1,67 @@ +#include <stdio.h> +#include <string.h> +#include <windows.h> +#include <winbase.h> +#include <wine/test.h> + + +#include "windef.h" +#define NO_SHLWAPI_REG +#include "shlwapi.h" +#undef NO_SHLWAPI_REG +#include "winbase.h" +#include "../cabinet.h" + + +typedef VOID (__stdcall *f_dllget)(PCABINETDLLVERSIONINFO); + + +static void test_dllget(HMODULE libHandle) +{ + PCABINETDLLVERSIONINFO verInfo; + char *version; + int sizeVerInfo; + DWORD FileVersionMS; + DWORD FileVersionLS; + int majorV; + int minorV; + int buildV; + int revisV; + + f_dllget DllGetVersion = (f_dllget)GetProcAddress(libHandle, "DllGetVersion"); + ok(DllGetVersion != NULL, "Function DllGetVersion in DLL not found: Error = %ld.\n", GetLastError()); + + + verInfo = malloc(sizeof(CABINETDLLVERSIONINFO)); + ok(verInfo != NULL, "Couldn't allocate memory to run tests properly!\n"); + if (DllGetVersion) + DllGetVersion(verInfo); + + FileVersionMS = verInfo->dwFileVersionMS; + FileVersionLS = verInfo->dwFileVersionLS; + + /*length of 4 DWORDs + buffer*/ + sizeVerInfo = 32; + + version = malloc(sizeVerInfo); + ok(version != NULL, "Couldn't allocate memory to run tests properly!\n"); + + majorV = (int)( FileVersionMS >> 16 ) & 0xffff; + minorV = (int)( FileVersionMS >> 0 ) & 0xffff; + buildV = (int)( FileVersionLS >> 16 ) & 0xffff; + revisV = (int)( FileVersionLS >> 0 ) & 0xffff; + + snprintf(version, sizeVerInfo, "%d.%d.%d.%d\n",majorV,minorV,buildV,revisV);; + todo_wine { + /* FIXME - Currently DllGetVersion isn't implemented correctly */ + ok(strcmp(version,"0.0.0.0\n") != 0, "Cabinet struct doesn't contain correct version: Error = %ld.\n", GetLastError()); + } +} + +START_TEST(version) +{ + HMODULE libHandle; + libHandle = LoadLibraryA("Cabinet.dll"); + ok(libHandle != NULL, "Cabinet.dll not found: Error = %ld.\n", GetLastError()); + test_dllget(libHandle); +}
From: "tobi.goergens" tobi.goergens@gmail.com
--- dlls/cabinet/Makefile.in | 2 +- dlls/cabinet/cabinet.rc | 22 +++++++++ dlls/cabinet/cabinet_main.c | 93 +++++++++++++++++++++++++++++++----- dlls/cabinet/tests/version.c | 5 +- 4 files changed, 104 insertions(+), 18 deletions(-)
diff --git a/dlls/cabinet/Makefile.in b/dlls/cabinet/Makefile.in index 4ee406480ac..22955d4027a 100644 --- a/dlls/cabinet/Makefile.in +++ b/dlls/cabinet/Makefile.in @@ -1,6 +1,6 @@ MODULE = cabinet.dll IMPORTLIB = cabinet -IMPORTS = $(ZLIB_PE_LIBS) +IMPORTS = $(ZLIB_PE_LIBS) kernelbase EXTRAINCL = $(ZLIB_PE_CFLAGS)
C_SRCS = \ diff --git a/dlls/cabinet/cabinet.rc b/dlls/cabinet/cabinet.rc index 8a022909d5c..bf045d53c28 100644 --- a/dlls/cabinet/cabinet.rc +++ b/dlls/cabinet/cabinet.rc @@ -26,3 +26,25 @@ #define WINE_PRODUCTVERSION_STR "5.0"
#include "wine/wine_common_ver.rc" + +100 VERSIONINFO +FILEVERSION WINE_FILEVERSION +PRODUCTVERSION WINE_PRODUCTVERSION +FILEOS VOS_NT +FILETYPE VFT_DLL +{ + BLOCK "StringFileInfo" + { + BLOCK "1200" + { + VALUE "FileDescription", WINE_FILEDESCRIPTION_STR + VALUE "FileVersion", WINE_FILEVERSION + VALUE "FileVersionStr", WINE_FILEVERSION_STR + VALUE "InternalName", WINE_FILENAME + VALUE "LegalCopyright", WINE_LEGALCOPYRIGHT + VALUE "OriginalFilename", WINE_FILENAME_STR + VALUE "ProductName", WINE_PRODUCTNAME_STR + VALUE "ProductVersion", WINE_PRODUCTVERSION_STR + } + } +} diff --git a/dlls/cabinet/cabinet_main.c b/dlls/cabinet/cabinet_main.c index 4187cf0140f..5823fa7cc6a 100644 --- a/dlls/cabinet/cabinet_main.c +++ b/dlls/cabinet/cabinet_main.c @@ -40,30 +40,97 @@ WINE_DEFAULT_DEBUG_CHANNEL(cabinet); /*********************************************************************** * DllGetVersion (CABINET.2) * - * Retrieves version information of the 'CABINET.DLL' + * Retrieves version information of the 'CABINET.DLL' using the + * CABINETDLLVERSIONINFO structure * * PARAMS - * pdvi [O] pointer to version information structure. + * cabVerInfo [O] pointer to CABINETDLLVERSIONINFO structure. * * RETURNS - * Success: S_OK - * Failure: E_INVALIDARG + * This function has no return value * * NOTES - * Supposedly returns version from IE6SP1RP1 + * Success: cabVerInfo points to mutet CABINETDLLVERSIONINFO structure + * Failure: cabVerInfo points to unmutet CABINETDLLVERSIONINFO structure + * Use GetLastError() to find out more about why the function failed + * */ -HRESULT WINAPI cabinet_dll_get_version (DLLVERSIONINFO *pdvi) + +VOID WINAPI cabinet_dll_get_version (PCABINETDLLVERSIONINFO cabVerInfo) { - WARN("hmmm... not right version number "5.1.1106.1"?\n");
- if (pdvi->cbSize != sizeof(DLLVERSIONINFO)) return E_INVALIDARG; + LPCSTR filename; + LPCSTR subBlock; + DWORD lenVersionInfo; + VOID *data; + VOID *buffer; + UINT lenRoot; + DWORD *handleVerInfo; + VS_FIXEDFILEINFO *versionInfo; + + filename = "Cabinet.dll"; + subBlock = "\"; + lenRoot = 0; + + if (cabVerInfo == NULL){ + SetLastError(ERROR_INVALID_PARAMETER); + TRACE("Bad Parameter: Error = %ld.\n", GetLastError()); + return; + } + + handleVerInfo = malloc(sizeof(DWORD)); + if (handleVerInfo == NULL){ + SetLastError(ERROR_OUTOFMEMORY); + TRACE("Cannot create handleVerInfo: Out of memory: Error = %ld.\n", GetLastError()); + return; + } + + lenVersionInfo = GetFileVersionInfoSizeA(filename, handleVerInfo); + if (lenVersionInfo == 0){ + TRACE("Cannot set lenVersionInfo: Couldn't parse File Version Info Size: Error = %ld.\n", GetLastError()); + return; + } + + data=HeapAlloc(GetProcessHeap(), 0, lenVersionInfo); + if (data == NULL) { + SetLastError(ERROR_OUTOFMEMORY); + TRACE("Cannot create data: Out of memory: Error = %ld.\n", GetLastError()); + return; + }
- pdvi->dwMajorVersion = 5; - pdvi->dwMinorVersion = 1; - pdvi->dwBuildNumber = 1106; - pdvi->dwPlatformID = 1;
- return S_OK; + if (GetFileVersionInfoA(filename, 0, lenVersionInfo, data) == 0){ + TRACE("Cannot get FileVersionInfo: Couldn't parse File Version Info Ressource: Error = %ld.\n", GetLastError()); + return; + } + + if (VerQueryValueA(data, subBlock, &buffer, &lenRoot) == 0){ + TRACE("Cannot query version info: Couldn't parse File Version Info Value: Error = %ld.\n", GetLastError()); + return; + } + else + { + if (lenRoot != 0) + { + versionInfo = (VS_FIXEDFILEINFO *)buffer; + if (versionInfo->dwSignature == 0xfeef04bd) + { + cabVerInfo->cbStruct = sizeof(CABINETDLLVERSIONINFO); + cabVerInfo->dwFileVersionMS = versionInfo->dwFileVersionMS; + cabVerInfo->dwFileVersionLS = versionInfo->dwFileVersionLS; + } + else + { + TRACE("Cannot verify struct: Version information has wrong signature: Error = %ld.\n", GetLastError()); + return; + } + } + else + { + TRACE("Cannot access struct: The length of the buffer holding version information is 0: Error = %ld.\n", GetLastError()); + return; + } + } }
/* FDI callback functions */ diff --git a/dlls/cabinet/tests/version.c b/dlls/cabinet/tests/version.c index 733ad451d31..8915bb6b775 100644 --- a/dlls/cabinet/tests/version.c +++ b/dlls/cabinet/tests/version.c @@ -52,10 +52,7 @@ static void test_dllget(HMODULE libHandle) revisV = (int)( FileVersionLS >> 0 ) & 0xffff;
snprintf(version, sizeVerInfo, "%d.%d.%d.%d\n",majorV,minorV,buildV,revisV);; - todo_wine { - /* FIXME - Currently DllGetVersion isn't implemented correctly */ - ok(strcmp(version,"0.0.0.0\n") != 0, "Cabinet struct doesn't contain correct version: Error = %ld.\n", GetLastError()); - } + ok(strcmp(version,"0.0.0.0\n") != 0, "Cabinet struct doesn't contain correct version: Error = %ld.\n", GetLastError()); }
START_TEST(version)
From: "tobi.goergens" tobi.goergens@gmail.com
--- dlls/cabinet/cabinet.spec | 2 +- dlls/cabinet/cabinet_main.c | 63 +++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-)
diff --git a/dlls/cabinet/cabinet.spec b/dlls/cabinet/cabinet.spec index 7cf709de60e..3a98245acc1 100644 --- a/dlls/cabinet/cabinet.spec +++ b/dlls/cabinet/cabinet.spec @@ -1,4 +1,4 @@ -1 stub GetDllVersion +1 stdcall GetDllVersion() 2 stdcall -private DllGetVersion (ptr) cabinet_dll_get_version 3 stdcall Extract(ptr str) 4 stub DeleteExtractedFiles diff --git a/dlls/cabinet/cabinet_main.c b/dlls/cabinet/cabinet_main.c index 5823fa7cc6a..2faa2af8be2 100644 --- a/dlls/cabinet/cabinet_main.c +++ b/dlls/cabinet/cabinet_main.c @@ -133,6 +133,69 @@ VOID WINAPI cabinet_dll_get_version (PCABINETDLLVERSIONINFO cabVerInfo) } }
+/*********************************************************************** + * GetDllVersion (CABINET.2) + * + * Returns the version of the Cabinet.dll + * + * PARAMS + * This function has to parameters + * + * RETURNS + * Success: cabDllVer: string of Cabinet.dll version + * Failure: empty string. + * Use GetLastError() to find out more about why the function failed + * + */ + +LPCSTR WINAPI GetDllVersion(void) +{ + PCABINETDLLVERSIONINFO cabVerInfo; + LPSTR cabDllVer; + int sizeVerInfo; + DWORD FileVersionMS; + DWORD FileVersionLS; + int majorV; + int minorV; + int buildV; + int revisV; + + cabVerInfo = malloc(sizeof(CABINETDLLVERSIONINFO)); + if(cabVerInfo == NULL) { + SetLastError(ERROR_OUTOFMEMORY); + TRACE("Cannot create cabVerInfo: Out of memory: Error = %ld.\n", GetLastError()); + return ""; + } + + cabinet_dll_get_version(cabVerInfo); + if (cabVerInfo->cbStruct==0) { + TRACE("Cannot access struct: The length of the version information structure is 0: Error = %ld.\n", GetLastError()); + return ""; + } + + FileVersionMS = cabVerInfo->dwFileVersionMS; + FileVersionLS = cabVerInfo->dwFileVersionLS; + + /*length of 4 DWORDs + buffer*/ + sizeVerInfo = 32; + + cabDllVer = malloc(sizeVerInfo); + if (cabDllVer == NULL) { + SetLastError(ERROR_OUTOFMEMORY); + TRACE("Cannot create cabDllVer: Out of memory: Error = %ld.\n", GetLastError()); + return ""; + } + + majorV = (int)( FileVersionMS >> 16 ) & 0xffff; + minorV = (int)( FileVersionMS >> 0 ) & 0xffff; + buildV = (int)( FileVersionLS >> 16 ) & 0xffff; + revisV = (int)( FileVersionLS >> 0 ) & 0xffff; + + snprintf(cabDllVer, sizeVerInfo, "%d.%d.%d.%d\n",majorV,minorV,buildV,revisV); + + return cabDllVer; +} + /* FDI callback functions */
static void * CDECL mem_alloc(ULONG cb)
From: "tobi.goergens" tobi.goergens@gmail.com
--- dlls/cabinet/tests/version.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/dlls/cabinet/tests/version.c b/dlls/cabinet/tests/version.c index 8915bb6b775..eb0a42f7718 100644 --- a/dlls/cabinet/tests/version.c +++ b/dlls/cabinet/tests/version.c @@ -14,7 +14,7 @@
typedef VOID (__stdcall *f_dllget)(PCABINETDLLVERSIONINFO); - +typedef LPCSTR (__stdcall *f_getdll)(void);
static void test_dllget(HMODULE libHandle) { @@ -55,10 +55,21 @@ static void test_dllget(HMODULE libHandle) ok(strcmp(version,"0.0.0.0\n") != 0, "Cabinet struct doesn't contain correct version: Error = %ld.\n", GetLastError()); }
+static void test_getdll(HMODULE libHandle) +{ + f_getdll GetDllVersion = (f_getdll)GetProcAddress(libHandle, "GetDllVersion"); + ok(GetDllVersion != NULL, "Function GetDllVersion in DLL not found: Error = %ld.\n", GetLastError()); + if (GetDllVersion){ + ok(strcmp(GetDllVersion(),"") != 0, "GetDllVersion returns empty version: Error = %ld.\n", GetLastError()); + ok(strcmp(GetDllVersion(),"0.0.0.0\n") != 0, "GetDllVersion doesn't return correct version: Error = %ld.\n", GetLastError()); + } +} + START_TEST(version) { HMODULE libHandle; libHandle = LoadLibraryA("Cabinet.dll"); ok(libHandle != NULL, "Cabinet.dll not found: Error = %ld.\n", GetLastError()); test_dllget(libHandle); + test_getdll(libHandle); }
I still don't see why it's necessary to change that much. CABINETDLLVERSIONINFO type is identical to DLLVERSIONINFO, renaming DllGetVersion() function to something else is not necessary. Same for changing function prototype.
Adding version information block to the dll is fine, if it's present on Windows. We usually do that when we find application that checks the version.
On Fri Aug 5 18:13:12 2022 +0000, Nikolay Sivov wrote:
I still don't see why it's necessary to change that much. CABINETDLLVERSIONINFO type is identical to DLLVERSIONINFO, renaming DllGetVersion() function to something else is not necessary. Same for changing function prototype. Adding version information block to the dll is fine, if it's present on Windows. We usually do that when we find application that checks the version.
The test I added in the first commit doesn't return the version when called with a CABINETDLLVERSION structure, which is expected in Windows. When I recall correctly, they are not exactly the same, I might be wrong though (I also don't have to much experience with how structures behave in WINAPI/C).
It was proposed above to rename DllGetVersion(): "That is because DllGetVersion() is also the name of an optional function supported by COM server DLLs" (can be found in the unresolved thread). When it's not renamed, compilation fails. Do you have any other solution to this?
I add the version information block as I didn't find another way to access the version information within the same DLL. The only other solution I came up with was to hard-code the dll version somewhere else as well, but this creates redundancy and one would have to change the version in multiple spots when wanted. This approach eliminates this issue.
On Fri Aug 5 18:13:12 2022 +0000, Tobias G��rgens wrote:
The test I added in the first commit doesn't return the version when called with a CABINETDLLVERSION structure, which is expected in Windows. When I recall correctly, they are not exactly the same, I might be wrong though (I also don't have to much experience with how structures behave in WINAPI/C). It was proposed above to rename DllGetVersion(): "That is because DllGetVersion() is also the name of an optional function supported by COM server DLLs" (can be found in the unresolved thread). When it's not renamed, compilation fails. Do you have any other solution to this? I add the version information block as I didn't find another way to access the version information within the same DLL. The only other solution I came up with was to hard-code the dll version somewhere else as well, but this creates redundancy and one would have to change the version in multiple spots when wanted. This approach eliminates this issue.
We already have it as DllGetVersion() in current code and it compiles fine. If the only issue is that returned fields are "incorrect" in some way, you can simply fix them up. But then again, if nothing depends on it I wouldn't bother.
On Fri Aug 5 18:17:31 2022 +0000, Nikolay Sivov wrote:
We already have it as DllGetVersion() in current code and it compiles fine. If the only issue is that returned fields are "incorrect" in some way, you can simply fix them up. But then again, if nothing depends on it I wouldn't bother.
Yes, it compiles currently, as it doesn't follow the microsoft doc: it has HRETURN as return value instead of void and uses the DLLVERSIONINFO structure. This is both not compliant with the MS docs. As I had issues with the current implementation in early testing, I fixed both. Though, if that is not wanted, I can revert these changes. However, this seems to me more like kicking the can down the road to be honest: should an application really rely on it, it would have to be corrected again, even though this already is a working implementation (at least in my testing)
A second issue would be that I wouldn't know how to fix the return field if a wrong structure is handed over
On Fri Aug 5 18:24:34 2022 +0000, Tobias G��rgens wrote:
Yes, it compiles currently, as it doesn't follow the microsoft doc: it has HRETURN as return value instead of void and uses the DLLVERSIONINFO structure. This is both not compliant with the MS docs. As I had issues with the current implementation in early testing, I fixed both. Though, if that is not wanted, I can revert these changes. However, this seems to me more like kicking the can down the road to be honest: should an application really rely on it, it would have to be corrected again, even though this already is a working implementation (at least in my testing) A second issue would be that I wouldn't know how to fix the return field if a wrong structure is handed over
How would you test if return value is void and not S_OK all the time? Structure has the same layout, only field names are different.
On Fri Aug 5 18:26:17 2022 +0000, Nikolay Sivov wrote:
How would you test if return value is void and not S_OK all the time? Structure has the same layout, only field names are different.
I didn't test the return value, but the returned structure, if the correct structure is handed over. And this didn't work
On Tue Aug 2 12:36:30 2022 +0000, Tobias G��rgens wrote:
The test commit almost works now, the only issue is that GetProcAdress returns an address even though GetDllVersion isn't implemented. Does someone know how I can fix this? Should I just remove the test for GetDllVersion, as it isn't even implemented yet?
Do you have any additional hints/ do you see any issues with the current implementation (additional to what Nikolay says)?
-
Nikolay Sivov (＠nsivov) -
tobi.goergens
-
Tobias Görgens
-
Tobias Görgens (＠tobi.goergens)