clang -Wimplicit-fallthrough and missing breaks

[Markus Mohrhard]

Hey,

On Wed, May 25, 2016 at 9:20 AM, Stephan Bergmann <sbergman at redhat.com>
wrote:

> On 05/25/2016 09:01 AM, Tor Lillqvist wrote:
>
>>
>>         --- a/vcl/opengl/win/gdiimpl.cxx
>>         +++ b/vcl/opengl/win/gdiimpl.cxx
>>         @@ -166,6 +166,7 @@ static LRESULT CALLBACK WndProc(HWND hwnd,
>>         UINT message, WPARAM wParam, LPARAM l
>>                  case VK_SPACE:
>>                      break;
>>                  }
>>         +        SAL_FALLTHROUGH; //TODO ???
>>              default:
>>                  return DefWindowProc(hwnd, message, wParam, lParam);
>>              }
>>
>>
>> I did not write this code, but looking at it, it seems fairly obvious
>> that the fall-through is intentional here. But it is beyond my
>> understanding why such a temporary window, that is used only for some
>> milliseconds, and never even displayed, would need to handle the Escape
>> and Space keys specially.
>>
>
> But it does not handle VK_SPACE specially:
>
> static LRESULT CALLBACK WndProc(HWND hwnd, UINT message, WPARAM wParam,
>> LPARAM lParam)
>> {
>>     switch (message)
>>     {
>>     case WM_CREATE:
>>         return 0;
>>     case WM_CLOSE:
>>         PostQuitMessage(0);
>>         return 0;
>>     case WM_DESTROY:
>>         return 0;
>>     case WM_KEYDOWN:
>>         switch(wParam)
>>         {
>>         case VK_ESCAPE:
>>             PostQuitMessage(0);
>>             return 0;
>>
>>         case VK_SPACE:
>>             break;
>>         }
>>         SAL_FALLTHROUGH; //TODO ???
>>     default:
>>         return DefWindowProc(hwnd, message, wParam, lParam);
>>     }
>> }
>>
>
> In the inner switch (in case WM_KEYDOWN), VK_SPACE and the (implicit)
> default both fall through to the outer switch's default case, which looks
> rather dubious to me (why mention VK_SPACE explicitly, then?). The code is
> like that ever since <
> https://cgit.freedesktop.org/libreoffice/core/commit/?id=0f6fca34909535b48bad41e73e5d3d0e86c744b9>
> "add anti-aliasing init"; whose author is in CC now.



That whole WM_KEYDOWN code is unnecessary.



>
>
> _______________________________________________
> LibreOffice mailing list
> LibreOffice at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/libreoffice
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/libreoffice/attachments/20160525/3e7f721d/attachment.html>