[PATCH 1/4] oledb32/tests: modify size of buffers in order to handle GCC 11.2 warning (GCC11)
in oledb32/tests/convert.c, gcc 11.2 complains (-Warray-bounds) about accessing fields from a pointer to VARIANT, while underlying storage buffer is strictly smaller than a VARIANT gcc doesn't check that accessed field is inside or not the underlying storage size of 20 used in most of the tests is not necessarly wrong wrt the fields adressed, even if I don't fully grasp the logic of a 20 byte buffer: - VARIANT is 16 bytes on 32bit systems, and 24 bytes on 64 bit systems - and all the other structures are 16 bytes at most anyway, make it always of the sizeof of a VARIANT, in order to keep gcc happy Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- dlls/oledb32/tests/convert.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/dlls/oledb32/tests/convert.c b/dlls/oledb32/tests/convert.c index 8ec64d40ae5..1f22851a2b3 100644 --- a/dlls/oledb32/tests/convert.c +++ b/dlls/oledb32/tests/convert.c @@ -327,7 +327,7 @@ static void test_converttoi1(void) { HRESULT hr; signed char dst; - BYTE src[20]; + BYTE src[sizeof(VARIANT)]; /* assuming that VARIANT is larger than all the types used in src */ DBSTATUS dst_status; DBLENGTH dst_len; static const WCHAR ten[] = {'1','0',0}; @@ -638,7 +638,7 @@ static void test_converttoi2(void) { HRESULT hr; signed short dst; - BYTE src[20]; + BYTE src[sizeof(VARIANT)]; /* assuming that VARIANT is larger than all the types used in src */ DBSTATUS dst_status; DBLENGTH dst_len; static const WCHAR ten[] = {'1','0',0}; @@ -950,7 +950,7 @@ static void test_converttoi4(void) { HRESULT hr; INT i4; - BYTE src[20]; + BYTE src[sizeof(VARIANT)]; /* assuming that VARIANT is larger than all the types used in src */ DBSTATUS dst_status; DBLENGTH dst_len; static const WCHAR ten[] = {'1','0',0}; @@ -1224,7 +1224,7 @@ static void test_converttoi8(void) { HRESULT hr; LARGE_INTEGER dst; - BYTE src[20]; + BYTE src[sizeof(VARIANT)]; /* assuming that VARIANT is larger than all the types used in src */ DBSTATUS dst_status; DBLENGTH dst_len; static const WCHAR ten[] = {'1','0',0}; @@ -2774,7 +2774,7 @@ static void test_converttoui4(void) { HRESULT hr; DWORD dst; - BYTE src[20]; + BYTE src[sizeof(VARIANT)]; DBSTATUS dst_status; DBLENGTH dst_len;
- otherwise, it generates a warning in destroy_region() Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- dlls/gdi32/region.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/dlls/gdi32/region.c b/dlls/gdi32/region.c index 8555be96f72..d769ea5ad98 100644 --- a/dlls/gdi32/region.c +++ b/dlls/gdi32/region.c @@ -412,7 +412,11 @@ static BOOL init_region( WINEREGION *pReg, INT n ) if (n > RGN_DEFAULT_RECTS) { - if (n > INT_MAX / sizeof(RECT)) return FALSE; + if (n > INT_MAX / sizeof(RECT)) + { + pReg->rects = NULL; + return FALSE; + } if (!(pReg->rects = malloc( n * sizeof( RECT ) ))) return FALSE; }
v2: fix GetModuleHandleExA rather than GetMoculdHandleA Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- dlls/kernelbase/loader.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/dlls/kernelbase/loader.c b/dlls/kernelbase/loader.c index 145d721bc26..4471d83dd24 100644 --- a/dlls/kernelbase/loader.c +++ b/dlls/kernelbase/loader.c @@ -355,7 +355,11 @@ BOOL WINAPI DECLSPEC_HOTPATCH GetModuleHandleExA( DWORD flags, LPCSTR name, HMOD if (!name || (flags & GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS)) return GetModuleHandleExW( flags, (LPCWSTR)name, module ); - if (!(nameW = file_name_AtoW( name, FALSE ))) return FALSE; + if (!(nameW = file_name_AtoW( name, FALSE ))) + { + if (module) *module = NULL; + return FALSE; + } return GetModuleHandleExW( flags, nameW, module ); }
(-Wmisleading indentation) This is the first patch of a long serie (more than 100 files to patch) as a remainder, gcc 11.2 with -Wall (implying -Wmisleading indentation) complains when we have: bla1; todo_wine ok(..., "..."); bla2; It complains on bla2; not beying indented as the todo_wine. To make gcc happy, indent the todo_wine as the rest. bla1; todo_wine ok(..., "..."); bla2; If this patch is accepted, I volunteer to keep pushing the remaining changes. With the following rules: - one patch per file - max ten patches per day - if one patch conflicts with another patch, other patch has priority as it should bring more value than pure space injection. Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- dlls/adsldp/tests/ldap.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/dlls/adsldp/tests/ldap.c b/dlls/adsldp/tests/ldap.c index fe9801a3972..161fc5d0bbc 100644 --- a/dlls/adsldp/tests/ldap.c +++ b/dlls/adsldp/tests/ldap.c @@ -190,7 +190,7 @@ static void test_ParseDisplayName(void) count = 0xdeadbeef; hr = MkParseDisplayName(bc, test[i].path, &count, &mk); -todo_wine_if(i == 0 || i == 1 || i == 11 || i == 12) + todo_wine_if(i == 0 || i == 1 || i == 11 || i == 12) ok(hr == test[i].hr, "%d: got %#x, expected %#x\n", i, hr, test[i].hr); if (hr == S_OK) { @@ -394,7 +394,7 @@ static void test_DirectorySearch(void) ok(hr == E_ADS_BAD_PARAMETER, "got %#x\n", hr); hr = IDirectorySearch_GetPreviousRow(ds, sh); -todo_wine + todo_wine ok(hr == E_ADS_BAD_PARAMETER, "got %#x\n", hr); while (IDirectorySearch_GetNextRow(ds, sh) != S_ADS_NOMORE_ROWS) @@ -430,9 +430,9 @@ todo_wine name = NULL; hr = IDirectorySearch_GetNextColumnName(ds, sh, &name); -todo_wine + todo_wine ok(hr == S_OK || broken(hr == S_ADS_NOMORE_COLUMNS) /* XP */, "got %#x\n", hr); -todo_wine + todo_wine ok((name && !wcscmp(name, L"ADsPath")) || broken(!name) /* XP */, "got %s\n", wine_dbgstr_w(name)); FreeADsMem(name); @@ -488,7 +488,7 @@ static void test_DirectoryObject(void) ok(hr == S_OK, "got %#x\n", hr); hr = IDirectoryObject_QueryInterface(dirobj, &IID_IADsOpenDSObject, (void **)&unk); -todo_wine + todo_wine ok(hr == E_NOINTERFACE, "got %#x\n", hr); if (hr == S_OK) IUnknown_Release(unk); hr = IDirectoryObject_QueryInterface(dirobj, &IID_IDispatch, (void **)&unk);
Hi Eric Hats off for taking this mundane, yet important endeavour. On Sat, 2 Oct 2021 at 13:06, Eric Pouech <eric.pouech(a)gmail.com> wrote:
If this patch is accepted, I volunteer to keep pushing the remaining changes. With the following rules: - one patch per file
Perhaps make that one patch per dll? Some dlls have 10+ C files in their tests/ Just a small comment from an outsider. HTH -Emil
On 10/3/21 3:21 AM, Emil Velikov wrote:
Hi Eric
Hats off for taking this mundane, yet important endeavour.
On Sat, 2 Oct 2021 at 13:06, Eric Pouech <eric.pouech(a)gmail.com> wrote:
If this patch is accepted, I volunteer to keep pushing the remaining changes. With the following rules: - one patch per file
Perhaps make that one patch per dll? Some dlls have 10+ C files in their tests/ Just a small comment from an outsider.
HTH -Emil
FWIW Alexandre sometimes edits the titles when committing the patches, but if you intend so send a lot of patches I'm sure it'll be appreciated to make them match the usual pattern already. This means a "<module>: " prefix, or "<module>/tests: " if test files only are changed (with some possible variations in <module> to indicate some area of the module, usually done like "<module>/<area>: "), followed by the title sentence with an upper case first letter, and a final dot / period. Then, although it probably matters a bit less, the commit messages should also probably use capitalized sentences. You can also add the v2: / v3:... / other contextual patch comments below the git send-email "---" marker (and above the first "diff"), so they won't be included in the commit message. Cheers, -- Rémi Bernon <rbernon(a)codeweavers.com>
Le 03/10/2021 à 03:21, Emil Velikov a écrit :
Hi Eric
Hats off for taking this mundane, yet important endeavour.
On Sat, 2 Oct 2021 at 13:06, Eric Pouech<eric.pouech(a)gmail.com> wrote:
If this patch is accepted, I volunteer to keep pushing the remaining changes. With the following rules: - one patch per file Perhaps make that one patch per dll? Some dlls have 10+ C files in their tests/ Just a small comment from an outsider.
HTH -Emil
I also want to minimize the number of conflicts... so a patch with 10 files is 10 ten times more risky to have a conflict and 10 files a day, for 120 files, makes 12 days, 3 weeks of effort (including some conflict resolution that will happen) so no need to hurry A+
Eric Pouech <eric.pouech(a)orange.fr> writes:
I also want to minimize the number of conflicts... so a patch with 10 files is 10 ten times more risky to have a conflict
It's not very likely to conflict, and such a conflict would be easy to solve, so it's not necessary to split such whitespace changes into tiny patches. One patch per dll is fine. -- Alexandre Julliard julliard(a)winehq.org
participants (5)
-
Alexandre Julliard -
Emil Velikov -
Eric Pouech -
Eric Pouech -
Rémi Bernon