[PATCH 1/9] comctl32/button: Use assignment instead of CopyRect
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- These notes apply to the entire patch series (but each patch is independent). Since patch 156187 was rejected and the reason for lack of including CopyRect in that patch was "it makes no sense. A straight assignment should be used instead", this patch series will do just that, and replace the remaining CopyRect calls with straight assignments (and some NULL checks where needed). To avoid redundant NULL checks, they have been added only to the rects that can actually be NULL. dlls/comctl32/button.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dlls/comctl32/button.c b/dlls/comctl32/button.c index 56a4730..46e85a9 100644 --- a/dlls/comctl32/button.c +++ b/dlls/comctl32/button.c @@ -1362,9 +1362,9 @@ static UINT BUTTON_CalcLayoutRects(const BUTTON_INFO *infoPtr, HDC hdc, RECT *la } heap_free(text); - CopyRect(labelRc, &labelRect); - CopyRect(imageRc, &imageRect); - CopyRect(textRc, &textRect); + *labelRc = labelRect; + if (imageRc) *imageRc = imageRect; + if (textRc) *textRc = textRect; return dtStyle; } -- 2.19.1
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/comctl32/combo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/comctl32/combo.c b/dlls/comctl32/combo.c index 12ea7bb..72a732a 100644 --- a/dlls/comctl32/combo.c +++ b/dlls/comctl32/combo.c @@ -333,7 +333,7 @@ static void CBCalcPlacement( /* * The button starts the same vertical position as the text area. */ - CopyRect(lprButton, lprEdit); + *lprButton = *lprEdit; /* * If the combobox is "simple" there is no button. -- 2.19.1
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/comctl32/edit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/comctl32/edit.c b/dlls/comctl32/edit.c index 1f17276..0a234bd 100644 --- a/dlls/comctl32/edit.c +++ b/dlls/comctl32/edit.c @@ -2260,7 +2260,7 @@ static void EDIT_SetRectNP(EDITSTATE *es, const RECT *rc) INT bw, bh; ExStyle = GetWindowLongPtrW(es->hwndSelf, GWL_EXSTYLE); - CopyRect(&es->format_rect, rc); + if (rc) es->format_rect = *rc; if (ExStyle & WS_EX_CLIENTEDGE) { es->format_rect.left++; -- 2.19.1
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/comctl32/trackbar.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dlls/comctl32/trackbar.c b/dlls/comctl32/trackbar.c index 7cb73ca..fa63453 100644 --- a/dlls/comctl32/trackbar.c +++ b/dlls/comctl32/trackbar.c @@ -1923,7 +1923,9 @@ TRACKBAR_WindowProc (HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam) return infoPtr->uThumbLen; case TBM_GETTHUMBRECT: - return CopyRect((LPRECT)lParam, &infoPtr->rcThumb); + if (!lParam) return FALSE; + *(RECT*)lParam = infoPtr->rcThumb; + return TRUE; case TBM_GETTIC: return TRACKBAR_GetTic (infoPtr, (INT)wParam); -- 2.19.1
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/user32/combo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/user32/combo.c b/dlls/user32/combo.c index 59c2e64..cb055bc 100644 --- a/dlls/user32/combo.c +++ b/dlls/user32/combo.c @@ -344,7 +344,7 @@ static void CBCalcPlacement( /* * The button starts the same vertical position as the text area. */ - CopyRect(lprButton, lprEdit); + *lprButton = *lprEdit; /* * If the combobox is "simple" there is no button. -- 2.19.1
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=45980 Your paranoid android. === debian9 (32 bit WoW report) === user32: msg.c:8713: Test failed: WaitForSingleObject failed 102 msg.c:8719: Test failed: destroy child on thread exit: 0: the msg 0x0082 was expected, but got msg 0x000f instead msg.c:8719: Test failed: destroy child on thread exit: 1: the msg 0x000f was expected, but got msg 0x0014 instead msg.c:8719: Test failed: destroy child on thread exit: 2: the msg sequence is not complete: expected 0014 - actual 0000 === debian9 (64 bit WoW report) === user32: msg.c:8713: Test failed: WaitForSingleObject failed 102 msg.c:8719: Test failed: destroy child on thread exit: 0: the msg 0x0082 was expected, but got msg 0x000f instead msg.c:8719: Test failed: destroy child on thread exit: 1: the msg 0x000f was expected, but got msg 0x0014 instead msg.c:8719: Test failed: destroy child on thread exit: 2: the msg sequence is not complete: expected 0014 - actual 0000
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/user32/edit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/user32/edit.c b/dlls/user32/edit.c index 6f8f412..0c1237d 100644 --- a/dlls/user32/edit.c +++ b/dlls/user32/edit.c @@ -2346,7 +2346,7 @@ static void EDIT_SetRectNP(EDITSTATE *es, const RECT *rc) INT bw, bh; ExStyle = GetWindowLongPtrW(es->hwndSelf, GWL_EXSTYLE); - CopyRect(&es->format_rect, rc); + if (rc) es->format_rect = *rc; if (ExStyle & WS_EX_CLIENTEDGE) { es->format_rect.left++; -- 2.19.1
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=45981 Your paranoid android. === debian9 (32 bit WoW report) === user32: input.c:2054: Test failed: expected WM_NCHITTEST message
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/quartz/videorenderer.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dlls/quartz/videorenderer.c b/dlls/quartz/videorenderer.c index 61f08f5..1d2db3e 100644 --- a/dlls/quartz/videorenderer.c +++ b/dlls/quartz/videorenderer.c @@ -483,7 +483,7 @@ static const BaseWindowFuncTable renderer_BaseWindowFuncTable = { static HRESULT WINAPI VideoRenderer_GetSourceRect(BaseControlVideo* iface, RECT *pSourceRect) { VideoRendererImpl *This = impl_from_BaseControlVideo(iface); - CopyRect(pSourceRect,&This->SourceRect); + if (pSourceRect) *pSourceRect = This->SourceRect; return S_OK; } @@ -549,7 +549,7 @@ static HRESULT WINAPI VideoRenderer_GetStaticImage(BaseControlVideo* iface, LONG static HRESULT WINAPI VideoRenderer_GetTargetRect(BaseControlVideo* iface, RECT *pTargetRect) { VideoRendererImpl *This = impl_from_BaseControlVideo(iface); - CopyRect(pTargetRect,&This->DestRect); + if (pTargetRect) *pTargetRect = This->DestRect; return S_OK; } @@ -616,14 +616,14 @@ static HRESULT WINAPI VideoRenderer_SetDefaultTargetRect(BaseControlVideo* iface static HRESULT WINAPI VideoRenderer_SetSourceRect(BaseControlVideo* iface, RECT *pSourceRect) { VideoRendererImpl *This = impl_from_BaseControlVideo(iface); - CopyRect(&This->SourceRect,pSourceRect); + if (pSourceRect) This->SourceRect = *pSourceRect; return S_OK; } static HRESULT WINAPI VideoRenderer_SetTargetRect(BaseControlVideo* iface, RECT *pTargetRect) { VideoRendererImpl *This = impl_from_BaseControlVideo(iface); - CopyRect(&This->DestRect,pTargetRect); + if (pTargetRect) This->DestRect = *pTargetRect; return S_OK; } -- 2.19.1
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=45982 Your paranoid android. === debian9 (32 bit WoW report) === user32: menu.c:2354: Test failed: test 27 menu: Timeout
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/quartz/vmr9.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dlls/quartz/vmr9.c b/dlls/quartz/vmr9.c index 5aa9799..413638b 100644 --- a/dlls/quartz/vmr9.c +++ b/dlls/quartz/vmr9.c @@ -582,7 +582,7 @@ static const BaseWindowFuncTable renderer_BaseWindowFuncTable = { static HRESULT WINAPI VMR9_GetSourceRect(BaseControlVideo* This, RECT *pSourceRect) { struct quartz_vmr* pVMR9 = impl_from_BaseControlVideo(This); - CopyRect(pSourceRect,&pVMR9->source_rect); + if (pSourceRect) *pSourceRect = pVMR9->source_rect; return S_OK; } @@ -648,7 +648,7 @@ static HRESULT WINAPI VMR9_GetStaticImage(BaseControlVideo* This, LONG *pBufferS static HRESULT WINAPI VMR9_GetTargetRect(BaseControlVideo* This, RECT *pTargetRect) { struct quartz_vmr* pVMR9 = impl_from_BaseControlVideo(This); - CopyRect(pTargetRect,&pVMR9->target_rect); + if (pTargetRect) *pTargetRect = pVMR9->target_rect; return S_OK; } @@ -715,14 +715,14 @@ static HRESULT WINAPI VMR9_SetDefaultTargetRect(BaseControlVideo* This) static HRESULT WINAPI VMR9_SetSourceRect(BaseControlVideo* This, RECT *pSourceRect) { struct quartz_vmr* pVMR9 = impl_from_BaseControlVideo(This); - CopyRect(&pVMR9->source_rect,pSourceRect); + if (pSourceRect) pVMR9->source_rect = *pSourceRect; return S_OK; } static HRESULT WINAPI VMR9_SetTargetRect(BaseControlVideo* This, RECT *pTargetRect) { struct quartz_vmr* pVMR9 = impl_from_BaseControlVideo(This); - CopyRect(&pVMR9->target_rect,pTargetRect); + if (pTargetRect) pVMR9->target_rect = *pTargetRect; return S_OK; } -- 2.19.1
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=45983 Your paranoid android. === debian9 (32 bit report) === user32: msg.c:8713: Test failed: WaitForSingleObject failed 102 msg.c:8719: Test failed: destroy child on thread exit: 0: the msg 0x0082 was expected, but got msg 0x000f instead msg.c:8719: Test failed: destroy child on thread exit: 1: the msg 0x000f was expected, but got msg 0x0014 instead msg.c:8719: Test failed: destroy child on thread exit: 2: the msg sequence is not complete: expected 0014 - actual 0000 === debian9 (32 bit Chinese:China report) === user32: menu.c:2354: Test failed: test 27 menu: Timeout msg.c:14491: Test failed: bad time 47a5b83 === debian9 (64 bit WoW report) === user32: input.c:2054: Test failed: expected WM_NCHITTEST message msg.c:8713: Test failed: WaitForSingleObject failed 102 msg.c:8719: Test failed: destroy child on thread exit: 0: the msg 0x0082 was expected, but got msg 0x000f instead msg.c:8719: Test failed: destroy child on thread exit: 1: the msg 0x000f was expected, but got msg 0x0014 instead msg.c:8719: Test failed: destroy child on thread exit: 2: the msg sequence is not complete: expected 0014 - actual 0000
I'm assuming those methods will do argument validation with proper error code should the RECTs be NULL. Needs tests of course. But that's the reason I didn't replace those when I did my CopyRect() removals back in the day. Also we are in change freeze and pure janitorial work is kinda forbidden during that; especially in the later rc versions. bye michael On 12/28/18 1:23 PM, Gabriel Ivăncescu wrote:
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/quartz/vmr9.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/dlls/quartz/vmr9.c b/dlls/quartz/vmr9.c index 5aa9799..413638b 100644 --- a/dlls/quartz/vmr9.c +++ b/dlls/quartz/vmr9.c @@ -582,7 +582,7 @@ static const BaseWindowFuncTable renderer_BaseWindowFuncTable = { static HRESULT WINAPI VMR9_GetSourceRect(BaseControlVideo* This, RECT *pSourceRect) { struct quartz_vmr* pVMR9 = impl_from_BaseControlVideo(This); - CopyRect(pSourceRect,&pVMR9->source_rect); + if (pSourceRect) *pSourceRect = pVMR9->source_rect; return S_OK; }
@@ -648,7 +648,7 @@ static HRESULT WINAPI VMR9_GetStaticImage(BaseControlVideo* This, LONG *pBufferS static HRESULT WINAPI VMR9_GetTargetRect(BaseControlVideo* This, RECT *pTargetRect) { struct quartz_vmr* pVMR9 = impl_from_BaseControlVideo(This); - CopyRect(pTargetRect,&pVMR9->target_rect); + if (pTargetRect) *pTargetRect = pVMR9->target_rect; return S_OK; }
@@ -715,14 +715,14 @@ static HRESULT WINAPI VMR9_SetDefaultTargetRect(BaseControlVideo* This) static HRESULT WINAPI VMR9_SetSourceRect(BaseControlVideo* This, RECT *pSourceRect) { struct quartz_vmr* pVMR9 = impl_from_BaseControlVideo(This); - CopyRect(&pVMR9->source_rect,pSourceRect); + if (pSourceRect) pVMR9->source_rect = *pSourceRect; return S_OK; }
static HRESULT WINAPI VMR9_SetTargetRect(BaseControlVideo* This, RECT *pTargetRect) { struct quartz_vmr* pVMR9 = impl_from_BaseControlVideo(This); - CopyRect(&pVMR9->target_rect,pTargetRect); + if (pTargetRect) pVMR9->target_rect = *pTargetRect; return S_OK; }
On 12/28/18 23:20, Michael Stefaniuc wrote:
I'm assuming those methods will do argument validation with proper error code should the RECTs be NULL. Needs tests of course. But that's the reason I didn't replace those when I did my CopyRect() removals back in the day.
Also we are in change freeze and pure janitorial work is kinda forbidden during that; especially in the later rc versions.
bye michael
Sorry, I had assumed that it's mostly janitorial work that is acceptable during code freeze, that's why I sent them, my bad :-) To be honest, I agree with Nikolay's point about most of these remaining CopyRect (except those not requiring NULL checks, like the combobox CopyRect removal). But in such case, they should have inline helpers to be consistent with the others. Hopefully it's reconsidered after code freeze (the original inline patch), though I will split it into PtInRect and CopyRect separately, at least for consistency.
On 12/29/18 12:24 PM, Gabriel Ivăncescu wrote:
On 12/28/18 23:20, Michael Stefaniuc wrote:
I'm assuming those methods will do argument validation with proper error code should the RECTs be NULL. Needs tests of course. But that's the reason I didn't replace those when I did my CopyRect() removals back in the day.
Also we are in change freeze and pure janitorial work is kinda forbidden during that; especially in the later rc versions.
Sorry, I had assumed that it's mostly janitorial work that is acceptable during code freeze, that's why I sent them, my bad :-)
To be honest, I agree with Nikolay's point about most of these remaining CopyRect (except those not requiring NULL checks, like the combobox CopyRect removal).
But in such case, they should have inline helpers to be consistent with the others. Hopefully it's reconsidered after code freeze (the original inline patch), though I will split it into PtInRect and CopyRect separately, at least for consistency. Really, there is no need for consistency here. The inline versions were introduced to avoid the addition of a cross dll function call when the open coded stuff was replaced.
bye michael
On 12/30/18 00:27, Michael Stefaniuc wrote:
On 12/29/18 12:24 PM, Gabriel Ivăncescu wrote:
On 12/28/18 23:20, Michael Stefaniuc wrote:
I'm assuming those methods will do argument validation with proper error code should the RECTs be NULL. Needs tests of course. But that's the reason I didn't replace those when I did my CopyRect() removals back in the day.
Also we are in change freeze and pure janitorial work is kinda forbidden during that; especially in the later rc versions.
Sorry, I had assumed that it's mostly janitorial work that is acceptable during code freeze, that's why I sent them, my bad :-)
To be honest, I agree with Nikolay's point about most of these remaining CopyRect (except those not requiring NULL checks, like the combobox CopyRect removal).
But in such case, they should have inline helpers to be consistent with the others. Hopefully it's reconsidered after code freeze (the original inline patch), though I will split it into PtInRect and CopyRect separately, at least for consistency. Really, there is no need for consistency here. The inline versions were introduced to avoid the addition of a cross dll function call when the open coded stuff was replaced.
bye michael
But the cross dll function call happens in this case also. I understand why they were introduced, but it doesn't mean that should be the only reason they can be introduced. For example, if this function started with open coding and then got replaced with CopyRect, I assume you would add the inline version, correct? Even though the code in the end would be identical to now. That's why it seems so arbitrary and confusing for someone who skims through the code. I understand why you did not add them yourself, but that doesn't mean they shouldn't be added, at least for consistency's sake. IMO. Clearly it breaks nothing since if they were to replace an open-coded variant, they would *also* affect any other code that currently uses CopyRect, and obviously in that case I'm sure it would be accepted immediately...
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/usp10/usp10.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/usp10/usp10.c b/dlls/usp10/usp10.c index abc2605..8d41703 100644 --- a/dlls/usp10/usp10.c +++ b/dlls/usp10/usp10.c @@ -2227,7 +2227,7 @@ static HRESULT SS_ItemOut( SCRIPT_STRING_ANALYSIS ssa, (cEnd >= 0 && analysis->pItem[iItem].iCharPos >= cEnd)) return S_OK; - CopyRect(&crc,prc); + if (prc) crc = *prc; if (fSelected) { BkMode = GetBkMode(analysis->hdc); -- 2.19.1
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=45984 Your paranoid android. === debian9 (32 bit report) === user32: menu.c:2354: Test failed: test 27 === debian9 (32 bit WoW report) === user32: menu.c:2354: Test failed: test 27 menu: Timeout === debian9 (64 bit WoW report) === user32: clipboard.c:760: Test failed: 1: gle 5 clipboard.c:765: Test failed: 1.0: got 0000 instead of 0007 clipboard.c:805: Test failed: 1: gle 1418 clipboard.c:815: Test failed: 1: count 4 clipboard.c:818: Test failed: 1: gle 1418 clipboard.c:833: Test failed: 1: gle 5 clipboard.c:838: Test failed: 1.0: got 0000 instead of 0007 clipboard.c:868: Test failed: 1: gle 1418 clipboard.c:717: Test failed: 2: gle 5 clipboard.c:719: Test failed: 2: gle 1418 clipboard.c:746: Test failed: 2: count 4 clipboard.c:749: Test failed: 2: gle 1418 clipboard.c:760: Test failed: 2: gle 5 clipboard.c:765: Test failed: 2.0: got 0000 instead of 000d clipboard.c:805: Test failed: 2: gle 1418 clipboard.c:815: Test failed: 2: count 4 clipboard.c:818: Test failed: 2: gle 1418 clipboard.c:833: Test failed: 2: gle 5 clipboard.c:838: Test failed: 2.0: got 0000 instead of 000d clipboard.c:868: Test failed: 2: gle 1418 clipboard.c:717: Test failed: 3: gle 5 clipboard.c:719: Test failed: 3: gle 1418 clipboard.c:746: Test failed: 3: count 5 clipboard.c:749: Test failed: 3: gle 1418 clipboard.c:755: Test failed: 3: 0003 not available clipboard.c:757: Test failed: 3: count 5 instead of 2 clipboard.c:760: Test failed: 3: gle 5 clipboard.c:765: Test failed: 3.0: got 0000 instead of 000e clipboard.c:805: Test failed: 3: gle 1418 clipboard.c:815: Test failed: 3: count 5 clipboard.c:818: Test failed: 3: gle 1418 clipboard.c:826: Test failed: 3: 0003 not available clipboard.c:828: Test failed: 3: count 5 instead of 2 clipboard.c:833: Test failed: 3: gle 5 clipboard.c:838: Test failed: 3.0: got 0000 instead of 000e clipboard.c:868: Test failed: 3: gle 1418 clipboard.c:717: Test failed: 4: gle 5 clipboard.c:719: Test failed: 4: gle 1418 clipboard.c:746: Test failed: 4: count 6 clipboard.c:749: Test failed: 4: gle 1418 clipboard.c:757: Test failed: 4: count 6 instead of 2 clipboard.c:760: Test failed: 4: gle 5 clipboard.c:765: Test failed: 4.0: got 0000 instead of 0003 clipboard.c:805: Test failed: 4: gle 1418 clipboard.c:815: Test failed: 4: count 6 clipboard.c:818: Test failed: 4: gle 1418 clipboard.c:828: Test failed: 4: count 6 instead of 2 clipboard.c:833: Test failed: 4: gle 5 clipboard.c:838: Test failed: 4.0: got 0000 instead of 0003 clipboard.c:868: Test failed: 4: gle 1418 clipboard.c:717: Test failed: 5: gle 5 clipboard.c:719: Test failed: 5: gle 1418 clipboard.c:746: Test failed: 5: count 7 clipboard.c:749: Test failed: 5: gle 1418 clipboard.c:755: Test failed: 5: 0008 not available clipboard.c:755: Test failed: 5: 0011 not available clipboard.c:757: Test failed: 5: count 7 instead of 3 clipboard.c:760: Test failed: 5: gle 5 clipboard.c:765: Test failed: 5.0: got 0000 instead of 0002 clipboard.c:805: Test failed: 5: gle 1418 clipboard.c:815: Test failed: 5: count 7 clipboard.c:818: Test failed: 5: gle 1418 clipboard.c:826: Test failed: 5: 0008 not available clipboard.c:826: Test failed: 5: 0011 not available clipboard.c:828: Test failed: 5: count 7 instead of 3 clipboard.c:833: Test failed: 5: gle 5 clipboard.c:838: Test failed: 5.0: got 0000 instead of 0002 clipboard.c:868: Test failed: 5: gle 1418 clipboard.c:717: Test failed: 6: gle 5 clipboard.c:719: Test failed: 6: gle 1418 clipboard.c:746: Test failed: 6: count 8 clipboard.c:749: Test failed: 6: gle 1418 clipboard.c:755: Test failed: 6: 0011 not available clipboard.c:757: Test failed: 6: count 8 instead of 3 clipboard.c:760: Test failed: 6: gle 5 clipboard.c:765: Test failed: 6.0: got 0000 instead of 0008 clipboard.c:805: Test failed: 6: gle 1418 clipboard.c:815: Test failed: 6: count 8 clipboard.c:818: Test failed: 6: gle 1418 clipboard.c:826: Test failed: 6: 0011 not available clipboard.c:828: Test failed: 6: count 8 instead of 3 clipboard.c:833: Test failed: 6: gle 5 clipboard.c:838: Test failed: 6.0: got 0000 instead of 0008 clipboard.c:868: Test failed: 6: gle 1418 clipboard.c:717: Test failed: 7: gle 5 clipboard.c:719: Test failed: 7: gle 1418 clipboard.c:746: Test failed: 7: count 9 clipboard.c:749: Test failed: 7: gle 1418 clipboard.c:757: Test failed: 7: count 9 instead of 3 clipboard.c:760: Test failed: 7: gle 5 clipboard.c:765: Test failed: 7.0: got 0000 instead of 0011 clipboard.c:805: Test failed: 7: gle 1418 clipboard.c:815: Test failed: 7: count 9 clipboard.c:818: Test failed: 7: gle 1418 clipboard.c:828: Test failed: 7: count 9 instead of 3 clipboard.c:833: Test failed: 7: gle 5 clipboard.c:838: Test failed: 7.0: got 0000 instead of 0011 clipboard.c:868: Test failed: 7: gle 1418 clipboard.c:874: Test failed: gle 5 clipboard.c:876: Test failed: gle 1418 clipboard.c:878: Test failed: gle 1418 clipboard.c:1089: Test failed: OpenClipboard failed: 5 clipboard.c:1094: Test failed: EmptyClipboard failed: 1418 clipboard.c:1096: Test failed: sequence diff 0 clipboard.c:1099: Test failed: EmptyClipboard failed: 1418 clipboard.c:1101: Test failed: sequence diff 0 clipboard.c:1101: Test failed: WM_CLIPBOARDUPDATE received clipboard.c:1101: Test failed: WM_DESTROYCLIPBOARD not received clipboard.c:1106: Test failed: SetClipboardData failed: 1418 clipboard.c:1108: Test failed: sequence diff 0 clipboard.c:1108: Test failed: WM_DRAWCLIPBOARD received clipboard.c:1112: Test failed: sequence diff 0 clipboard.c:1116: Test failed: sequence diff 0 clipboard.c:1120: Test failed: CF_OEMTEXT available clipboard.c:1124: Test failed: CloseClipboard failed: 1418 clipboard.c:1127: Test failed: sequence diff 0 clipboard.c:1127: Test failed: WM_DRAWCLIPBOARD not received clipboard.c:1127: Test failed: WM_CLIPBOARDUPDATE not received clipboard.c:1138: Test failed: wrong owner (nil) clipboard.c:1142: Test failed: WM_RENDERFORMAT received 0000 clipboard.c:1146: Test failed: didn't get data for CF_OEMTEXT clipboard.c:1148: Test failed: WM_RENDERFORMAT received 0000 clipboard.c:1174: Test failed: WM_DESTROYCLIPBOARD not received clipboard.c:1176: Test failed: wrong format count 0 on WM_DESTROYCLIPBOARD
On 12/28/18 3:22 PM, Gabriel Ivăncescu wrote:
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> ---
These notes apply to the entire patch series (but each patch is independent).
Since patch 156187 was rejected and the reason for lack of including CopyRect in that patch was "it makes no sense. A straight assignment should be used instead", this patch series will do just that, and replace the remaining CopyRect calls with straight assignments (and some NULL checks where needed).
To avoid redundant NULL checks, they have been added only to the rects that can actually be NULL.
Even if CopyRect() doesn't necessarily make sense sometimes, it's not wrong to use it. I personally don't think such cleanup is useful unless you change surrounding code and going for straight assignment without null check works better.
case TBM_GETTHUMBRECT: - return CopyRect((LPRECT)lParam, &infoPtr->rcThumb); + if (!lParam) return FALSE; + *(RECT*)lParam = infoPtr->rcThumb; + return TRUE;
That's an example when it doesn't improve anything.
On 12/28/18 21:35, Nikolay Sivov wrote:
On 12/28/18 3:22 PM, Gabriel Ivăncescu wrote:
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> ---
These notes apply to the entire patch series (but each patch is independent).
Since patch 156187 was rejected and the reason for lack of including CopyRect in that patch was "it makes no sense. A straight assignment should be used instead", this patch series will do just that, and replace the remaining CopyRect calls with straight assignments (and some NULL checks where needed).
To avoid redundant NULL checks, they have been added only to the rects that can actually be NULL.
Even if CopyRect() doesn't necessarily make sense sometimes, it's not wrong to use it. I personally don't think such cleanup is useful unless you change surrounding code and going for straight assignment without null check works better.
case TBM_GETTHUMBRECT: - return CopyRect((LPRECT)lParam, &infoPtr->rcThumb); + if (!lParam) return FALSE; + *(RECT*)lParam = infoPtr->rcThumb; + return TRUE;
That's an example when it doesn't improve anything.
Yeah, I understand that, and I agree, but then it should be made inline like the others. I don't get why that patch was rejected in the first place, then. It seems totally arbitrary and inconsistent, there's nothing special about CopyRect compared to the others, it should also be there. Hopefully after code freeze and perhaps separated from the PtInRect patch, it will be reconsidered... (the inline patch, not this one)
participants (4)
-
Gabriel Ivăncescu -
Marvin -
Michael Stefaniuc -
Nikolay Sivov