maxmustermann/terminal
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Fix a bunch of static analysis issues (#553)

Browse source 
* static analysis fixes
* using C++ style casts
* explicit delete changed to reset(nullptr)
* fix for null apiMsg.OtherId during tracing in Compare()
* changed INVALID_ID macro to constexpr
* properly handle null ReplyMsg in ConsoleIoThread()
* Fixed wrong static_cast for State.InputBuffer
* compensate for null reply message to fix deref problem of ReplyMsg in srvinit.cpp by changing signature in DeviceComm.h
This commit is contained in:
Eric Budai 2019-05-23 13:35:30 -04:00 committed by Dustin L. Howett (MSFT)
parent 82e75ce3e2
commit 06a5583c86
15 changed files with 125 additions and 125 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
src/buffer/out/OutputCell.hpp
View file

@ -25,7 +25,7 @@ Author:
class InvalidCharInfoConversionException : public std::exception
{
const char* what() noexcept
const char* what() const noexcept
{
return "Cannot convert to CHAR_INFO without explicit TextAttribute";
}

2
src/host/ft_uia/KeyPressTests.cs
View file

@ -208,7 +208,7 @@ namespace Conhost.UIA.Tests
for (int i = 0; i < text.Count(); ++i)
{
string line = text.ElementAt(i);
line.Trim(' ');
line = line.Trim(' ');
if (!line.Equals(""))
{
possiblePromptLines.Add(line);

4
src/host/screenInfo.cpp
View file

@ -678,7 +678,7 @@ VOID SCREEN_INFORMATION::InternalUpdateScrollBars()
}
pWindow->UpdateScrollBar(true,
_IsAltBuffer() | gci.IsTerminalScrolling(),
_IsAltBuffer() || gci.IsTerminalScrolling(),
_viewport.Height(),
gci.IsTerminalScrolling() ? _virtualBottom : buffer.BottomInclusive(),
_viewport.Top());
@ -1280,7 +1280,7 @@ void SCREEN_INFORMATION::_AdjustViewportSize(const RECT* const prcClientNew,
const Viewport oldViewport = Viewport(_viewport);
_InternalSetViewportSize(pcoordSize, fResizeFromLeft, fResizeFromTop);
_InternalSetViewportSize(pcoordSize, fResizeFromTop, fResizeFromLeft);
// MSFT 13194969, related to 12092729.
// If we're in virtual terminal mode, and the viewport dimensions change,

2
src/host/srvinit.cpp
View file

@ -656,7 +656,7 @@ DWORD WINAPI ConsoleIoThread(LPVOID /*lpParameter*/)
}
// TODO: 9115192 correct mixed NTSTATUS/HRESULT
HRESULT hr = ServiceLocator::LocateGlobals().pDeviceComm->ReadIo(&ReplyMsg->Complete, &ReceiveMsg);
HRESULT hr = ServiceLocator::LocateGlobals().pDeviceComm->ReadIo(ReplyMsg, &ReceiveMsg);
if (FAILED(hr))
{
if (hr == HRESULT_FROM_WIN32(ERROR_PIPE_NOT_CONNECTED))

5
src/host/utf8ToWideCharParser.cpp
View file

@ -326,7 +326,10 @@ unsigned int Utf8ToWideCharParser::_ParseFullRange(_In_reads_(cb) const byte* co
LOG_LAST_ERROR();
_currentState = _State::Error;
}
_currentState = _State::Finished;
else
{
_currentState = _State::Finished;
}
}
return bufferSize;
}

6
src/interactivity/win32/UiaTextRange.cpp
View file

@ -20,7 +20,7 @@ using namespace Microsoft::Console::Interactivity::Win32::UiaTextRangeTracing;
//#define UIATEXTRANGE_DEBUG_MSGS 1
#undef UIATEXTRANGE_DEBUG_MSGS
IdType UiaTextRange::id = 0;
IdType UiaTextRange::id = 1;
UiaTextRange::MoveState::MoveState(const UiaTextRange& range,
const MovementDirection direction) :
@ -466,7 +466,7 @@ IFACEMETHODIMP UiaTextRange::Compare(_In_opt_ ITextRangeProvider* pRange, _Out_
}
// tracing
ApiMsgCompare apiMsg;
apiMsg.OtherId = other->GetId();
apiMsg.OtherId = other == nullptr ? InvalidId : other->GetId();
apiMsg.Equal = !!*pRetVal;
Tracing::s_TraceUia(this, ApiCall::Compare, &apiMsg);
@ -964,7 +964,7 @@ IFACEMETHODIMP UiaTextRange::MoveEndpointByRange(_In_ TextPatternRangeEndpoint e
}
ApiMsgMoveEndpointByRange apiMsg;
apiMsg.OriginalEnd = _start;
apiMsg.OriginalStart = _start;
apiMsg.OriginalEnd = _end;
apiMsg.Endpoint = endpoint;
apiMsg.TargetEndpoint = targetEndpoint;

3
src/interactivity/win32/UiaTextRange.hpp
View file

@ -29,7 +29,6 @@ Author(s):
class UiaTextRangeTests;
#endif
// The UiaTextRange deals with several data structures that have
// similar semantics. In order to keep the information from these data
// structures separated, each structure has its own naming for a
@ -68,6 +67,8 @@ typedef unsigned int Column;
// the first char of the 0th row in the text buffer row array.
typedef unsigned int Endpoint;
constexpr IdType InvalidId = 0;
namespace Microsoft::Console::Interactivity::Win32
{

204
src/interactivity/win32/window.cpp
View file

@ -239,133 +239,129 @@ NTSTATUS Window::_MakeWindow(_In_ Settings* const pSettings,
if (NT_SUCCESS(status))
{
if (NT_SUCCESS(status))
SCREEN_INFORMATION& siAttached = GetScreenInfo();
siAttached.RefreshFontWithRenderer();
// Save reference to settings
_pSettings = pSettings;
// Figure out coordinates and how big to make the window from the desired client viewport size
// Put left, top, right and bottom into rectProposed for checking against monitor screens below
RECT rectProposed = { pSettings->GetWindowOrigin().X, pSettings->GetWindowOrigin().Y, 0, 0 };
_CalculateWindowRect(pSettings->GetWindowSize(), &rectProposed); //returns with rectangle filled out
if (!WI_IsFlagSet(gci.Flags, CONSOLE_AUTO_POSITION))
{
SCREEN_INFORMATION& siAttached = GetScreenInfo();
siAttached.RefreshFontWithRenderer();
// Save reference to settings
_pSettings = pSettings;
// Figure out coordinates and how big to make the window from the desired client viewport size
// Put left, top, right and bottom into rectProposed for checking against monitor screens below
RECT rectProposed = { pSettings->GetWindowOrigin().X, pSettings->GetWindowOrigin().Y, 0, 0 };
_CalculateWindowRect(pSettings->GetWindowSize(), &rectProposed); //returns with rectangle filled out
if (!WI_IsFlagSet(gci.Flags, CONSOLE_AUTO_POSITION))
//if launched from a shortcut, ensure window is visible on screen
if (pSettings->IsStartupTitleIsLinkNameSet())
{
//if launched from a shortcut, ensure window is visible on screen
if (pSettings->IsStartupTitleIsLinkNameSet())
// if window would be fully OFFscreen, change position so it is ON screen.
// This doesn't change the actual coordinates
// stored in the link, just the starting position
// of the window.
// When the user reconnects the other monitor, the
// window will be where he left it. Great for take
// home laptop scenario.
if (!MonitorFromRect(&rectProposed, MONITOR_DEFAULTTONULL))
{
// if window would be fully OFFscreen, change position so it is ON screen.
// This doesn't change the actual coordinates
// stored in the link, just the starting position
// of the window.
// When the user reconnects the other monitor, the
// window will be where he left it. Great for take
// home laptop scenario.
if (!MonitorFromRect(&rectProposed, MONITOR_DEFAULTTONULL))
{
//Monitor we'll move to
HMONITOR hMon = MonitorFromRect(&rectProposed, MONITOR_DEFAULTTONEAREST);
MONITORINFO mi = { 0 };
//Monitor we'll move to
HMONITOR hMon = MonitorFromRect(&rectProposed, MONITOR_DEFAULTTONEAREST);
MONITORINFO mi = { 0 };
//get origin of monitor's workarea
mi.cbSize = sizeof(MONITORINFO);
GetMonitorInfo(hMon, &mi);
//get origin of monitor's workarea
mi.cbSize = sizeof(MONITORINFO);
GetMonitorInfo(hMon, &mi);
//Adjust right and bottom to new positions, relative to monitor workarea's origin
//Need to do this before adjusting left/top so RECT_* calculations are correct
rectProposed.right = mi.rcWork.left + RECT_WIDTH(&rectProposed);
rectProposed.bottom = mi.rcWork.top + RECT_HEIGHT(&rectProposed);
//Adjust right and bottom to new positions, relative to monitor workarea's origin
//Need to do this before adjusting left/top so RECT_* calculations are correct
rectProposed.right = mi.rcWork.left + RECT_WIDTH(&rectProposed);
rectProposed.bottom = mi.rcWork.top + RECT_HEIGHT(&rectProposed);
// Move origin to top left of nearest
// monitor's WORKAREA (accounting for taskbar
// and any app toolbars)
rectProposed.left = mi.rcWork.left;
rectProposed.top = mi.rcWork.top;
}
// Move origin to top left of nearest
// monitor's WORKAREA (accounting for taskbar
// and any app toolbars)
rectProposed.left = mi.rcWork.left;
rectProposed.top = mi.rcWork.top;
}
}
}
// Attempt to create window
HWND hWnd = CreateWindowExW(
CONSOLE_WINDOW_EX_FLAGS,
CONSOLE_WINDOW_CLASS,
gci.GetTitle().c_str(),
CONSOLE_WINDOW_FLAGS,
WI_IsFlagSet(gci.Flags,
CONSOLE_AUTO_POSITION) ? CW_USEDEFAULT : rectProposed.left,
rectProposed.top, // field is ignored if CW_USEDEFAULT was chosen above
RECT_WIDTH(&rectProposed),
RECT_HEIGHT(&rectProposed),
HWND_DESKTOP,
nullptr,
nullptr,
this // handle to this window class, passed to WM_CREATE to help dispatching to this instance
);
// Attempt to create window
HWND hWnd = CreateWindowExW(
CONSOLE_WINDOW_EX_FLAGS,
CONSOLE_WINDOW_CLASS,
gci.GetTitle().c_str(),
CONSOLE_WINDOW_FLAGS,
WI_IsFlagSet(gci.Flags, CONSOLE_AUTO_POSITION) ? CW_USEDEFAULT : rectProposed.left,
rectProposed.top, // field is ignored if CW_USEDEFAULT was chosen above
RECT_WIDTH(&rectProposed),
RECT_HEIGHT(&rectProposed),
HWND_DESKTOP,
nullptr,
nullptr,
this // handle to this window class, passed to WM_CREATE to help dispatching to this instance
);
if (hWnd == nullptr)
if (hWnd == nullptr)
{
DWORD const gle = GetLastError();
RIPMSG1(RIP_WARNING, "CreateWindow failed with gle = 0x%x", gle);
status = NTSTATUS_FROM_WIN32(gle);
}
if (NT_SUCCESS(status))
{
_hWnd = hWnd;
if (useDx)
{
DWORD const gle = GetLastError();
RIPMSG1(RIP_WARNING, "CreateWindow failed with gle = 0x%x", gle);
status = NTSTATUS_FROM_WIN32(gle);
status = NTSTATUS_FROM_HRESULT(pDxEngine->SetHwnd(hWnd));
if (NT_SUCCESS(status))
{
status = NTSTATUS_FROM_HRESULT(pDxEngine->Enable());
}
}
else
{
status = NTSTATUS_FROM_HRESULT(pGdiEngine->SetHwnd(hWnd));
}
if (NT_SUCCESS(status))
{
_hWnd = hWnd;
// Set alpha on window if requested
ApplyWindowOpacity();
if (useDx)
{
status = NTSTATUS_FROM_HRESULT(pDxEngine->SetHwnd(hWnd));
if (NT_SUCCESS(status))
{
status = NTSTATUS_FROM_HRESULT(pDxEngine->Enable());
}
}
else
{
status = NTSTATUS_FROM_HRESULT(pGdiEngine->SetHwnd(hWnd));
}
status = Menu::CreateInstance(hWnd);
if (NT_SUCCESS(status))
{
// Set alpha on window if requested
ApplyWindowOpacity();
gci.ConsoleIme.RefreshAreaAttributes();
status = Menu::CreateInstance(hWnd);
// Do WM_GETICON workaround. Must call WM_SETICON once or apps calling WM_GETICON will get null.
LOG_IF_FAILED(Icon::Instance().ApplyWindowMessageWorkaround(hWnd));
if (NT_SUCCESS(status))
// Set up the hot key for this window.
if (gci.GetHotKey() != 0)
{
gci.ConsoleIme.RefreshAreaAttributes();
// Do WM_GETICON workaround. Must call WM_SETICON once or apps calling WM_GETICON will get null.
LOG_IF_FAILED(Icon::Instance().ApplyWindowMessageWorkaround(hWnd));
// Set up the hot key for this window.
if (gci.GetHotKey() != 0)
{
SendMessageW(hWnd, WM_SETHOTKEY, gci.GetHotKey(), 0);
}
ServiceLocator::LocateHighDpiApi<WindowDpiApi>()->EnableChildWindowDpiMessage(_hWnd, TRUE /*fEnable*/);
// Post a window size update so that the new console window will size itself correctly once it's up and
// running. This works around chicken & egg cases involving window size calculations having to do with font
// sizes, DPI, and non-primary monitors (see MSFT #2367234).
siAttached.PostUpdateWindowSize();
// Locate window theming modules and try to set the dark mode.
try
{
WindowTheme theme;
LOG_IF_FAILED(theme.TrySetDarkMode(_hWnd));
}
CATCH_LOG();
SendMessageW(hWnd, WM_SETHOTKEY, gci.GetHotKey(), 0);
}
ServiceLocator::LocateHighDpiApi<WindowDpiApi>()->EnableChildWindowDpiMessage(_hWnd, TRUE /*fEnable*/);
// Post a window size update so that the new console window will size itself correctly once it's up and
// running. This works around chicken & egg cases involving window size calculations having to do with font
// sizes, DPI, and non-primary monitors (see MSFT #2367234).
siAttached.PostUpdateWindowSize();
// Locate window theming modules and try to set the dark mode.
try
{
WindowTheme theme;
LOG_IF_FAILED(theme.TrySetDarkMode(_hWnd));
}
CATCH_LOG();
}
}
}

2
src/propsheet/fontdlg.cpp
View file

@ -134,7 +134,7 @@ BOOL IsBoldOnlyTTFont(_In_ PCWSTR pwszTTFace, _In_opt_ PCWSTR pwszAltTTFace)
// only care if this TT font's name matches
if ((0 != lstrcmp(FontInfo[i].FaceName, pwszTTFace)) && // wrong face name and
(pwszAltTTFace != NULL || // either pwszAltTTFace is NULL or
(pwszAltTTFace == NULL || // either pwszAltTTFace is NULL or
(0 != lstrcmp(FontInfo[i].FaceName, pwszAltTTFace)))) // pwszAltTTFace is wrong too
{
// A TrueType font, but not the one we're interested in

2
src/propslib/RegistrySerialization.cpp
View file

@ -395,7 +395,7 @@ NTSTATUS RegistrySerialization::s_UpdateValue(const HKEY hConsoleKey,
bool fDeleteKey = false;
if (hConsoleKey != hKey)
{
Status = s_QueryValue(hConsoleKey, pwszValueName, sizeof(Data), dwType, Data, nullptr);
Status = s_QueryValue(hConsoleKey, pwszValueName, cbDataLength, dwType, Data, nullptr);
if (NT_SUCCESS(Status))
{
fDeleteKey = (memcmp(pbData, Data, cbDataLength) == 0);

4
src/renderer/base/thread.cpp
View file

@ -41,13 +41,13 @@ RenderThread::~RenderThread()
if (_hPaintEnabledEvent != INVALID_HANDLE_VALUE)
{
CloseHandle(_hPaintEnabledEvent);
_hEvent = INVALID_HANDLE_VALUE;
_hPaintEnabledEvent = INVALID_HANDLE_VALUE;
}
if (_hPaintCompletedEvent != INVALID_HANDLE_VALUE)
{
CloseHandle(_hPaintCompletedEvent);
_hEvent = INVALID_HANDLE_VALUE;
_hPaintCompletedEvent = INVALID_HANDLE_VALUE;
}
}

2
src/renderer/vt/tracing.cpp
View file

@ -43,7 +43,7 @@ std::string toPrintableString(const std::string_view& inString)
for (size_t i = 0; i < inString.length(); i++)
{
unsigned char c = inString[i];
if (c < '\x20' && c < '\x7f')
if (c < '\x20')
{
retval += "^";
char actual = (c + 0x40);

4
src/server/ApiMessage.cpp
View file

@ -153,7 +153,7 @@ HRESULT _CONSOLE_API_MSG::ReleaseMessageBuffers()
if (State.InputBuffer != nullptr)
{
delete[] State.InputBuffer;
delete[] static_cast<BYTE*>(State.InputBuffer);
State.InputBuffer = nullptr;
}
@ -170,7 +170,7 @@ HRESULT _CONSOLE_API_MSG::ReleaseMessageBuffers()
LOG_IF_FAILED(_pDeviceComm->WriteOutput(&IoOperation));
}
delete[] State.OutputBuffer;
delete[] static_cast<BYTE*>(State.OutputBuffer);
State.OutputBuffer = nullptr;
}

6
src/server/DeviceComm.cpp
View file

@ -41,12 +41,12 @@ HRESULT DeviceComm::SetServerInformation(_In_ CD_IO_SERVER_INFORMATION* const pS
// Return Value:
// - HRESULT S_OK or suitable error.
[[nodiscard]]
HRESULT DeviceComm::ReadIo(_In_opt_ CD_IO_COMPLETE* const pCompletion,
HRESULT DeviceComm::ReadIo(_In_opt_ PCONSOLE_API_MSG const pReplyMsg,
_Out_ CONSOLE_API_MSG* const pMessage) const
{
HRESULT hr = _CallIoctl(IOCTL_CONDRV_READ_IO,
pCompletion,
pCompletion == nullptr ? 0 : sizeof(*pCompletion),
pReplyMsg == nullptr ? nullptr : &pReplyMsg->Complete,
pReplyMsg == nullptr ? 0 : sizeof(pReplyMsg->Complete),
&pMessage->Descriptor,
sizeof(CONSOLE_API_MSG) - FIELD_OFFSET(CONSOLE_API_MSG, Descriptor));

2
src/server/DeviceComm.h
View file

@ -29,7 +29,7 @@ public:
[[nodiscard]]
HRESULT SetServerInformation(_In_ CD_IO_SERVER_INFORMATION* const pServerInfo) const;
[[nodiscard]]
HRESULT ReadIo(_In_opt_ CD_IO_COMPLETE* const pCompletion,
HRESULT ReadIo(_In_opt_ PCONSOLE_API_MSG const pReplyMsg,
_Out_ CONSOLE_API_MSG* const pMessage) const;
[[nodiscard]]
HRESULT CompleteIo(_In_ CD_IO_COMPLETE* const pCompletion) const;