From 48a387938dcc4aea7e3e2ece4494fc68bc8fa4e5 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Mon, 11 Nov 2024 16:26:50 +0100 Subject: [PATCH 1/6] [winpr,env] fix use of getcwd do not rely on GNU_SOURCE extensions --- winpr/libwinpr/environment/environment.c | 28 +++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/winpr/libwinpr/environment/environment.c b/winpr/libwinpr/environment/environment.c index 600b15f20..84743e061 100644 --- a/winpr/libwinpr/environment/environment.c +++ b/winpr/libwinpr/environment/environment.c @@ -24,12 +24,15 @@ #include #include #include +#include #include #include #ifndef _WIN32 +#include + #ifdef WINPR_HAVE_UNISTD_H #include #endif @@ -43,20 +46,35 @@ DWORD GetCurrentDirectoryA(DWORD nBufferLength, LPSTR lpBuffer) { - char* cwd = NULL; size_t length = 0; + char* cwd = NULL; + char* ccwd = NULL; - cwd = getcwd(NULL, 0); + do + { + length += MAX_PATH; + char* tmp = realloc(cwd, length); + if (!tmp) + { + free(cwd); + return 0; + } + cwd = tmp; - if (!cwd) + ccwd = getcwd(cwd, length); + } while (!ccwd && (errno == ERANGE)); + + if (!ccwd) + { + free(cwd); return 0; + } - length = strlen(cwd); + length = strnlen(cwd, length); if ((nBufferLength == 0) && (lpBuffer == NULL)) { free(cwd); - return (DWORD)length; } else From 7f27e168b35b9e52800c5f4a24b95c9ba162cdb2 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Mon, 11 Nov 2024 16:20:13 +0100 Subject: [PATCH 2/6] [emu,scard] flag allocator with nolint The allocator keeps an internal list of allocated contexts. No manual free required. --- libfreerdp/emu/scard/smartcard_virtual_gids.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libfreerdp/emu/scard/smartcard_virtual_gids.c b/libfreerdp/emu/scard/smartcard_virtual_gids.c index fb3f72e32..d9158a9ca 100644 --- a/libfreerdp/emu/scard/smartcard_virtual_gids.c +++ b/libfreerdp/emu/scard/smartcard_virtual_gids.c @@ -1503,11 +1503,13 @@ BOOL vgids_init(vgidsContext* ctx, const char* cert, const char* privateKey, con goto init_failed; /* create masterfile */ + // NOLINTNEXTLINE(clang-analyzer-unix.Malloc) masterEF = vgids_ef_new(ctx, VGIDS_EFID_MASTER); if (!masterEF) goto init_failed; /* create cardid file with cardid DO */ + // NOLINTNEXTLINE(clang-analyzer-unix.Malloc) cardidEF = vgids_ef_new(ctx, VGIDS_EFID_CARDID); if (!cardidEF) goto init_failed; @@ -1516,6 +1518,7 @@ BOOL vgids_init(vgidsContext* ctx, const char* cert, const char* privateKey, con goto init_failed; /* create user common file */ + // NOLINTNEXTLINE(clang-analyzer-unix.Malloc) commonEF = vgids_ef_new(ctx, VGIDS_EFID_COMMON); if (!commonEF) goto init_failed; From 08839a11de1d2b05d7fa59fe5c31e79a029be681 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Mon, 11 Nov 2024 17:03:58 +0100 Subject: [PATCH 3/6] [utils,passphrase] NULL checks --- libfreerdp/utils/passphrase.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/libfreerdp/utils/passphrase.c b/libfreerdp/utils/passphrase.c index e73ad4e2c..5bc08a0a5 100644 --- a/libfreerdp/utils/passphrase.c +++ b/libfreerdp/utils/passphrase.c @@ -132,7 +132,6 @@ static const char* freerdp_passphrase_read_tty(rdpContext* context, const char* { BOOL terminal_needs_reset = FALSE; char term_name[L_ctermid] = { 0 }; - int term_file = 0; FILE* fout = NULL; @@ -144,15 +143,29 @@ static const char* freerdp_passphrase_read_tty(rdpContext* context, const char* ctermid(term_name); int terminal_fildes = 0; - if (from_stdin || strcmp(term_name, "") == 0 || (term_file = open(term_name, O_RDWR)) == -1) + if (from_stdin || (strcmp(term_name, "") == 0)) { fout = stdout; terminal_fildes = STDIN_FILENO; } else { - fout = fdopen(term_file, "w"); - terminal_fildes = term_file; + const int term_file = open(term_name, O_RDWR); + if (term_file < 0) + { + fout = stdout; + terminal_fildes = STDIN_FILENO; + } + else + { + fout = fdopen(term_file, "w"); + if (!fout) + { + close(term_file); + return NULL; + } + terminal_fildes = term_file; + } } struct termios orig_flags = { 0 }; From 7797d8d6693e3fb2737ccdc3db174078578dda80 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Mon, 11 Nov 2024 17:09:06 +0100 Subject: [PATCH 4/6] [ci,tidy] disable readability-avoid-nested-conditional-operator the check is flagging every (cond) ? foo : bar as too complicated, useless check. --- .clang-tidy | 1 + 1 file changed, 1 insertion(+) diff --git a/.clang-tidy b/.clang-tidy index ed8f011cc..d639d1b19 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -60,6 +60,7 @@ Checks: > -llvm-qualified-auto, -llvm-else-after-return, -readability-else-after-return, + -readability-avoid-nested-conditional-operator, -modernize-use-trailing-return-type, -modernize-return-braced-init-list, -modernize-macro-to-enum, From db07add07a7f3b44b62641e90ae4961594c9f1b2 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Mon, 11 Nov 2024 18:01:42 +0100 Subject: [PATCH 5/6] [deprecation] replace all usages of sprintf --- .../urbdrc/client/libusb/libusb_udevice.c | 3 +- libfreerdp/primitives/test/measure.h | 74 +++++++++---------- libfreerdp/primitives/test/prim_test.c | 17 +++-- winpr/libwinpr/crt/string.c | 2 +- winpr/libwinpr/utils/wlog/Layout.c | 20 +++-- 5 files changed, 61 insertions(+), 55 deletions(-) diff --git a/channels/urbdrc/client/libusb/libusb_udevice.c b/channels/urbdrc/client/libusb/libusb_udevice.c index 5206ad1f7..cbb5bdf12 100644 --- a/channels/urbdrc/client/libusb/libusb_udevice.c +++ b/channels/urbdrc/client/libusb/libusb_udevice.c @@ -1615,7 +1615,8 @@ static int udev_get_device_handle(URBDRC_PLUGIN* urbdrc, libusb_context* ctx, UD error = 0; WLog_Print(urbdrc->log, WLOG_DEBUG, " Port: %d", pdev->port_number); /* gen device path */ - (void)sprintf(pdev->path, "%" PRIu16 "-%d", bus_number, pdev->port_number); + (void)_snprintf(pdev->path, sizeof(pdev->path), "%" PRIu16 "-%d", bus_number, + pdev->port_number); WLog_Print(urbdrc->log, WLOG_DEBUG, " DevPath: %s", pdev->path); } diff --git a/libfreerdp/primitives/test/measure.h b/libfreerdp/primitives/test/measure.h index 2d4f36e97..e97091e83 100644 --- a/libfreerdp/primitives/test/measure.h +++ b/libfreerdp/primitives/test/measure.h @@ -71,23 +71,19 @@ #endif // GOOGLE_PROFILER extern float measure_delta_time(UINT64 t0, UINT64 t1); -extern void measure_floatprint(float t, char* output); +extern void measure_floatprint(float t, char* output, size_t len); -#define MEASURE_LOOP_START(_prefix_, _count_) \ - { \ - UINT64 _start, _stop; \ - char* _prefix; \ - int _count = (_count_); \ - int _loop; \ - float _delta; \ - char _str1[32], _str2[32]; \ - _prefix = _strdup(_prefix_); \ - _str1[0] = '\0'; \ - _str2[0] = '\0'; \ - _start = winpr_GetTickCount64NS(); \ - PROFILER_START(_prefix); \ - _loop = (_count); \ - do \ +#define MEASURE_LOOP_START(_prefix_, _count_) \ + { \ + int _count = (_count_); \ + int _loop; \ + char str1[32] = { 0 }; \ + char str2[32] = { 0 }; \ + char* _prefix = _strdup(_prefix_); \ + const UINT64 start = winpr_GetTickCount64NS(); \ + PROFILER_START(_prefix); \ + _loop = (_count); \ + do \ { #define MEASURE_LOOP_STOP \ @@ -95,33 +91,33 @@ extern void measure_floatprint(float t, char* output); while (--_loop) \ ; -#define MEASURE_GET_RESULTS(_result_) \ - PROFILER_STOP; \ - _stop = winpr_GetTickCount64NS(); \ - _delta = measure_delta_time(_start, _stop); \ - (_result_) = (float)_count / _delta; \ - free(_prefix); \ +#define MEASURE_GET_RESULTS(_result_) \ + PROFILER_STOP; \ + const UINT64 stop = winpr_GetTickCount64NS(); \ + const float delta = measure_delta_time(start, stop); \ + (_result_) = (float)_count / delta; \ + free(_prefix); \ } -#define MEASURE_SHOW_RESULTS(_result_) \ - PROFILER_STOP; \ - _stop = winpr_GetTickCount64NS(); \ - _delta = measure_delta_time(_start, _stop); \ - (_result_) = (float)_count / _delta; \ - measure_floatprint((float)_count / _delta, _str1); \ - printf("%s: %9d iterations in %5.1f seconds = %s/s \n", _prefix, _count, _delta, _str1); \ - free(_prefix); \ +#define MEASURE_SHOW_RESULTS(_result_) \ + PROFILER_STOP; \ + const UINT64 stop = winpr_GetTickCount64NS(); \ + const float delta = measure_delta_time(start, stop); \ + (_result_) = (float)_count / delta; \ + measure_floatprint((float)_count / delta, str1); \ + printf("%s: %9d iterations in %5.1f seconds = %s/s \n", _prefix, _count, delta, str1); \ + free(_prefix); \ } -#define MEASURE_SHOW_RESULTS_SCALED(_scale_, _label_) \ - PROFILER_STOP; \ - _stop = winpr_GetTickCount64NS(); \ - _delta = measure_delta_time(_start, _stop); \ - measure_floatprint((float)_count / _delta, _str1); \ - measure_floatprint((float)_count / _delta * (_scale_), _str2); \ - printf("%s: %9d iterations in %5.1f seconds = %s/s = %s%s \n", _prefix, _count, _delta, _str1, \ - _str2, _label_); \ - free(_prefix); \ +#define MEASURE_SHOW_RESULTS_SCALED(_scale_, _label_) \ + PROFILER_STOP; \ + const UINT64 stop = winpr_GetTickCount64NS(); \ + const float delta = measure_delta_time(start, stop); \ + measure_floatprint((float)_count / delta, str1); \ + measure_floatprint((float)_count / delta * (_scale_), str2); \ + printf("%s: %9d iterations in %5.1f seconds = %s/s = %s%s \n", _prefix, _count, delta, str1, \ + str2, _label_); \ + free(_prefix); \ } #define MEASURE_TIMED(_label_, _init_iter_, _test_time_, _result_, _call_) \ diff --git a/libfreerdp/primitives/test/prim_test.c b/libfreerdp/primitives/test/prim_test.c index 5aafecb86..ffb7c6b5a 100644 --- a/libfreerdp/primitives/test/prim_test.c +++ b/libfreerdp/primitives/test/prim_test.c @@ -44,7 +44,7 @@ float measure_delta_time(UINT64 t0, UINT64 t1) } /* ------------------------------------------------------------------------- */ -void measure_floatprint(float t, char* output) +void measure_floatprint(float t, char* output, size_t len) { /* I don't want to link against -lm, so avoid log,exp,... */ float f = 10.0; @@ -57,19 +57,20 @@ void measure_floatprint(float t, char* output) i = ((int)(t / f + 0.5f)) * (int)f; if (t < 0.0f) - (void)sprintf(output, "%f", t); + (void)_snprintf(output, len, "%f", t); else if (i == 0) - (void)sprintf(output, "%d", (int)(t + 0.5f)); + (void)_snprintf(output, len, "%d", (int)(t + 0.5f)); else if (t < 1e+3f) - (void)sprintf(output, "%3d", i); + (void)_snprintf(output, len, "%3d", i); else if (t < 1e+6f) - (void)sprintf(output, "%3d,%03d", i / 1000, i % 1000); + (void)_snprintf(output, len, "%3d,%03d", i / 1000, i % 1000); else if (t < 1e+9f) - (void)sprintf(output, "%3d,%03d,000", i / 1000000, (i % 1000000) / 1000); + (void)_snprintf(output, len, "%3d,%03d,000", i / 1000000, (i % 1000000) / 1000); else if (t < 1e+12f) - (void)sprintf(output, "%3d,%03d,000,000", i / 1000000000, (i % 1000000000) / 1000000); + (void)_snprintf(output, len, "%3d,%03d,000,000", i / 1000000000, + (i % 1000000000) / 1000000); else - (void)sprintf(output, "%f", t); + (void)_snprintf(output, len, "%f", t); } void prim_test_setup(BOOL performance) diff --git a/winpr/libwinpr/crt/string.c b/winpr/libwinpr/crt/string.c index a41424201..be8d8608f 100644 --- a/winpr/libwinpr/crt/string.c +++ b/winpr/libwinpr/crt/string.c @@ -204,7 +204,7 @@ int winpr_vasprintf(char** s, size_t* slen, WINPR_FORMAT_ARG const char* templ, return -1; va_copy(ap, oap); - const int plen = vsprintf(str, templ, ap); + const int plen = vsnprintf(str, (size_t)length, templ, ap); va_end(ap); if (length != plen) diff --git a/winpr/libwinpr/utils/wlog/Layout.c b/winpr/libwinpr/utils/wlog/Layout.c index 86d942371..f94d8a453 100644 --- a/winpr/libwinpr/utils/wlog/Layout.c +++ b/winpr/libwinpr/utils/wlog/Layout.c @@ -44,6 +44,11 @@ struct format_option_recurse; +struct format_tid_arg +{ + char tid[32]; +}; + struct format_option { const char* fmt; @@ -90,7 +95,9 @@ static void WLog_PrintMessagePrefix(wLog* log, wLogMessage* message, static const char* get_tid(void* arg) { - char* str = arg; + struct format_tid_arg* targ = arg; + WINPR_ASSERT(targ); + size_t tid = 0; #if defined __linux__ && !defined ANDROID /* On Linux we prefer to see the LWP id */ @@ -98,8 +105,8 @@ static const char* get_tid(void* arg) #else tid = (size_t)GetCurrentThreadId(); #endif - (void)sprintf(str, "%08" PRIxz, tid); - return str; + (void)_snprintf(targ->tid, sizeof(targ->tid), "%08" PRIxz, tid); + return targ->tid; } static BOOL log_invalid_fmt(const char* what) @@ -230,7 +237,8 @@ BOOL WLog_Layout_GetMessagePrefix(wLog* log, wLogLayout* layout, wLogMessage* me WINPR_ASSERT(layout); WINPR_ASSERT(message); - char tid[32] = { 0 }; + struct format_tid_arg targ = { 0 }; + SYSTEMTIME localTime = { 0 }; GetLocalTime(&localTime); @@ -266,8 +274,8 @@ BOOL WLog_Layout_GetMessagePrefix(wLog* log, wLogLayout* layout, wLogMessage* me { ENTRY("%pid"), ENTRY("%u"), NULL, (void*)(size_t)GetCurrentProcessId(), NULL, &recurse }, /* process id */ { ENTRY("%se"), ENTRY("%02u"), NULL, (void*)(size_t)localTime.wSecond, NULL, - &recurse }, /* seconds */ - { ENTRY("%tid"), ENTRY("%s"), get_tid, tid, NULL, &recurse }, /* thread id */ + &recurse }, /* seconds */ + { ENTRY("%tid"), ENTRY("%s"), get_tid, &targ, NULL, &recurse }, /* thread id */ { ENTRY("%yr"), ENTRY("%u"), NULL, (void*)(size_t)localTime.wYear, NULL, &recurse }, /* year */ { ENTRY("%{"), ENTRY("%}"), NULL, log->context, skip_if_null, From 9e15af885085c87ebd563e9ffcaba4713db12c8b Mon Sep 17 00:00:00 2001 From: akallabeth Date: Tue, 12 Nov 2024 08:48:09 +0100 Subject: [PATCH 6/6] [winpr,assert] fix coverity pragma rewrite pragma according to https://community.blackduck.com/s/article/Suppressing-False-Positive-Intentional-defects --- winpr/include/winpr/assert.h | 62 ++++++++++++++++++++---------------- 1 file changed, 34 insertions(+), 28 deletions(-) diff --git a/winpr/include/winpr/assert.h b/winpr/include/winpr/assert.h index cae3420d8..b9257cd0b 100644 --- a/winpr/include/winpr/assert.h +++ b/winpr/include/winpr/assert.h @@ -34,21 +34,24 @@ extern "C" { #endif -#define WINPR_ASSERT(cond) \ - do \ - { \ - WINPR_PRAGMA_DIAG_PUSH \ - WINPR_PRAGMA_DIAG_TAUTOLOGICAL_CONSTANT_OUT_OF_RANGE_COMPARE \ - WINPR_PRAGMA_DIAG_TAUTOLOGICAL_VALUE_RANGE_COMPARE \ - WINPR_PRAGMA_DIAG_IGNORED_UNKNOWN_PRAGMAS \ - WINPR_DO_PRAGMA(coverity compliance deviate "NO_EFFECT:SUPPRESS" \ - "WINPR_ASSERT") \ - WINPR_DO_PRAGMA(coverity compliance deviate "CONSTANT_EXPRESSION_RESULT:SUPPRESS" \ - "WINPR_ASSERT") \ - \ - if (!(cond)) \ - winpr_int_assert(#cond, __FILE__, __func__, __LINE__); \ - WINPR_PRAGMA_DIAG_POP \ +#define WINPR_ASSERT(cond) \ + do \ + { \ + WINPR_PRAGMA_DIAG_PUSH \ + WINPR_PRAGMA_DIAG_TAUTOLOGICAL_CONSTANT_OUT_OF_RANGE_COMPARE \ + WINPR_PRAGMA_DIAG_TAUTOLOGICAL_VALUE_RANGE_COMPARE \ + WINPR_PRAGMA_DIAG_IGNORED_UNKNOWN_PRAGMAS \ + WINPR_DO_PRAGMA(coverity compliance block \x28 deviate "CONSTANT_EXPRESSION_RESULT" \ + "WINPR_ASSERT" \x29 \ + \x28 deviate "NO_EFFECT" \ + "WINPR_ASSERT" \x29) \ + \ + if (!(cond)) \ + winpr_int_assert(#cond, __FILE__, __func__, __LINE__); \ + \ + WINPR_DO_PRAGMA(coverity compliance end_block "CONSTANT_EXPRESSION_RESULT" \ + "NO_EFFECT") \ + WINPR_PRAGMA_DIAG_POP \ } while (0) static INLINE WINPR_NORETURN(void winpr_int_assert(const char* condstr, const char* file, @@ -65,19 +68,22 @@ extern "C" #endif #else -#define WINPR_ASSERT(cond) \ - do \ - { \ - WINPR_PRAGMA_DIAG_PUSH \ - WINPR_PRAGMA_DIAG_TAUTOLOGICAL_CONSTANT_OUT_OF_RANGE_COMPARE \ - WINPR_PRAGMA_DIAG_TAUTOLOGICAL_VALUE_RANGE_COMPARE \ - WINPR_PRAGMA_DIAG_IGNORED_UNKNOWN_PRAGMAS \ - WINPR_DO_PRAGMA(coverity compliance deviate "NO_EFFECT:SUPPRESS" \ - "WINPR_ASSERT") \ - WINPR_DO_PRAGMA(coverity compliance deviate "CONSTANT_EXPRESSION_RESULT:SUPPRESS" \ - "WINPR_ASSERT") \ - assert(cond); \ - WINPR_PRAGMA_DIAG_POP \ +#define WINPR_ASSERT(cond) \ + do \ + { \ + WINPR_PRAGMA_DIAG_PUSH \ + WINPR_PRAGMA_DIAG_TAUTOLOGICAL_CONSTANT_OUT_OF_RANGE_COMPARE \ + WINPR_PRAGMA_DIAG_TAUTOLOGICAL_VALUE_RANGE_COMPARE \ + WINPR_PRAGMA_DIAG_IGNORED_UNKNOWN_PRAGMAS \ + WINPR_DO_PRAGMA(coverity compliance block \x28 deviate "CONSTANT_EXPRESSION_RESULT" \ + "WINPR_ASSERT" \x29 \ + \x28 deviate "NO_EFFECT" \ + "WINPR_ASSERT" \x29) \ + assert(cond); \ + \ + WINPR_DO_PRAGMA(coverity compliance end_block "CONSTANT_EXPRESSION_RESULT" \ + "NO_EFFECT") \ + WINPR_PRAGMA_DIAG_POP \ } while (0) #endif