June 2018 - Wine-Devel - list.winehq.org

Re: [PATCH] iphlpapi: Cast caddr_t to char* before doing pointer arithmetic

by Simon Richter

Hi, On Thu, Jun 28, 2018 at 01:54:45PM +0200, Alex Henrie wrote: > - while (ifPtr && ifPtr < ifc->ifc_buf + ifc->ifc_len) { > + while (ifPtr && (char *)ifPtr < ifc->ifc_buf + ifc->ifc_len) { Quick question: what if ifc->ifc_len == 1 ? In this case, can I read the length field of the record, or would that bring me out of bounds here? Simon

June 28, 2018

Re: Bug #45385 related to keyboard and probably wineserver - where to start the search

by Kai Krakow

2018-06-28 12:39 GMT+02:00 Matteo Bruni <matteo.mystral(a)gmail.com>: > 2018-06-28 9:05 GMT+02:00 Kai Krakow <kai(a)kaishome.de>: >> Hello again! >> >> 2018-06-28 8:56 GMT+02:00 Kai Krakow <kai(a)kaishome.de>: >>> Hello! >>> >>> 2018-06-27 21:14 GMT+02:00 Alex Henrie <alexhenrie24(a)gmail.com>: >>>> On Wed, Jun 27, 2018 at 11:24 AM John Found <johnfound(a)asm32.info> wrote: >>>>> >>>>> While reading the code in wineX11.drv/keyboard.c I noticed the following fragment from the function X11DRV_KeymapNotify: >>>>> >>>>> for (vkey = VK_LSHIFT; vkey <= VK_RMENU; vkey++) >>>>> { >>>>> int m = vkey - VK_LSHIFT; >>>>> >>> if (modifiers[m].vkey && !(keystate[vkey] & 0x80) != !modifiers[m].pressed) >>>>> { >>>>> TRACE( "Adjusting state for vkey %#.2x. State before %#.2x\n", >>>>> modifiers[m].vkey, keystate[vkey]); >>>>> >>>>> update_key_state( keystate, vkey, modifiers[m].pressed ); >>>>> changed = TRUE; >>>>> } >>>>> } >>>>> >>>>> Can someone explain what the condition pointed by >>> is doing? (Unfortunately, my C skills are pretty low...) >>>>> >>>>> Why not simply "(modifiers[m].vkey && (keystate[vkey] & 0x80) != modifiers[m].pressed)"? >>>> >>>> I don't immediately know what this if statement is for, but I can tell >>>> you that the ! operator converts a value of zero to 1 and a nonzero >>>> value to 0. So, if keystate[vkey] & 0x80 is 0 then >>>> modifiers[m].pressed must be nonzero and vice versa. >>> >>> I always was under the impression that the bang operator does a binary >>> inversion of the value. Thus, it would convert 0x7F to 0x80. This also >>> means it would convert 0x00 (signed) to 0xFF (signed) which turns 0 >>> into -1, not 1. >>> >>> But however it works, this code is not working in any obvious way and >>> should maybe be rewritten. >>> >>> Just my two cents. >> >> I just looked it up, the bang operator seems to be a logical (not >> binary) operator in C99: >> https://stackoverflow.com/questions/3661751/what-is-0-in-c >> >> According to this, I question how the code above is supposed to work >> in the first place because "!modifiers[m].pressed" would always be 0 >> or 1, and it is compared to 0 or 0x80. > > No, it's compared to 0 or 1 (there is a ! on the left hand side of the == too) Oh *facepalm*, you're right. I didn't notice the opening and closing parentheses in the correct position. >> This is exploiting implicit >> behavior which is almost always a bad thing to do if you need to >> understand code later. > > It's pretty clear and explicit I'd say. Maybe but still not very visible... But this should answer the OPs question why it is done in this way. Thanks for clarification, Kai

June 28, 2018

[PATCH] iphlpapi: Cast caddr_t to char* before doing pointer arithmetic

by Alex Henrie

Signed-off-by: Alex Henrie <alexhenrie24(a)gmail.com> --- Fixes https://bugs.winehq.org/show_bug.cgi?id=18164 dlls/iphlpapi/ifenum.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dlls/iphlpapi/ifenum.c b/dlls/iphlpapi/ifenum.c index eed1be15d1..c3abdf0e53 100644 --- a/dlls/iphlpapi/ifenum.c +++ b/dlls/iphlpapi/ifenum.c @@ -1013,13 +1013,13 @@ static DWORD enumIPAddresses(PDWORD pcAddresses, struct ifconf *ifc) if (ioctlRet == 0) { ifPtr = ifc->ifc_buf; - while (ifPtr && ifPtr < ifc->ifc_buf + ifc->ifc_len) { + while (ifPtr && (char *)ifPtr < ifc->ifc_buf + ifc->ifc_len) { struct ifreq *ifr = (struct ifreq *)ifPtr; if (ifr->ifr_addr.sa_family == AF_INET) numAddresses++; - ifPtr += ifreq_len((struct ifreq *)ifPtr); + ifPtr = (char *)ifPtr + ifreq_len((struct ifreq *)ifPtr); } } else @@ -1074,10 +1074,10 @@ DWORD getIPAddrTable(PMIB_IPADDRTABLE *ppIpAddrTable, HANDLE heap, DWORD flags) ret = NO_ERROR; (*ppIpAddrTable)->dwNumEntries = numAddresses; ifPtr = ifc.ifc_buf; - while (!ret && ifPtr && ifPtr < ifc.ifc_buf + ifc.ifc_len) { + while (!ret && ifPtr && (char *)ifPtr < ifc.ifc_buf + ifc.ifc_len) { struct ifreq *ifr = (struct ifreq *)ifPtr; - ifPtr += ifreq_len(ifr); + ifPtr = (char *)ifPtr + ifreq_len(ifr); if (ifr->ifr_addr.sa_family != AF_INET) continue; -- 2.18.0

June 28, 2018

Re: [PATCH 6/6] user32: For an owner-drawn listbox without strings WM_MEASUREITEM still needs correct itemData.

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=39445 Your paranoid android. === wvistau64_zh_CN (32 bit msg) === msg.c:5180: Test failed: ShowWindow(SW_RESTORE):overlapped: 18: the msg sequence is not complete: expected 0000 - actual 0088

June 28, 2018

Re: [PATCH 4/6] user32: Fix order of items passed in WM_COMPAREITEM data.

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=39443 Your paranoid android. === wvistau64_zh_CN (32 bit msg) === msg.c:5180: Test failed: ShowWindow(SW_RESTORE):overlapped: 18: the msg sequence is not complete: expected 0000 - actual 0088

June 28, 2018

Re: [PATCH 2/6] user32/tests: Add a message test for an owner-drawn sorted listbox.

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=39441 Your paranoid android. === wvistau64_zh_CN (32 bit msg) === msg.c:5182: Test failed: ShowWindow(SW_RESTORE):overlapped: 18: the msg sequence is not complete: expected 0000 - actual 0088

June 28, 2018

Re: [PATCH 1/6] user32/tests: Add a test for WM_MEASUREITEM when inserting an item to an owner-drawn listbox.

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=39440 Your paranoid android. === wvistau64_zh_CN (32 bit msg) === msg.c:5139: Test failed: ShowWindow(SW_RESTORE):overlapped: 18: the msg sequence is not complete: expected 0000 - actual 0088

June 28, 2018

[PATCH 6/6] user32: For an owner-drawn listbox without strings WM_MEASUREITEM still needs correct itemData.

by Dmitry Timoshkov

Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru> --- dlls/user32/listbox.c | 2 +- dlls/user32/tests/msg.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/user32/listbox.c b/dlls/user32/listbox.c index 9caccf6097..af8ee40311 100644 --- a/dlls/user32/listbox.c +++ b/dlls/user32/listbox.c @@ -1554,7 +1554,7 @@ static LRESULT LISTBOX_InsertItem( LB_DESCR *descr, INT index, mis.CtlType = ODT_LISTBOX; mis.CtlID = id; mis.itemID = index; - mis.itemData = data; + mis.itemData = str ? (ULONG_PTR)str : data; mis.itemHeight = descr->item_height; SendMessageW( descr->owner, WM_MEASUREITEM, id, (LPARAM)&mis ); item->height = mis.itemHeight ? mis.itemHeight : 1; diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c index da017667e0..052b9b9aae 100644 --- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -14387,7 +14387,7 @@ static void test_listbox_messages(void) ret = SendMessageA(listbox, LB_ADDSTRING, 0, (LPARAM)"item 2"); ok(ret == 2, "expected 2, got %ld\n", ret); - ok_sequence(wm_lb_addstring_sort_ownerdraw, "LB_ADDSTRING", TRUE); + ok_sequence(wm_lb_addstring_sort_ownerdraw, "LB_ADDSTRING", FALSE); check_lb_state(listbox, 3, LB_ERR, 0, 0); log_all_parent_messages--; -- 2.16.3

June 28, 2018

[PATCH 5/6] user32: Fix the listbox sorting algorithm.

by Dmitry Timoshkov

Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru> --- dlls/user32/listbox.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/dlls/user32/listbox.c b/dlls/user32/listbox.c index 490033902c..9caccf6097 100644 --- a/dlls/user32/listbox.c +++ b/dlls/user32/listbox.c @@ -828,10 +828,11 @@ static INT LISTBOX_FindStringPos( LB_DESCR *descr, LPCWSTR str, BOOL exact ) { INT index, min, max, res; - if (!(descr->style & LBS_SORT)) return -1; /* Add it at the end */ + if (!descr->nb_items || !(descr->style & LBS_SORT)) return -1; /* Add it at the end */ + min = 0; - max = descr->nb_items; - while (min != max) + max = descr->nb_items - 1; + while (min <= max) { index = (min + max) / 2; if (HAS_STRINGS(descr)) @@ -854,10 +855,10 @@ static INT LISTBOX_FindStringPos( LB_DESCR *descr, LPCWSTR str, BOOL exact ) res = SendMessageW( descr->owner, WM_COMPAREITEM, id, (LPARAM)&cis ); } if (!res) return index; - if (res > 0) max = index; + if (res > 0) max = index - 1; else min = index + 1; } - return exact ? -1 : max; + return exact ? -1 : min; } -- 2.16.3

June 28, 2018

[PATCH 4/6] user32: Fix order of items passed in WM_COMPAREITEM data.

by Dmitry Timoshkov

Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru> --- dlls/user32/listbox.c | 12 ++++++------ dlls/user32/tests/msg.c | 4 ---- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/dlls/user32/listbox.c b/dlls/user32/listbox.c index 8df67634d3..490033902c 100644 --- a/dlls/user32/listbox.c +++ b/dlls/user32/listbox.c @@ -835,7 +835,7 @@ static INT LISTBOX_FindStringPos( LB_DESCR *descr, LPCWSTR str, BOOL exact ) { index = (min + max) / 2; if (HAS_STRINGS(descr)) - res = LISTBOX_lstrcmpiW( descr->locale, str, descr->items[index].str); + res = LISTBOX_lstrcmpiW( descr->locale, descr->items[index].str, str ); else { COMPAREITEMSTRUCT cis; @@ -846,15 +846,15 @@ static INT LISTBOX_FindStringPos( LB_DESCR *descr, LPCWSTR str, BOOL exact ) cis.hwndItem = descr->self; /* note that some application (MetaStock) expects the second item * to be in the listbox */ - cis.itemID1 = -1; - cis.itemData1 = (ULONG_PTR)str; - cis.itemID2 = index; - cis.itemData2 = descr->items[index].data; + cis.itemID1 = index; + cis.itemData1 = descr->items[index].data; + cis.itemID2 = -1; + cis.itemData2 = (ULONG_PTR)str; cis.dwLocaleId = descr->locale; res = SendMessageW( descr->owner, WM_COMPAREITEM, id, (LPARAM)&cis ); } if (!res) return index; - if (res < 0) max = index; + if (res > 0) max = index; else min = index + 1; } return exact ? -1 : max; diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c index 5a5d5ed5fe..da017667e0 100644 --- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -2276,9 +2276,7 @@ static void add_message_(int line, const struct recvd_message *msg) ok(msg->wParam == cis->CtlID, "expected %#x, got %#lx\n", cis->CtlID, msg->wParam); ok(cis->hwndItem == ctrl, "expected %p, got %p\n", ctrl, cis->hwndItem); -todo_wine ok((int)cis->itemID1 >= 0, "expected >= 0, got %d\n", cis->itemID1); -todo_wine ok((int)cis->itemID2 == -1, "expected -1, got %d\n", cis->itemID2); sprintf( seq->output, "%s: %p WM_COMPAREITEM: CtlType %#x, CtlID %#x, itemID1 %#x, itemData1 %#lx, itemID2 %#x, itemData2 %#lx", @@ -14385,10 +14383,8 @@ static void test_listbox_messages(void) ret = SendMessageA(listbox, LB_ADDSTRING, 0, (LPARAM)"item 0"); ok(ret == 0, "expected 0, got %ld\n", ret); ret = SendMessageA(listbox, LB_ADDSTRING, 0, (LPARAM)"item 1"); -todo_wine ok(ret == 1, "expected 1, got %ld\n", ret); ret = SendMessageA(listbox, LB_ADDSTRING, 0, (LPARAM)"item 2"); -todo_wine ok(ret == 2, "expected 2, got %ld\n", ret); ok_sequence(wm_lb_addstring_sort_ownerdraw, "LB_ADDSTRING", TRUE); -- 2.16.3

June 28, 2018