-Wsign-compare: Problem

[rchildre3]

Describe the bug

Warn when a comparison between signed and unsigned values could produce an incorrect result when the signed value is converted to unsigned. In C++, this warning is also enabled by -Wall.

man 1 gcc

~/Detours/src/creatwth.cpp: In function ‘BOOL DetourProcessViaHelperDllsW(DWORD, DWORD, const CHAR**, PDETOUR_CREATE_PROCESS_ROUTINEW)’:
~/Detours/src/creatwth.cpp:1472:28: warning: comparison of integer expressions of different signedness: ‘int’ and ‘unsigned int’ [-Wsign-compare]
 1472 |     if (cchWrittenWideChar >= ARRAYSIZE(szDllName) || cchWrittenWideChar <= 0) {
      |                            ^
~/Detours/src/detours.cpp: In function ‘BOOL detour_is_region_empty(PDETOUR_REGION)’:
~/Detours/src/detours.cpp:1660:23: warning: comparison of integer expressions of different signedness: ‘int’ and ‘const ULONG’ {aka ‘const long unsigned int’} [-Wsign-compare]
 1660 |     for (int i = 0; i < DETOUR_TRAMPOLINES_PER_REGION; i++) {
      |                     ~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
~/Detours/src/detours.cpp: In function ‘BYTE detour_align_from_trampoline(PDETOUR_TRAMPOLINE, BYTE)’:
~/Detours/src/detours.cpp:1855:24: warning: comparison of integer expressions of different signedness: ‘LONG’ {aka ‘long int’} and ‘unsigned int’ [-Wsign-compare]
 1855 |     for (LONG n = 0; n < ARRAYSIZE(pTrampoline->rAlign); n++) {
      |                        ^
~/Detours/src/detours.cpp: In function ‘LONG detour_align_from_target(PDETOUR_TRAMPOLINE, LONG)’:
~/Detours/src/detours.cpp:1865:24: warning: comparison of integer expressions of different signedness: ‘LONG’ {aka ‘long int’} and ‘unsigned int’ [-Wsign-compare]
 1865 |     for (LONG n = 0; n < ARRAYSIZE(pTrampoline->rAlign); n++) {
      |                        ^
~/Detours/src/detours.cpp: In function ‘LONG DetourDetach(void**, PVOID)’:
~/Detours/src/detours.cpp:2697:35: warning: comparison of integer expressions of different signedness: ‘LONG’ {aka ‘long int’} and ‘unsigned int’ [-Wsign-compare]
 2697 |     if (cbTarget == 0 || cbTarget > sizeof(pTrampoline->rbCode)) {
      |                          ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Expected behavior Compiles without warnings

Detours version https://github.com/microsoft/Detours/commit/827b89608ce66892963d1467d15a86f0b2832a78

[rchildre3]

@bgianfo I have a patch ready for these, but I am unsure about the intent of one section of code: https://github.com/microsoft/Detours/blob/6782fe6e6ab11ae34ae66182aa5a73b5fdbcd839/src/detours.cpp#L2426-L2428 The (unsigned byte) size of original target code is assigned to a signed size container?