Fixes made in response to running a static code analyzer under MS Windows.

Most of these are probably harmless, but the changes to SDL_immdevice.c and SDL_pixels.c appear to have fixed genuine bugs.

SDL_audiocvt.c: By separating the calculation of the divisor, I got rid of the suspicion that dividing a double by an integer led to loss of precision.
SDL_immdevice.c: Added a missing test, one that could have otherwise led to dereferencing a null pointer.
SDL_events.c, SDL_gamecontroller.c, SDL_joystick.c, SDL_malloc.c, SDL_video.c: Made it clear the return values weren't used.
SDL_hidapi_shield.c: The size is zero, so nothing bad would have happened, but the SDL_memset() was still being given an address outside of the array's range.
SDL_dinputjoystick.c: Initialize local data, just in case IDirectInputDevice8_GetProperty() isn't guaranteed to write to it.
SDL_render_sw.c: drawstate.viewport could be null (as seen on line 691).
SDL.c: SDL_MostSignificantBitIndex32() could return -1, though I don't know if you want to cope with that (what I did) or SDL_assert() that it can't happen.
SDL_hints.c: Replaced boolean tests on pointer values with comparisons to NULL.
SDL_pixels.c: Looks like the switch is genuinely missing a break!
SDL_rect_impl.h: The MacOS static checker pointed out issues with the X comparisons that were handled by assertions; I added assertions for the Y comparisons.
SDL_yuv.c, SDL_windowskeyboard.c, SDL_windowswindow.c: Checked error-result returns.
This commit is contained in:
ulatekh 2022-10-05 19:26:09 -07:00 committed by Ryan C. Gordon
parent 7ebdae5dc9
commit ec58a817ef
17 changed files with 63 additions and 41 deletions

View File

@ -125,8 +125,9 @@ static void
SDL_PrivateSubsystemRefCountIncr(Uint32 subsystem)
{
int subsystem_index = SDL_MostSignificantBitIndex32(subsystem);
SDL_assert(SDL_SubsystemRefCount[subsystem_index] < 255);
++SDL_SubsystemRefCount[subsystem_index];
SDL_assert(subsystem_index < 0 || SDL_SubsystemRefCount[subsystem_index] < 255);
if (subsystem_index >= 0)
++SDL_SubsystemRefCount[subsystem_index];
}
/* Private helper to decrement a subsystem's ref counter. */
@ -134,7 +135,7 @@ static void
SDL_PrivateSubsystemRefCountDecr(Uint32 subsystem)
{
int subsystem_index = SDL_MostSignificantBitIndex32(subsystem);
if (SDL_SubsystemRefCount[subsystem_index] > 0) {
if (subsystem_index >= 0 && SDL_SubsystemRefCount[subsystem_index] > 0) {
--SDL_SubsystemRefCount[subsystem_index];
}
}
@ -144,22 +145,22 @@ static SDL_bool
SDL_PrivateShouldInitSubsystem(Uint32 subsystem)
{
int subsystem_index = SDL_MostSignificantBitIndex32(subsystem);
SDL_assert(SDL_SubsystemRefCount[subsystem_index] < 255);
return (SDL_SubsystemRefCount[subsystem_index] == 0) ? SDL_TRUE : SDL_FALSE;
SDL_assert(subsystem_index < 0 || SDL_SubsystemRefCount[subsystem_index] < 255);
return (subsystem_index >= 0 && SDL_SubsystemRefCount[subsystem_index] == 0) ? SDL_TRUE : SDL_FALSE;
}
/* Private helper to check if a system needs to be quit. */
static SDL_bool
SDL_PrivateShouldQuitSubsystem(Uint32 subsystem) {
int subsystem_index = SDL_MostSignificantBitIndex32(subsystem);
if (SDL_SubsystemRefCount[subsystem_index] == 0) {
if (subsystem_index >= 0 && SDL_SubsystemRefCount[subsystem_index] == 0) {
return SDL_FALSE;
}
/* If we're in SDL_Quit, we shut down every subsystem, even if refcount
* isn't zero.
*/
return (SDL_SubsystemRefCount[subsystem_index] == 1 || SDL_bInMainQuit) ? SDL_TRUE : SDL_FALSE;
return ((subsystem_index >= 0 && SDL_SubsystemRefCount[subsystem_index] == 1) || SDL_bInMainQuit) ? SDL_TRUE : SDL_FALSE;
}
void

View File

@ -112,7 +112,7 @@ SDL_ResetHint(const char *name)
if (SDL_strcmp(name, hint->name) == 0) {
if ((env == NULL && hint->value != NULL) ||
(env != NULL && hint->value == NULL) ||
(env && SDL_strcmp(env, hint->value) != 0)) {
(env != NULL && SDL_strcmp(env, hint->value) != 0)) {
for (entry = hint->callbacks; entry; ) {
/* Save the next entry in case this one is deleted */
SDL_HintWatch *next = entry->next;
@ -140,7 +140,7 @@ SDL_ResetHints(void)
env = SDL_getenv(hint->name);
if ((env == NULL && hint->value != NULL) ||
(env != NULL && hint->value == NULL) ||
(env && SDL_strcmp(env, hint->value) != 0)) {
(env != NULL && SDL_strcmp(env, hint->value) != 0)) {
for (entry = hint->callbacks; entry; ) {
/* Save the next entry in case this one is deleted */
SDL_HintWatch *next = entry->next;
@ -169,7 +169,7 @@ SDL_GetHint(const char *name)
env = SDL_getenv(name);
for (hint = SDL_hints; hint; hint = hint->next) {
if (SDL_strcmp(name, hint->name) == 0) {
if (!env || hint->priority == SDL_HINT_OVERRIDE) {
if (env == NULL || hint->priority == SDL_HINT_OVERRIDE) {
return hint->value;
}
break;

View File

@ -403,7 +403,8 @@ SDL_BuildAudioTypeCVTFromFloat(SDL_AudioCVT *cvt, const SDL_AudioFormat dst_fmt)
cvt->len_mult *= mult;
cvt->len_ratio *= mult;
} else if (src_bitsize > dst_bitsize) {
cvt->len_ratio /= (src_bitsize / dst_bitsize);
const int div = (src_bitsize / dst_bitsize);
cvt->len_ratio /= div;
}
retval = 1; /* added a converter. */
}

View File

@ -399,7 +399,9 @@ static int SDLCALL sort_endpoints(const void *_a, const void *_b)
{
LPWSTR a = ((const EndpointItem *)_a)->devid;
LPWSTR b = ((const EndpointItem *)_b)->devid;
if (!a && b) {
if (!a && !b) {
return 0;
} else if (!a && b) {
return -1;
} else if (a && !b) {
return 1;

View File

@ -147,7 +147,7 @@ SDL_AutoUpdateSensorsChanged(void *userdata, const char *name, const char *oldVa
static void SDLCALL
SDL_PollSentinelChanged(void *userdata, const char *name, const char *oldValue, const char *hint)
{
SDL_EventState(SDL_POLLSENTINEL, SDL_GetStringBoolean(hint, SDL_TRUE) ? SDL_ENABLE : SDL_DISABLE);
(void)SDL_EventState(SDL_POLLSENTINEL, SDL_GetStringBoolean(hint, SDL_TRUE) ? SDL_ENABLE : SDL_DISABLE);
}
/**
@ -566,12 +566,12 @@ SDL_StartEventLoop(void)
#endif /* !SDL_THREADS_DISABLED */
/* Process most event types */
SDL_EventState(SDL_TEXTINPUT, SDL_DISABLE);
SDL_EventState(SDL_TEXTEDITING, SDL_DISABLE);
SDL_EventState(SDL_SYSWMEVENT, SDL_DISABLE);
(void)SDL_EventState(SDL_TEXTINPUT, SDL_DISABLE);
(void)SDL_EventState(SDL_TEXTEDITING, SDL_DISABLE);
(void)SDL_EventState(SDL_SYSWMEVENT, SDL_DISABLE);
#if 0 /* Leave these events enabled so apps can respond to items being dragged onto them at startup */
SDL_EventState(SDL_DROPFILE, SDL_DISABLE);
SDL_EventState(SDL_DROPTEXT, SDL_DISABLE);
(void)SDL_EventState(SDL_DROPFILE, SDL_DISABLE);
(void)SDL_EventState(SDL_DROPTEXT, SDL_DISABLE);
#endif
SDL_EventQ.active = SDL_TRUE;

View File

@ -3009,7 +3009,7 @@ SDL_GameControllerEventState(int state)
break;
default:
for (i = 0; i < SDL_arraysize(event_list); ++i) {
SDL_EventState(event_list[i], state);
(void)SDL_EventState(event_list[i], state);
}
break;
}

View File

@ -1798,7 +1798,7 @@ SDL_JoystickEventState(int state)
break;
default:
for (i = 0; i < SDL_arraysize(event_list); ++i) {
SDL_EventState(event_list[i], state);
(void)SDL_EventState(event_list[i], state);
}
break;
}

View File

@ -169,7 +169,8 @@ HIDAPI_DriverShield_SendCommand(SDL_HIDAPI_Device *device, Uint8 cmd, const void
}
/* Zero unused data in the payload */
SDL_memset(&cmd_pkt.payload[size], 0, sizeof(cmd_pkt.payload) - size);
if (size != sizeof(cmd_pkt.payload))
SDL_memset(&cmd_pkt.payload[size], 0, sizeof(cmd_pkt.payload) - size);
if (SDL_HIDAPI_SendRumbleAndUnlock(device, (Uint8*)&cmd_pkt, sizeof(cmd_pkt)) != sizeof(cmd_pkt)) {
return SDL_SetError("Couldn't send command packet");

View File

@ -329,6 +329,7 @@ QueryDeviceInfo(LPDIRECTINPUTDEVICE8 device, Uint16* vendor_id, Uint16* product_
dipdw.diph.dwHeaderSize = sizeof(dipdw.diph);
dipdw.diph.dwObj = 0;
dipdw.diph.dwHow = DIPH_DEVICE;
dipdw.dwData = 0;
if (FAILED(IDirectInputDevice8_GetProperty(device, DIPROP_VIDPID, &dipdw.diph))) {
return SDL_FALSE;

View File

@ -733,7 +733,7 @@ SW_RunCommandQueue(SDL_Renderer * renderer, SDL_RenderCommand *cmd, void *vertic
SetDrawState(surface, &drawstate);
/* Apply viewport */
if (drawstate.viewport->x || drawstate.viewport->y) {
if (drawstate.viewport != NULL && (drawstate.viewport->x || drawstate.viewport->y)) {
int i;
for (i = 0; i < count; i++) {
verts[i].x += drawstate.viewport->x;
@ -760,7 +760,7 @@ SW_RunCommandQueue(SDL_Renderer * renderer, SDL_RenderCommand *cmd, void *vertic
SetDrawState(surface, &drawstate);
/* Apply viewport */
if (drawstate.viewport->x || drawstate.viewport->y) {
if (drawstate.viewport != NULL && (drawstate.viewport->x || drawstate.viewport->y)) {
int i;
for (i = 0; i < count; i++) {
verts[i].x += drawstate.viewport->x;
@ -787,7 +787,7 @@ SW_RunCommandQueue(SDL_Renderer * renderer, SDL_RenderCommand *cmd, void *vertic
SetDrawState(surface, &drawstate);
/* Apply viewport */
if (drawstate.viewport->x || drawstate.viewport->y) {
if (drawstate.viewport != NULL && (drawstate.viewport->x || drawstate.viewport->y)) {
int i;
for (i = 0; i < count; i++) {
verts[i].x += drawstate.viewport->x;
@ -815,7 +815,7 @@ SW_RunCommandQueue(SDL_Renderer * renderer, SDL_RenderCommand *cmd, void *vertic
PrepTextureForCopy(cmd);
/* Apply viewport */
if (drawstate.viewport->x || drawstate.viewport->y) {
if (drawstate.viewport != NULL && (drawstate.viewport->x || drawstate.viewport->y)) {
dstrect->x += drawstate.viewport->x;
dstrect->y += drawstate.viewport->y;
}
@ -873,7 +873,7 @@ SW_RunCommandQueue(SDL_Renderer * renderer, SDL_RenderCommand *cmd, void *vertic
PrepTextureForCopy(cmd);
/* Apply viewport */
if (drawstate.viewport->x || drawstate.viewport->y) {
if (drawstate.viewport != NULL && (drawstate.viewport->x || drawstate.viewport->y)) {
copydata->dstrect.x += drawstate.viewport->x;
copydata->dstrect.y += drawstate.viewport->y;
}
@ -901,7 +901,7 @@ SW_RunCommandQueue(SDL_Renderer * renderer, SDL_RenderCommand *cmd, void *vertic
PrepTextureForCopy(cmd);
/* Apply viewport */
if (drawstate.viewport->x || drawstate.viewport->y) {
if (drawstate.viewport != NULL && (drawstate.viewport->x || drawstate.viewport->y)) {
SDL_Point vp;
vp.x = drawstate.viewport->x;
vp.y = drawstate.viewport->y;
@ -924,7 +924,7 @@ SW_RunCommandQueue(SDL_Renderer * renderer, SDL_RenderCommand *cmd, void *vertic
GeometryFillData *ptr = (GeometryFillData *) verts;
/* Apply viewport */
if (drawstate.viewport->x || drawstate.viewport->y) {
if (drawstate.viewport != NULL && (drawstate.viewport->x || drawstate.viewport->y)) {
SDL_Point vp;
vp.x = drawstate.viewport->x;
vp.y = drawstate.viewport->y;

View File

@ -2580,7 +2580,7 @@ init_mparams(void)
#else /* (FOOTERS && !INSECURE) */
s = (size_t) 0x58585858U;
#endif /* (FOOTERS && !INSECURE) */
ACQUIRE_MAGIC_INIT_LOCK();
(void)ACQUIRE_MAGIC_INIT_LOCK();
if (mparams.magic == 0) {
mparams.magic = s;
/* Set up lock for main malloc area */

View File

@ -436,6 +436,7 @@ SDL_MasksToPixelFormatEnum(int bpp, Uint32 Rmask, Uint32 Gmask, Uint32 Bmask,
return SDL_PIXELFORMAT_RGB24;
#endif
}
break;
case 32:
if (Rmask == 0) {
return SDL_PIXELFORMAT_RGB888;

View File

@ -400,9 +400,11 @@ SDL_INTERSECTRECTANDLINE(const RECTTYPE * rect, SCALARTYPE *X1, SCALARTYPE *Y1,
outcode1 = COMPUTEOUTCODE(rect, x, y);
} else {
if (outcode2 & CODE_TOP) {
SDL_assert(y2 != y1); /* if equal: division by zero. */
y = recty1;
x = x1 + ((x2 - x1) * (y - y1)) / (y2 - y1);
} else if (outcode2 & CODE_BOTTOM) {
SDL_assert(y2 != y1); /* if equal: division by zero. */
y = recty2;
x = x1 + ((x2 - x1) * (y - y1)) / (y2 - y1);
} else if (outcode2 & CODE_LEFT) {

View File

@ -4337,8 +4337,8 @@ SDL_StartTextInput(void)
SDL_Window *window;
/* First, enable text events */
SDL_EventState(SDL_TEXTINPUT, SDL_ENABLE);
SDL_EventState(SDL_TEXTEDITING, SDL_ENABLE);
(void)SDL_EventState(SDL_TEXTINPUT, SDL_ENABLE);
(void)SDL_EventState(SDL_TEXTEDITING, SDL_ENABLE);
/* Then show the on-screen keyboard, if any */
window = SDL_GetFocusWindow();
@ -4393,8 +4393,8 @@ SDL_StopTextInput(void)
}
/* Finally disable text events */
SDL_EventState(SDL_TEXTINPUT, SDL_DISABLE);
SDL_EventState(SDL_TEXTEDITING, SDL_DISABLE);
(void)SDL_EventState(SDL_TEXTINPUT, SDL_DISABLE);
(void)SDL_EventState(SDL_TEXTEDITING, SDL_DISABLE);
}
void

View File

@ -601,9 +601,10 @@ SDL_ConvertPixels_ARGB8888_to_YUV(int width, int height, const void *src, int sr
Uint8 *plane_interleaved_uv;
Uint32 y_stride, uv_stride, y_skip, uv_skip;
GetYUVPlanes(width, height, dst_format, dst, dst_pitch,
(const Uint8 **)&plane_y, (const Uint8 **)&plane_u, (const Uint8 **)&plane_v,
&y_stride, &uv_stride);
if (GetYUVPlanes(width, height, dst_format, dst, dst_pitch,
(const Uint8 **)&plane_y, (const Uint8 **)&plane_u, (const Uint8 **)&plane_v,
&y_stride, &uv_stride) != 0)
return -1;
plane_interleaved_uv = (plane_y + height * y_stride);
y_skip = (y_stride - width);

View File

@ -389,13 +389,21 @@ WIN_ShouldShowNativeUI()
static void
IME_Init(SDL_VideoData *videodata, HWND hwnd)
{
HRESULT hResult = S_OK;
if (videodata->ime_initialized)
return;
videodata->ime_hwnd_main = hwnd;
if (SUCCEEDED(WIN_CoInitialize())) {
videodata->ime_com_initialized = SDL_TRUE;
CoCreateInstance(&CLSID_TF_ThreadMgr, NULL, CLSCTX_INPROC_SERVER, &IID_ITfThreadMgr, (LPVOID *)&videodata->ime_threadmgr);
hResult = CoCreateInstance(&CLSID_TF_ThreadMgr, NULL, CLSCTX_INPROC_SERVER, &IID_ITfThreadMgr, (LPVOID *)&videodata->ime_threadmgr);
if (hResult != S_OK)
{
videodata->ime_available = SDL_FALSE;
SDL_SetError("CoCreateInstance() failed, HRESULT is %08X", (unsigned int)hResult);
return;
}
}
videodata->ime_initialized = SDL_TRUE;
videodata->ime_himm32 = SDL_LoadObject("imm32.dll");

View File

@ -739,15 +739,18 @@ WIN_GetWindowBordersSize(_THIS, SDL_Window * window, int *top, int *left, int *b
/* rcClient stores the size of the inner window, while rcWindow stores the outer size relative to the top-left
* screen position; so the top/left values of rcClient are always {0,0} and bottom/right are {height,width} */
GetClientRect(hwnd, &rcClient);
GetWindowRect(hwnd, &rcWindow);
if (!GetClientRect(hwnd, &rcClient))
SDL_SetError("GetClientRect() failed, error %08X", (unsigned int)GetLastError());
if (!GetWindowRect(hwnd, &rcWindow))
SDL_SetError("GetWindowRect() failed, error %08X", (unsigned int)GetLastError());
/* convert the top/left values to make them relative to
* the window; they will end up being slightly negative */
ptDiff.y = rcWindow.top;
ptDiff.x = rcWindow.left;
ScreenToClient(hwnd, &ptDiff);
if (!ScreenToClient(hwnd, &ptDiff))
SDL_SetError("ScreenToClient() failed, error %08X", (unsigned int)GetLastError());
rcWindow.top = ptDiff.y;
rcWindow.left = ptDiff.x;
@ -757,7 +760,8 @@ WIN_GetWindowBordersSize(_THIS, SDL_Window * window, int *top, int *left, int *b
ptDiff.y = rcWindow.bottom;
ptDiff.x = rcWindow.right;
ScreenToClient(hwnd, &ptDiff);
if (!ScreenToClient(hwnd, &ptDiff))
SDL_SetError("ScreenToClient() failed, error %08X", (unsigned int)GetLastError());
rcWindow.bottom = ptDiff.y;
rcWindow.right = ptDiff.x;