comctl32: Add missing break (Coverity)

Michael Stefaniuc mstefani at redhat.com
Wed Feb 23 11:30:47 CST 2011


Nikolay Sivov wrote:
> On 2/23/2011 19:56, Michael Stefaniuc wrote:
>> Nikolay Sivov wrote:
>>> On 2/23/2011 19:28, Amine Khaldi wrote:
>>>> CIDs 1581 and 1583.
>>>> @@ -1852,6 +1852,7 @@ COMBOEX_EditWndProc (HWND hwnd, UINT uMsg,
>>>> WPARAM wParam, LPARAM lParam,
>>>>
>>>>           case VK_UP:
>>>>           step = -1;
>>>> +        break;
>>>>           case VK_DOWN:
>>>>           /* by default, step is 1 */
>>>>           oldItem = SendMessageW (infoPtr->hwndSelf, CB_GETCURSEL,
>>>> 0, 0);
>>> This is wrong.
>>>
>>>> @@ -2297,6 +2297,7 @@ static LRESULT
>>>> TOOLBAR_Cust_AvailDragListNotification(const CUSTDLG_INFO *custIn
>>>>               TOOLBAR_Cust_AddButton(custInfo, hwnd, nIndexFrom,
>>>> nIndexTo);
>>>>           }
>>>>       }
>>>> +    break;
>>>>       case DL_CANCELDRAG:
>>>>           /* Clear drag arrow */
>>>>           DrawInsert(hwnd, hwndList, -1);
>>> Why? Looks to me it's fine to clear on dropped case too. Coverity is a
>>> bit paranoid about missed breaks.
>> A missing break is not trivial to detect; especially if something is
>> done in a specific case. That's why Wine has a ton of "/* fall through
>> */" annotations. IMHO they should be added here too .
> Well, there's no third option basing on code - report all or report
> none, cause you can't figure out automatically is it ok to fall or not.
> A comment you mean is kind of suppression sign for checker?. Speaking
I don't really care about checkers but about people reading the code.
Adding code to silence a checker is bad taste; adding a comment to
improve the readability of the code is good.

> about this particular case, I have a better comboex fix in mind, and for
> toolbar I think it's fine to add a comment.
Cool, thanks.

bye
	michael



More information about the wine-devel mailing list