From ba7fd06ec41673753952bc3e1e9cba466c27ba92 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Fri, 23 Aug 2024 11:57:26 +0200 Subject: [PATCH] [coverity] fix some warnings * mostly dead store and identical code branches. * some possible integer overflows --- client/common/client.c | 7 +------ libfreerdp/cache/persistent.c | 4 ++-- libfreerdp/codec/color.c | 12 ++--------- libfreerdp/codec/progressive.c | 20 +++++++++---------- libfreerdp/common/settings.c | 8 ++++---- libfreerdp/crypto/per.c | 2 ++ libfreerdp/emu/scard/smartcard_virtual_gids.c | 3 --- libfreerdp/utils/smartcard_call.c | 1 + 8 files changed, 21 insertions(+), 36 deletions(-) diff --git a/client/common/client.c b/client/common/client.c index db14e4e24..1f78f75ef 100644 --- a/client/common/client.c +++ b/client/common/client.c @@ -636,12 +636,7 @@ static DWORD client_cli_accept_certificate(freerdp* instance) if ((answer == EOF) || feof(stdin)) { - printf("\nError: Could not read answer from stdin."); - - if (fromStdin) - printf(" - Run without parameter \"--from-stdin\" to set trust."); - - printf("\n"); + printf("\nError: Could not read answer from stdin.\n"); return 0; } diff --git a/libfreerdp/cache/persistent.c b/libfreerdp/cache/persistent.c index 8499c7eca..d9f62c84a 100644 --- a/libfreerdp/cache/persistent.c +++ b/libfreerdp/cache/persistent.c @@ -142,13 +142,13 @@ static int persistent_cache_read_entry_v3(rdpPersistentCache* persistent, WINPR_ASSERT(persistent); WINPR_ASSERT(entry); - if (fread((void*)&entry3, sizeof(entry3), 1, persistent->fp) != 1) + if (fread(&entry3, sizeof(entry3), 1, persistent->fp) != 1) return -1; entry->key64 = entry3.key64; entry->width = entry3.width; entry->height = entry3.height; - entry->size = entry3.width * entry3.height * 4; + entry->size = 4ul * entry3.width * entry3.height; entry->flags = 0; if (entry->size > persistent->bmpSize) diff --git a/libfreerdp/codec/color.c b/libfreerdp/codec/color.c index b6a04a8fe..614a68cd0 100644 --- a/libfreerdp/codec/color.c +++ b/libfreerdp/codec/color.c @@ -84,8 +84,6 @@ BOOL freerdp_image_copy_from_monochrome(BYTE* WINPR_RESTRICT pDstData, UINT32 Ds UINT32 backColor, UINT32 foreColor, const gdiPalette* WINPR_RESTRICT palette) { - BOOL vFlip = 0; - UINT32 monoStep = 0; const UINT32 dstBytesPerPixel = FreeRDPGetBytesPerPixel(DstFormat); if (!pDstData || !pSrcData || !palette) @@ -94,19 +92,13 @@ BOOL freerdp_image_copy_from_monochrome(BYTE* WINPR_RESTRICT pDstData, UINT32 Ds if (nDstStep == 0) nDstStep = dstBytesPerPixel * nWidth; - vFlip = FALSE; - monoStep = (nWidth + 7) / 8; + const UINT32 monoStep = (nWidth + 7) / 8; for (UINT32 y = 0; y < nHeight; y++) { - const BYTE* monoBits = NULL; BYTE* pDstLine = &pDstData[((nYDst + y) * nDstStep)]; UINT32 monoBit = 0x80; - - if (!vFlip) - monoBits = &pSrcData[monoStep * y]; - else - monoBits = &pSrcData[monoStep * (nHeight - y - 1)]; + const BYTE* monoBits = &pSrcData[monoStep * y]; for (UINT32 x = 0; x < nWidth; x++) { diff --git a/libfreerdp/codec/progressive.c b/libfreerdp/codec/progressive.c index fff4b8998..df38f6848 100644 --- a/libfreerdp/codec/progressive.c +++ b/libfreerdp/codec/progressive.c @@ -1060,12 +1060,10 @@ fail: static INLINE INT16 progressive_rfx_srl_read(RFX_PROGRESSIVE_UPGRADE_STATE* WINPR_RESTRICT state, UINT32 numBits) { - UINT32 k = 0; - UINT32 bit = 0; - UINT32 max = 0; - UINT32 mag = 0; - UINT32 sign = 0; + WINPR_ASSERT(state); + wBitStream* bs = state->srl; + WINPR_ASSERT(bs); if (state->nz) { @@ -1073,12 +1071,12 @@ static INLINE INT16 progressive_rfx_srl_read(RFX_PROGRESSIVE_UPGRADE_STATE* WINP return 0; } - k = state->kp / 8; + const UINT32 k = state->kp / 8; if (!state->mode) { /* zero encoding */ - bit = (bs->accumulator & 0x80000000) ? 1 : 0; + const UINT32 bit = (bs->accumulator & 0x80000000) ? 1 : 0; BitStream_Shift(bs, 1); if (!bit) @@ -1117,7 +1115,7 @@ static INLINE INT16 progressive_rfx_srl_read(RFX_PROGRESSIVE_UPGRADE_STATE* WINP state->mode = 0; /* zero encoding is next */ /* unary encoding */ /* read sign bit */ - sign = (bs->accumulator & 0x80000000) ? 1 : 0; + const UINT32 sign = (bs->accumulator & 0x80000000) ? 1 : 0; BitStream_Shift(bs, 1); if (state->kp < 6) @@ -1128,12 +1126,12 @@ static INLINE INT16 progressive_rfx_srl_read(RFX_PROGRESSIVE_UPGRADE_STATE* WINP if (numBits == 1) return sign ? -1 : 1; - mag = 1; - max = (1 << numBits) - 1; + UINT32 mag = 1; + const UINT32 max = (1 << numBits) - 1; while (mag < max) { - bit = (bs->accumulator & 0x80000000) ? 1 : 0; + const UINT32 bit = (bs->accumulator & 0x80000000) ? 1 : 0; BitStream_Shift(bs, 1); if (bit) diff --git a/libfreerdp/common/settings.c b/libfreerdp/common/settings.c index 3beba43d9..eb8e8093c 100644 --- a/libfreerdp/common/settings.c +++ b/libfreerdp/common/settings.c @@ -564,14 +564,14 @@ void freerdp_device_collection_free(rdpSettings* settings) if (settings->DeviceArray) { for (UINT32 index = 0; index < settings->DeviceArraySize; index++) - freerdp_settings_set_pointer_array(settings, FreeRDP_DeviceArray, index, NULL); + (void)freerdp_settings_set_pointer_array(settings, FreeRDP_DeviceArray, index, NULL); } free(settings->DeviceArray); - freerdp_settings_set_pointer(settings, FreeRDP_DeviceArray, NULL); - freerdp_settings_set_uint32(settings, FreeRDP_DeviceArraySize, 0); - freerdp_settings_set_uint32(settings, FreeRDP_DeviceCount, 0); + (void)freerdp_settings_set_pointer(settings, FreeRDP_DeviceArray, NULL); + (void)freerdp_settings_set_uint32(settings, FreeRDP_DeviceArraySize, 0); + (void)freerdp_settings_set_uint32(settings, FreeRDP_DeviceCount, 0); } BOOL freerdp_static_channel_collection_del(rdpSettings* settings, const char* name) diff --git a/libfreerdp/crypto/per.c b/libfreerdp/crypto/per.c index ab683f9f8..55f921130 100644 --- a/libfreerdp/crypto/per.c +++ b/libfreerdp/crypto/per.c @@ -575,6 +575,8 @@ BOOL per_read_numeric_string(wStream* s, UINT16 min) BOOL per_write_numeric_string(wStream* s, const BYTE* num_str, UINT16 length, UINT16 min) { + WINPR_ASSERT(num_str || (length == 0)); + const UINT16 mlength = (length >= min) ? length - min : min; if (!per_write_length(s, mlength)) diff --git a/libfreerdp/emu/scard/smartcard_virtual_gids.c b/libfreerdp/emu/scard/smartcard_virtual_gids.c index 4f29c2575..751de34d3 100644 --- a/libfreerdp/emu/scard/smartcard_virtual_gids.c +++ b/libfreerdp/emu/scard/smartcard_virtual_gids.c @@ -1526,9 +1526,6 @@ BOOL vgids_init(vgidsContext* ctx, const char* cert, const char* privateKey, con if (size <= 0) goto init_failed; - if (size <= 0) - goto init_failed; - cmrec.wKeyExchangeKeySizeBits = (WORD)size * 8; if (!vgids_ef_write_do(commonEF, VGIDS_DO_CMAPFILE, &cmrec, sizeof(cmrec))) goto init_failed; diff --git a/libfreerdp/utils/smartcard_call.c b/libfreerdp/utils/smartcard_call.c index 93988b287..00153cb10 100644 --- a/libfreerdp/utils/smartcard_call.c +++ b/libfreerdp/utils/smartcard_call.c @@ -1285,6 +1285,7 @@ static LONG smartcard_StatusW_Call(scard_call_context* smartcard, wStream* out, } /* SCardStatusW returns number of characters, we need number of bytes */ + WINPR_ASSERT(ret.cBytes != SCARD_AUTOALLOCATE); ret.cBytes *= sizeof(WCHAR); status = smartcard_pack_status_return(out, &ret, TRUE);