From b3b9decfab8a26851e2bc85d777824220ff804b6 Mon Sep 17 00:00:00 2001 From: Bob Arnson Date: Mon, 24 Apr 2023 22:16:34 -0400 Subject: Fix WcaErrorMessage `cArgs==-1` case. - Count args before creating message record. - Document terminating NULL requirement. - Add terminating NULL in MessageExit* macros. - Enhance tests for problems encountered fixing this nightmare. Fixes https://github.com/wixtoolset/issues/issues/7422. Fixes https://github.com/wixtoolset/issues/issues/7444. --- src/libs/wcautil/WixToolset.WcaUtil/inc/wcautil.h | 6 ++-- src/libs/wcautil/WixToolset.WcaUtil/wcawrap.cpp | 33 +++++++++++++--------- src/test/burn/WixTestTools/MSIExec.cs | 2 +- .../ProductNonVitalUserGroup/NonVitalUserGroup.wxs | 2 ++ 4 files changed, 26 insertions(+), 17 deletions(-) diff --git a/src/libs/wcautil/WixToolset.WcaUtil/inc/wcautil.h b/src/libs/wcautil/WixToolset.WcaUtil/inc/wcautil.h index 9b331b34..b72c8d3f 100644 --- a/src/libs/wcautil/WixToolset.WcaUtil/inc/wcautil.h +++ b/src/libs/wcautil/WixToolset.WcaUtil/inc/wcautil.h @@ -15,9 +15,9 @@ extern "C" { #include "dutil.h" -#define MessageExitOnLastErrorSource(d, x, e, s, ...) { x = ::GetLastError(); x = HRESULT_FROM_WIN32(x); if (FAILED(x)) { ExitTraceSource(d, x, s, __VA_ARGS__); WcaErrorMessage(e, x, MB_OK, -1, __VA_ARGS__); goto LExit; } } -#define MessageExitOnFailureSource(d, x, e, s, ...) if (FAILED(x)) { ExitTraceSource(d, x, s, __VA_ARGS__); WcaErrorMessage(e, x, INSTALLMESSAGE_ERROR | MB_OK, -1, __VA_ARGS__); goto LExit; } -#define MessageExitOnNullWithLastErrorSource(d, p, x, e, s, ...) if (NULL == p) { x = ::GetLastError(); x = HRESULT_FROM_WIN32(x); if (!FAILED(x)) { x = E_FAIL; } ExitTraceSource(d, x, s, __VA_ARGS__); WcaErrorMessage(e, x, MB_OK, -1, __VA_ARGS__); goto LExit; } +#define MessageExitOnLastErrorSource(d, x, e, s, ...) { x = ::GetLastError(); x = HRESULT_FROM_WIN32(x); if (FAILED(x)) { ExitTraceSource(d, x, s, __VA_ARGS__); WcaErrorMessage(e, x, MB_OK, -1, __VA_ARGS__, NULL); goto LExit; } } +#define MessageExitOnFailureSource(d, x, e, s, ...) if (FAILED(x)) { ExitTraceSource(d, x, s, __VA_ARGS__); WcaErrorMessage(e, x, INSTALLMESSAGE_ERROR | MB_OK, -1, __VA_ARGS__, NULL); goto LExit; } +#define MessageExitOnNullWithLastErrorSource(d, p, x, e, s, ...) if (NULL == p) { x = ::GetLastError(); x = HRESULT_FROM_WIN32(x); if (!FAILED(x)) { x = E_FAIL; } ExitTraceSource(d, x, s, __VA_ARGS__); WcaErrorMessage(e, x, MB_OK, -1, __VA_ARGS__, NULL); goto LExit; } #define MessageExitOnLastError(x, e, s, ...) MessageExitOnLastErrorSource(DUTIL_SOURCE_DEFAULT, x, e, s, __VA_ARGS__) #define MessageExitOnFailure(x, e, s, ...) MessageExitOnFailureSource(DUTIL_SOURCE_DEFAULT, x, e, s, __VA_ARGS__) diff --git a/src/libs/wcautil/WixToolset.WcaUtil/wcawrap.cpp b/src/libs/wcautil/WixToolset.WcaUtil/wcawrap.cpp index 86b7602e..9259e97a 100644 --- a/src/libs/wcautil/WixToolset.WcaUtil/wcawrap.cpp +++ b/src/libs/wcautil/WixToolset.WcaUtil/wcawrap.cpp @@ -27,7 +27,9 @@ WcaErrorMessage() - sends an error message from the CustomAction using the Error table NOTE: Any and all var_args (...) must be WCHAR* - If you pass -1 to cArgs the count will be determined + If you pass -1 to cArgs, the count will be determined by + looking for a trailing NULL argment. If you omit a terminating + NULL, the results are undefined and probably crashy. ********************************************************************/ extern "C" UINT __cdecl WcaErrorMessage( __in int iError, @@ -41,6 +43,22 @@ extern "C" UINT __cdecl WcaErrorMessage( MSIHANDLE hRec = NULL; va_list args = NULL; + if (-1 == cArgs) + { + LPCWSTR wzArg = NULL; + va_list iter = NULL; + + va_start(iter, cArgs); + cArgs = 0; + + while (NULL != (wzArg = va_arg(iter, WCHAR*)) && L'\0' != *wzArg) + { + ++cArgs; + } + + va_end(iter); + } + uiType |= INSTALLMESSAGE_ERROR; // ensure error type is set hRec = ::MsiCreateRecord(cArgs + 2); if (!hRec) @@ -56,18 +74,6 @@ extern "C" UINT __cdecl WcaErrorMessage( ExitOnFailure(HRESULT_FROM_WIN32(er), "failed to set hresult code into error message"); va_start(args, cArgs); - if (-1 == cArgs) - { - LPCWSTR wzArg = NULL; - va_list iter = args; - cArgs = 0; - - while (NULL != (wzArg = va_arg(iter, WCHAR*)) && L'\0' != *wzArg) - { - ++cArgs; - } - } - for (INT i = 0; i < cArgs; i++) { er = ::MsiRecordSetStringW(hRec, i + 3, va_arg(args, WCHAR*)); @@ -76,6 +82,7 @@ extern "C" UINT __cdecl WcaErrorMessage( va_end(args); er = WcaProcessMessage(static_cast(uiType), hRec); + LExit: if (args) { diff --git a/src/test/burn/WixTestTools/MSIExec.cs b/src/test/burn/WixTestTools/MSIExec.cs index 98f54c56..f8c73afc 100644 --- a/src/test/burn/WixTestTools/MSIExec.cs +++ b/src/test/burn/WixTestTools/MSIExec.cs @@ -111,7 +111,7 @@ namespace WixTestTools this.ForceRestart = false; this.PromptRestart = false; this.LogFile = String.Empty; - this.LoggingOptions = MSIExecLoggingOptions.VOICEWARMUP; + this.LoggingOptions = MSIExecLoggingOptions.Log_All_Information | MSIExecLoggingOptions.Verbose_Output | MSIExecLoggingOptions.Extra_Debugging_Information; // `/l*vx` this.OtherArguments = String.Empty; } diff --git a/src/test/msi/TestData/UtilExtensionUserTests/ProductNonVitalUserGroup/NonVitalUserGroup.wxs b/src/test/msi/TestData/UtilExtensionUserTests/ProductNonVitalUserGroup/NonVitalUserGroup.wxs index 461648ee..7ab4bbe1 100644 --- a/src/test/msi/TestData/UtilExtensionUserTests/ProductNonVitalUserGroup/NonVitalUserGroup.wxs +++ b/src/test/msi/TestData/UtilExtensionUserTests/ProductNonVitalUserGroup/NonVitalUserGroup.wxs @@ -10,12 +10,14 @@ + + -- cgit v1.2.3-55-g6feb