address a few more issues found by static analysis

* mark move constructors and operators noexcept.
* eliminate large stack allocations in several places.
* some incorrect checks for Windows handles.
This commit is contained in:
Christoph Oelckers 2024-01-06 12:25:16 +01:00
parent 763259d654
commit 406cb04952
12 changed files with 70 additions and 36 deletions

View file

@ -299,6 +299,13 @@ if( MSVC )
# These must be silenced because the code is full of them. Expect some of undefined behavior hidden in this mess. :( # These must be silenced because the code is full of them. Expect some of undefined behavior hidden in this mess. :(
#set( ALL_C_FLAGS "${ALL_C_FLAGS} /wd4244 /wd4018 /wd4267" ) #set( ALL_C_FLAGS "${ALL_C_FLAGS} /wd4244 /wd4018 /wd4267" )
# annoying intellisense warnings that are virtually always false positives and mostly happen in third party code.
#6297: overflow when shifting
#6386: Buffer overrun when writing (i.e. missing range check for array writes)
#6385: Reading invalid data (i.e. missing range check for array reads)
#6308: missing null check on realloc
set( ALL_C_FLAGS "${ALL_C_FLAGS} /wd6297 /wd6386 /wd6385 /wd6308" )
# Most of these need to be cleaned out. The source is currently infested with far too much conditional compilation which is a constant source of problems. # Most of these need to be cleaned out. The source is currently infested with far too much conditional compilation which is a constant source of problems.
set( ALL_C_FLAGS "${ALL_C_FLAGS} /DUSE_OPENGL=1 /DNOASM=1 /DWIN32" ) set( ALL_C_FLAGS "${ALL_C_FLAGS} /DUSE_OPENGL=1 /DNOASM=1 /DWIN32" )

View file

@ -465,19 +465,16 @@ const FSoundFontInfo *FSoundFontManager::FindSoundFont(const char *name, int all
// //
//========================================================================== //==========================================================================
FSoundFontReader *FSoundFontManager::OpenSoundFont(const char *name, int allowed) FSoundFontReader *FSoundFontManager::OpenSoundFont(const char *const name, int allowed)
{ {
if (name == nullptr) return nullptr;
// First check if the given name is inside the loaded resources. // First check if the given name is inside the loaded resources.
// To avoid clashes this will only be done if the name has the '.cfg' extension. // To avoid clashes this will only be done if the name has the '.cfg' extension.
// Sound fonts cannot be loaded this way. // Sound fonts cannot be loaded this way.
if (name != nullptr) const char *p = name + strlen(name) - 4;
if (p > name && !stricmp(p, ".cfg") && fileSystem.CheckNumForFullName(name) >= 0)
{ {
const char *p = name + strlen(name) - 4; return new FLumpPatchSetReader(name);
if (p > name && !stricmp(p, ".cfg") && fileSystem.CheckNumForFullName(name) >= 0)
{
return new FLumpPatchSetReader(name);
}
} }
// Next check if the file is a .sf file // Next check if the file is a .sf file

View file

@ -432,14 +432,14 @@ static FString ReplayGainHash(ZMusicCustomReader* reader, int flength, int playe
{ {
std::string playparam = _playparam; std::string playparam = _playparam;
uint8_t buffer[50000]; // for performance reasons only hash the start of the file. If we wanted to do this to large waveform songs it'd cause noticable lag. TArray<uint8_t> buffer(50000, true); // for performance reasons only hash the start of the file. If we wanted to do this to large waveform songs it'd cause noticable lag.
uint8_t digest[16]; uint8_t digest[16];
char digestout[33]; char digestout[33];
auto length = reader->read(reader, buffer, 50000); auto length = reader->read(reader, buffer.data(), 50000);
reader->seek(reader, 0, SEEK_SET); reader->seek(reader, 0, SEEK_SET);
MD5Context md5; MD5Context md5;
md5.Init(); md5.Init();
md5.Update(buffer, (int)length); md5.Update(buffer.data(), (int)length);
md5.Final(digest); md5.Final(digest);
for (size_t j = 0; j < sizeof(digest); ++j) for (size_t j = 0; j < sizeof(digest); ++j)
@ -448,7 +448,7 @@ static FString ReplayGainHash(ZMusicCustomReader* reader, int flength, int playe
} }
digestout[32] = 0; digestout[32] = 0;
auto type = ZMusic_IdentifyMIDIType((uint32_t*)buffer, 32); auto type = ZMusic_IdentifyMIDIType((uint32_t*)buffer.data(), 32);
if (type == MIDI_NOTMIDI) return FStringf("%d:%s", flength, digestout); if (type == MIDI_NOTMIDI) return FStringf("%d:%s", flength, digestout);
// get the default for MIDI synth // get the default for MIDI synth

View file

@ -45,6 +45,7 @@
#include <stdlib.h> #include <stdlib.h>
#include <assert.h> #include <assert.h>
#include <memory>
#include "ancientzip.h" #include "ancientzip.h"
namespace FileSys { namespace FileSys {
@ -342,10 +343,19 @@ int FZipExploder::Explode(unsigned char *out, unsigned int outsize,
int ShrinkLoop(unsigned char *out, unsigned int outsize, FileReader &_In, unsigned int InLeft) int ShrinkLoop(unsigned char *out, unsigned int outsize, FileReader &_In, unsigned int InLeft)
{ {
// don't allocate this on the stack, it's a bit on the large side.
struct work
{
unsigned char ReadBuf[256];
unsigned short Parent[HSIZE];
unsigned char Value[HSIZE], Stack[HSIZE];
};
auto s = std::make_unique<work>();
unsigned char* ReadBuf = s->ReadBuf;
unsigned short* Parent = s->Parent;
unsigned char* Value = s->Value, * Stack = s->Stack;
FileReader *In = &_In; FileReader *In = &_In;
unsigned char ReadBuf[256];
unsigned short Parent[HSIZE];
unsigned char Value[HSIZE], Stack[HSIZE];
unsigned char *newstr; unsigned char *newstr;
int len; int len;
int KwKwK, codesize = 9; /* start at 9 bits/code */ int KwKwK, codesize = 9; /* start at 9 bits/code */

View file

@ -381,12 +381,14 @@ static HANDLE WriteMyMiniDump (void)
{ {
MiniDumpThreadData dumpdata = { file, pMiniDumpWriteDump, &exceptor }; MiniDumpThreadData dumpdata = { file, pMiniDumpWriteDump, &exceptor };
DWORD id; DWORD id;
HANDLE thread = CreateThread (NULL, 0, WriteMiniDumpInAnotherThread, HANDLE thread = CreateThread (NULL, 0, WriteMiniDumpInAnotherThread, &dumpdata, 0, &id);
&dumpdata, 0, &id); if (thread != nullptr)
WaitForSingleObject (thread, INFINITE);
if (GetExitCodeThread (thread, &id))
{ {
good = id; WaitForSingleObject(thread, INFINITE);
if (GetExitCodeThread(thread, &id))
{
good = id;
}
} }
} }
} }

View file

@ -421,7 +421,8 @@ void FDInputJoystick::ProcessInput()
return; return;
} }
state = (uint8_t *)alloca(DataFormat.dwDataSize); TArray<uint8_t> statearr(DataFormat.dwDataSize, true);
state = statearr.data();
hr = Device->GetDeviceState(DataFormat.dwDataSize, state); hr = Device->GetDeviceState(DataFormat.dwDataSize, state);
if (FAILED(hr)) if (FAILED(hr))
return; return;

View file

@ -334,7 +334,8 @@ LRESULT CALLBACK WndProc (HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam)
if (!GetRawInputData((HRAWINPUT)lParam, RID_INPUT, NULL, &size, sizeof(RAWINPUTHEADER)) && if (!GetRawInputData((HRAWINPUT)lParam, RID_INPUT, NULL, &size, sizeof(RAWINPUTHEADER)) &&
size != 0) size != 0)
{ {
uint8_t *buffer = (uint8_t *)alloca(size); TArray<uint8_t> array(size, true);
uint8_t *buffer = array.data();
if (GetRawInputData((HRAWINPUT)lParam, RID_INPUT, buffer, &size, sizeof(RAWINPUTHEADER)) == size) if (GetRawInputData((HRAWINPUT)lParam, RID_INPUT, buffer, &size, sizeof(RAWINPUTHEADER)) == size)
{ {
int code = GET_RAWINPUT_CODE_WPARAM(wParam); int code = GET_RAWINPUT_CODE_WPARAM(wParam);
@ -691,12 +692,15 @@ void I_PutInClipboard (const char *str)
auto wstr = WideString(str); auto wstr = WideString(str);
HGLOBAL cliphandle = GlobalAlloc (GMEM_DDESHARE, wstr.length() * 2 + 2); HGLOBAL cliphandle = GlobalAlloc (GMEM_DDESHARE, wstr.length() * 2 + 2);
if (cliphandle != NULL) if (cliphandle != nullptr)
{ {
wchar_t *ptr = (wchar_t *)GlobalLock (cliphandle); wchar_t *ptr = (wchar_t *)GlobalLock (cliphandle);
wcscpy (ptr, wstr.c_str()); if (ptr)
GlobalUnlock (cliphandle); {
SetClipboardData (CF_UNICODETEXT, cliphandle); wcscpy(ptr, wstr.c_str());
GlobalUnlock(cliphandle);
SetClipboardData(CF_UNICODETEXT, cliphandle);
}
} }
CloseClipboard (); CloseClipboard ();
} }

View file

@ -284,12 +284,12 @@ int DoMain (HINSTANCE hInstance)
HANDLE stdinput = GetStdHandle(STD_INPUT_HANDLE); HANDLE stdinput = GetStdHandle(STD_INPUT_HANDLE);
ShowWindow(mainwindow.GetHandle(), SW_HIDE); ShowWindow(mainwindow.GetHandle(), SW_HIDE);
WriteFile(StdOut, "Press any key to exit...", 24, &bytes, NULL); if (StdOut != nullptr) WriteFile(StdOut, "Press any key to exit...", 24, &bytes, nullptr);
FlushConsoleInputBuffer(stdinput); FlushConsoleInputBuffer(stdinput);
SetConsoleMode(stdinput, 0); SetConsoleMode(stdinput, 0);
ReadConsole(stdinput, &bytes, 1, &bytes, NULL); ReadConsole(stdinput, &bytes, 1, &bytes, NULL);
} }
else if (StdOut == NULL) else if (StdOut == nullptr)
{ {
mainwindow.ShowErrorPane(nullptr); mainwindow.ShowErrorPane(nullptr);
} }

View file

@ -77,10 +77,19 @@ static void CheckOpenGL(void)
if (opengl32dll == 0) if (opengl32dll == 0)
{ {
opengl32dll = LoadLibrary(L"OpenGL32.DLL"); opengl32dll = LoadLibrary(L"OpenGL32.DLL");
createcontext = (HGLRC(WINAPI*)(HDC)) GetProcAddress(opengl32dll, "wglCreateContext"); if (opengl32dll != 0)
deletecontext = (BOOL(WINAPI*)(HGLRC)) GetProcAddress(opengl32dll, "wglDeleteContext"); {
makecurrent = (BOOL(WINAPI*)(HDC, HGLRC)) GetProcAddress(opengl32dll, "wglMakeCurrent"); createcontext = (HGLRC(WINAPI*)(HDC)) GetProcAddress(opengl32dll, "wglCreateContext");
getprocaddress = (PROC(WINAPI*)(LPCSTR)) GetProcAddress(opengl32dll, "wglGetProcAddress"); deletecontext = (BOOL(WINAPI*)(HGLRC)) GetProcAddress(opengl32dll, "wglDeleteContext");
makecurrent = (BOOL(WINAPI*)(HDC, HGLRC)) GetProcAddress(opengl32dll, "wglMakeCurrent");
getprocaddress = (PROC(WINAPI*)(LPCSTR)) GetProcAddress(opengl32dll, "wglGetProcAddress");
}
else
{
// Should this ever happen we have no choice but to hard abort, there is no good way to recover.
MessageBoxA(0, "OpenGL32.dll not found", "Fatal error", MB_OK | MB_ICONERROR | MB_TASKMODAL);
exit(3);
}
} }
} }

View file

@ -39,6 +39,7 @@
#ifdef HAVE_MMX #ifdef HAVE_MMX
#include "hqnx_asm/hqnx_asm.h" #include "hqnx_asm/hqnx_asm.h"
#endif #endif
#include <memory>
#include "xbr/xbrz.h" #include "xbr/xbrz.h"
#include "xbr/xbrz_old.h" #include "xbr/xbrz_old.h"
#include "parallel_for.h" #include "parallel_for.h"
@ -304,7 +305,8 @@ static unsigned char *hqNxAsmHelper( void (*hqNxFunction) ( int*, unsigned char*
initdone = true; initdone = true;
} }
HQnX_asm::CImage cImageIn; auto pImageIn = std::make_unique<HQnX_asm::CImage>();
auto& cImageIn = *pImageIn;
cImageIn.SetImage(inputBuffer, inWidth, inHeight, 32); cImageIn.SetImage(inputBuffer, inWidth, inHeight, 32);
cImageIn.Convert32To17(); cImageIn.Convert32To17();

View file

@ -510,7 +510,8 @@ bool M_ReadIDAT (FileReader &file, uint8_t *buffer, int width, int height, int p
{ {
i += bytesPerRowOut * 2; i += bytesPerRowOut * 2;
} }
inputLine = (Byte *)alloca (i); TArray<Byte> inputArray(i, true);
inputLine = inputArray.data();
adam7buff[0] = inputLine + 4 + bytesPerRowOut; adam7buff[0] = inputLine + 4 + bytesPerRowOut;
adam7buff[1] = adam7buff[0] + bytesPerRowOut; adam7buff[1] = adam7buff[0] + bytesPerRowOut;
adam7buff[2] = adam7buff[1] + bytesPerRowOut; adam7buff[2] = adam7buff[1] + bytesPerRowOut;
@ -925,7 +926,8 @@ bool M_SaveBitmap(const uint8_t *from, ESSType color_type, int width, int height
temprow[i] = &temprow_storage[temprow_size * i]; temprow[i] = &temprow_storage[temprow_size * i];
} }
Byte buffer[PNG_WRITE_SIZE]; TArray<Byte> array(PNG_WRITE_SIZE, true);
auto buffer = array.data();
z_stream stream; z_stream stream;
int err; int err;
int y; int y;

View file

@ -282,7 +282,7 @@ GainAnalyzer::ResetSampleFrequency(int samplefreq) {
int int
GainAnalyzer::InitGainAnalysis(int samplefreq) { GainAnalyzer::InitGainAnalysis(int samplefreq) {
*this = {}; memset(this, 0, sizeof(*this));
if (ResetSampleFrequency(samplefreq) != INIT_GAIN_ANALYSIS_OK) { if (ResetSampleFrequency(samplefreq) != INIT_GAIN_ANALYSIS_OK) {
return INIT_GAIN_ANALYSIS_ERROR; return INIT_GAIN_ANALYSIS_ERROR;
} }