Skip to content

SAL annotation updates & fix warnings #5617

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
TravisEz13 merged 3 commits into from
Jan 6, 2018
Merged

SAL annotation updates & fix warnings #5617

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6944b7f
SAL annotation updates and fix warnings
dantraMSFT Dec 4, 2017
394d413
Address PR feedback.
dantraMSFT Dec 15, 2017
97e9982
Rewrite ReportOperationComplete for clarity
dantraMSFT Jan 3, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions src/powershell-native/nativemsh/pwrshcommon/NativeMsh.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,25 +78,27 @@ namespace NativeMsh
// Note: During successful calls the following values must be freed by the caller:
// pwszMonadVersion
// pwszRuntimeVersion
// pwzsRegKeyValue
// pwszRegKeyValue
//
// The caller must take care to check to see if they must be freed during error scenarios
// because the function may fail after allocating one or more strings.
//
_Success_(return == 0)
unsigned int GetRegistryInfo(
__deref_out_opt PWSTR * pwszMonadVersion,
__out PWSTR * pwszMonadVersion,
__inout_ecount(1) int * lpMonadMajorVersion,
int monadMinorVersion,
__deref_out_opt PWSTR * pwszRuntimeVersion,
__out PWSTR * pwszRuntimeVersion,
LPCWSTR lpszRegKeyNameToRead,
__deref_out_opt PWSTR * pwzsRegKeyValue);
__out PWSTR * pwszRegKeyValue);

_Success_(return == 0)
unsigned int GetRegistryInfo(
__deref_out_opt PWSTR * pwszMonadVersion,
__out PWSTR * pwszMonadVersion,
__inout_ecount(1) int * lpMonadMajorVersion,
int monadMinorVersion,
__deref_out_opt PWSTR * pwszRuntimeVersion,
__deref_out_opt PWSTR * pwszConsoleHostAssemblyName);
__out PWSTR * pwszRuntimeVersion,
__out PWSTR * pwszConsoleHostAssemblyName);

unsigned int LaunchCoreCLR(
ClrHostWrapper* hostWrapper,
Expand Down
35 changes: 26 additions & 9 deletions src/powershell-native/nativemsh/pwrshcommon/pwrshcommon.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1238,31 +1238,47 @@ namespace NativeMsh
// Note: During successful calls the following values must be freed by the caller:
// pwszMonadVersion
// pwszRuntimeVersion
// pwzsRegKeyValue
// pwszRegKeyValue
//
// The caller must take care to check to see if they must be freed during error scenarios
// because the function may fail after allocating one or more strings.
//
_Success_(return == 0)
unsigned int PwrshCommon::GetRegistryInfo(
__deref_out_opt PWSTR * pwszMonadVersion,
__out PWSTR * pwszMonadVersion,
__inout_ecount(1) int * lpMonadMajorVersion,
int monadMinorVersion,
__deref_out_opt PWSTR * pwszRuntimeVersion,
__out PWSTR * pwszRuntimeVersion,
LPCWSTR lpszRegKeyNameToRead,
__deref_out_opt PWSTR * pwzsRegKeyValue)
__out PWSTR * pwszRegKeyValue)
{
HKEY hEngineKey = NULL;
bool bEngineKeyOpened = true;
unsigned int exitCode = EXIT_CODE_SUCCESS;
wchar_t * wszMshEngineRegKeyPath = NULL;
LPWSTR wszFullMonadVersion = NULL;

if (NULL != pwszRegKeyValue)
{
*pwszRegKeyValue = NULL;
}

if (NULL != pwszRuntimeVersion)
{
*pwszRuntimeVersion = NULL;
}

if (NULL != pwszMonadVersion)
{
*pwszMonadVersion = NULL;
}

do
{
if (NULL == pwszMonadVersion ||
NULL == lpMonadMajorVersion ||
NULL == pwszRuntimeVersion ||
NULL == pwzsRegKeyValue)
NULL == pwszRegKeyValue)
{
exitCode = EXIT_CODE_READ_REGISTRY_FAILURE;
break;
Expand Down Expand Up @@ -1315,7 +1331,7 @@ namespace NativeMsh
{
LPCWSTR wszRequestedRegValueName = lpszRegKeyNameToRead;
if (!this->RegQueryREG_SZValue(hEngineKey, wszRequestedRegValueName,
wszMshEngineRegKeyPath, pwzsRegKeyValue))
wszMshEngineRegKeyPath, pwszRegKeyValue))
{
exitCode = EXIT_CODE_READ_REGISTRY_FAILURE;
break;
Expand Down Expand Up @@ -1361,12 +1377,13 @@ namespace NativeMsh
return exitCode;
}

_Success_(return == 0)
unsigned int PwrshCommon::GetRegistryInfo(
__deref_out_opt PWSTR * pwszMonadVersion,
__out PWSTR * pwszMonadVersion,
__inout_ecount(1) int * lpMonadMajorVersion,
int monadMinorVersion,
__deref_out_opt PWSTR * pwszRuntimeVersion,
__deref_out_opt PWSTR * pwszConsoleHostAssemblyName)
__out PWSTR * pwszRuntimeVersion,
__out PWSTR * pwszConsoleHostAssemblyName)
{
return GetRegistryInfo(pwszMonadVersion,
lpMonadMajorVersion,
Expand Down
2 changes: 1 addition & 1 deletion src/powershell-native/nativemsh/pwrshexe/CssMainEntry.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ int __cdecl wmain(const int argc, const wchar_t* argv[])
if (debug)
{
::wprintf(L" Attach the debugger to powershell.exe and press any key to continue\n");
::getchar();
(void) ::getchar();
}

if (helpRequested)
Expand Down
64 changes: 40 additions & 24 deletions src/powershell-native/nativemsh/pwrshplugin/entrypoints.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,18 @@ LPCWSTR g_MAIN_BINARY_NAME = L"pwrshplugin.dll";
// the caller should free pwszErrorMessage using LocalFree().
// returns: If the function succeeds the return value is the number of CHARs stored int the output
// buffer, excluding the terminating null character. If the function fails the return value is zero.
#pragma prefast(push)
#pragma prefast (disable: 28196)
DWORD GetFormattedErrorMessage(__deref_out PWSTR * pwszErrorMessage, DWORD dwMessageId, va_list* arguments)
#pragma prefast(pop)
_Success_(return > 0)
DWORD GetFormattedErrorMessage(__out PWSTR * pwszErrorMessage, DWORD dwMessageId, va_list* arguments)
{
DWORD dwLength = 0;
LPWSTR wszSystemErrorMessage = NULL;

do
{
if (NULL == pwszErrorMessage)
{
break;
}
*pwszErrorMessage = NULL;

if (NULL == g_hResourceInstance)
Expand All @@ -51,8 +54,6 @@ DWORD GetFormattedErrorMessage(__deref_out PWSTR * pwszErrorMessage, DWORD dwMes
#endif
}

LPWSTR wszSystemErrorMessage = NULL;
//string function
dwLength = FormatMessageW(
FORMAT_MESSAGE_FROM_HMODULE | FORMAT_MESSAGE_ALLOCATE_BUFFER,
g_hResourceInstance,
Expand All @@ -62,28 +63,36 @@ DWORD GetFormattedErrorMessage(__deref_out PWSTR * pwszErrorMessage, DWORD dwMes
0,
arguments);

if (dwLength > 0)
if (dwLength == 0)
{
*pwszErrorMessage = new WCHAR[dwLength + 1];
if (NULL != *pwszErrorMessage)
{
//string function
if (FAILED(StringCchCopyW(*pwszErrorMessage, dwLength + 1, wszSystemErrorMessage)))
{
dwLength = 0;
delete [] (*pwszErrorMessage);
*pwszErrorMessage = NULL;
}
}
LocalFree(wszSystemErrorMessage);
break;
}

*pwszErrorMessage = new WCHAR[dwLength + 1];
if (*pwszErrorMessage == NULL)
{
dwLength = 0;
break;
}

if (FAILED(StringCchCopyW(*pwszErrorMessage, dwLength + 1, wszSystemErrorMessage)))
{
dwLength = 0;
delete [] (*pwszErrorMessage);
*pwszErrorMessage = NULL;
}

}while(false);

if (NULL != wszSystemErrorMessage)
{
LocalFree(wszSystemErrorMessage);
}

return dwLength;
}

DWORD GetFormattedErrorMessage(__deref_out PZPWSTR pwszErrorMessage, DWORD dwMessageId, ...)
DWORD GetFormattedErrorMessage(__out PWSTR * pwszErrorMessage, DWORD dwMessageId, ...)
{
DWORD result = 0;

Expand Down Expand Up @@ -307,12 +316,19 @@ DWORD ReportOperationComplete(WSMAN_PLUGIN_REQUEST *requestDetails, DWORD errorC

DWORD result = EXIT_CODE_SUCCESS;
PWSTR pwszErrorMessage = NULL;
GetFormattedErrorMessage(&pwszErrorMessage, errorCode);

result = WSManPluginOperationComplete(requestDetails, 0, errorCode, pwszErrorMessage);
if (GetFormattedErrorMessage(&pwszErrorMessage, errorCode) == 0)
{
/* if an error message could not be loaded, generate a fallback
message to provide a level of diagnosability.
*/
WCHAR szFallbackMessageMessage[64] = L"\0";
swprintf_s(szFallbackMessageMessage, _countof(szFallbackMessageMessage), L"ReportOperationComplete: %d", errorCode);

if (NULL != pwszErrorMessage)
result = WSManPluginOperationComplete(requestDetails, 0, errorCode, szFallbackMessageMessage);
}
else
{
result = WSManPluginOperationComplete(requestDetails, 0, errorCode, pwszErrorMessage);
delete[] pwszErrorMessage;
}

Expand Down
2 changes: 1 addition & 1 deletion src/powershell-native/nativemsh/pwrshplugin/entrypoints.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@

const int EXIT_CODE_BAD_INPUT = 100;

extern DWORD GetFormattedErrorMessage(__deref_out PWSTR * pwszErrorMessage, DWORD dwMessageId, ...);
extern DWORD GetFormattedErrorMessage(__out PWSTR * pwszErrorMessage, DWORD dwMessageId, ...);
extern unsigned int ConstructPowerShellVersion(int iPSMajorVersion, int iPSMinorVersion, __deref_out_opt PWSTR *pwszMonadVersion);

5 changes: 4 additions & 1 deletion src/powershell-native/nativemsh/pwrshplugin/pwrshclrhost.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,10 @@ unsigned int PowerShellClrWorker::LoadWorkerCallbackPtrs(
{
PWSTR msg = NULL;
exitCode = g_MANAGED_METHOD_RESOLUTION_FAILED;
GetFormattedErrorMessage(&msg, exitCode);
/* NOTE: don't use a string literal for a fallback; PlugInException expects msg to allocated
and will free it but also supports NULL.
*/
(void) GetFormattedErrorMessage(&msg, exitCode);
*pPluginException = new PlugInException(exitCode, msg);
return exitCode;
}
Expand Down
22 changes: 18 additions & 4 deletions src/powershell-native/nativemsh/pwrshplugin/pwrshplugin.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,11 @@ class PwrshPlugInMediator
{
iMajorVersion = iVPSMajorVersion;

if (NULL != wszMgdPlugInFileName)
Copy link
Member

@mirichmo mirichmo Dec 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This already occurs on line 671. It seems unnecessary to do it twice. Did the scanner require it?

Copy link
Contributor Author

@dantraMSFT dantraMSFT Dec 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check is added because wszMgdPlugInFileName was not being set in all paths; specifically error paths.

{
*wszMgdPlugInFileName = NULL;
}

exitCode = ConstructPowerShellVersion(iVPSMajorVersion, iVPSMinorVersion, &wszMonadVersion);
if (EXIT_CODE_SUCCESS != exitCode)
{
Expand Down Expand Up @@ -561,10 +566,19 @@ class PwrshPlugInMediator
_In_ PWSTR wszMgdPlugInFileName,
_In_ PWSTR wszVCLRVersion, // Conditionally set to wszCLRVersion on success
_In_ PWSTR wszVAppBase, // Conditionally set to wszAppBase on success
__out_opt PlugInException** pPluginException )
__out PlugInException** pPluginException )
{
unsigned int exitCode = EXIT_CODE_SUCCESS;

if (NULL != pPluginException)
{
*pPluginException = NULL;
}
else
{
return g_INVALID_INPUT;
}

if (bIsPluginLoaded)
{
return g_MANAGED_PLUGIN_ALREADY_LOADED;
Expand All @@ -577,8 +591,6 @@ class PwrshPlugInMediator

do
{
*pPluginException = NULL;

// Setting global AppBase and CLR Version
wszCLRVersion = wszVCLRVersion;
wszAppBase = wszVAppBase;
Expand Down Expand Up @@ -768,7 +780,9 @@ class PwrshPlugInMediator
else
{
PWSTR msg = NULL;
GetFormattedErrorMessage(&msg, g_OPTION_SET_NOT_COMPLY, g_BUILD_VERSION);
(void) GetFormattedErrorMessage(&msg, g_OPTION_SET_NOT_COMPLY, g_BUILD_VERSION);
/* NOTE: Passing possible NULL msg. PlugInExpecption requires allocated
or NULL. Literal strings are not supported. */
Copy link
Member

@mirichmo mirichmo Dec 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment seems unnecessary here too

throw new PlugInException(exitCode, msg);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/powershell-native/nativemsh/pwrshplugin/pwrshplugindefs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ class PlugInException
// the memory.
PWSTR extendedErrorInformation;

PlugInException(DWORD msgId, __in PWSTR msg)
PlugInException(DWORD msgId, __in_opt PWSTR msg)
{
dwMessageId = msgId;
extendedErrorInformation = msg;
Expand Down