Explicit fall-through in switch statements?

[Frédéric Delanoy]

For every wine version, static checkers (like coverity) detect cases
where a switch case automatically falls-through to the next case.

Shouldn't be there a rule that such cases are always marked with a
"fall-through comment"?
With the possible exception of case with no statement? like 'case FOO:
case BAR: do_something();...'?

This would probably help IMHO, especially when people refactor old code.

Frédéric

[Bruno Jesus]

On Sun, Nov 25, 2012 at 9:20 PM, Frédéric Delanoy
<frederic.delanoy(a)gmail.com> wrote:
> For every wine version, static checkers (like coverity) detect cases
> where a switch case automatically falls-through to the next case.
>
> Shouldn't be there a rule that such cases are always marked with a
> "fall-through comment"?
> With the possible exception of case with no statement? like 'case FOO:
> case BAR: do_something();...'?

I remember about a similar discussion:
http://www.winehq.org/pipermail/wine-devel/2011-September/092475.html
> Frédéric

Best wishes,
Bruno

[Andrew Talbot]

Frédéric Delanoy wrote:
> For every wine version, static checkers (like coverity) detect cases
> where a switch case automatically falls-through to the next case.
>
> Shouldn't be there a rule that such cases are always marked with a
> "fall-through comment"?
> With the possible exception of case with no statement? like 'case FOO:
> case BAR: do_something();...'?
>
> This would probably help IMHO, especially when people refactor old code.
>
> Frédéric

I reckon there are currently about 105 unmarked fall-throughs in the dlls.

Unless people consider it offensive, one option might be to make careful use
of git blame to find out who wrote the particular cases and ask them whether
their fall-throughs were deliberate, marking up or fixing with breakS as
appropriate.

-- 
Andy.

[Vincent Povirk]

It might be useful to post a listing of the files where unmarked
fall-throughs (falls-through?) appear, and I could see if any of them
are on my turf.

[Andrew Talbot]

Vincent Povirk wrote:
> It might be useful to post a listing of the files where unmarked
> fall-throughs (falls-through?) appear, and I could see if any of them
> are on my turf.

Here is a rough-and-ready list of where they are.

dlls/msvcp100/exception.c (line 498) case EXCEPTION:
dlls/msvcp100/exception.c (line 503) case EXCEPTION_BAD_ALLOC:
dlls/msvcp100/exception.c (line 508) case EXCEPTION_LOGIC_ERROR:
dlls/msvcp100/exception.c (line 513) case EXCEPTION_LENGTH_ERROR:
dlls/msvcp100/exception.c (line 518) case EXCEPTION_OUT_OF_RANGE:
dlls/msvcp100/exception.c (line 523) case EXCEPTION_INVALID_ARGUMENT:
dlls/msvcp100/exception.c (line 528) case EXCEPTION_RUNTIME_ERROR:
dlls/msvcp100/exception.c (line 533) case EXCEPTION_FAILURE:

dlls/comctl32/listview.c (line 5875) default:

dlls/comctl32/monthcal.c (line 2059) default:

dlls/comctl32/tab.c (line 3465) case WM_SETFOCUS:

dlls/comctl32/toolbar.c (line 2307) case DL_CANCELDRAG:

dlls/comctl32/treeview.c (line 4113) case TVHT_ONITEMLABEL:

dlls/d3d10core/view.c (line 136) case D3D10_RESOURCE_DIMENSION_BUFFER:
dlls/d3d10core/view.c (line 375) case D3D10_RESOURCE_DIMENSION_BUFFER:

dlls/ddraw/utils.c (line 450) default:
dlls/ddraw/utils.c (line 464) case 8:

dlls/dmusic/collection.c (line 409) case 8:

dlls/dplayx/dplay.c (line 708) case DPMSGCMD_FORWARDADDPLAYER:

dlls/gdi32/bidi.c (line 515) default:

dlls/gdi32/painting.c (line 161) default:

dlls/gdiplus/graphics.c (line 962) default:

dlls/gdiplus/graphicspath.c (line 1756) default:

dlls/gphoto2.ds/gphoto2_main.c (line 1023) default:

dlls/inetcomm/pop3transport.c (line 129) default:
dlls/inetcomm/pop3transport.c (line 154) default:
dlls/inetcomm/pop3transport.c (line 200) default:
                                                   
dlls/jscript/dispex.c (line 457) case PROP_PROTREF:

dlls/jscript/regexp.c (line 1680) case PROP_PROTREF:
dlls/jscript/regexp.c (line 1941) case PROP_PROTREF:

dlls/kernel32/volume.c (line 786) case FS_FAT32:

dlls/mshtml/htmlevent.c (line 1093) case FS_FAT32:

dlls/mshtml/script.c (line 211) case FS_FAT32:

dlls/mshtml/script.c (line 216) case FS_FAT32:

dlls/msvcp90/exception.c (line 498) case FS_FAT32:
dlls/msvcp90/exception.c (line 503) case FS_FAT32:
dlls/msvcp90/exception.c (line 508) case FS_FAT32:
dlls/msvcp90/exception.c (line 513) case FS_FAT32:
dlls/msvcp90/exception.c (line 518) case FS_FAT32:
dlls/msvcp90/exception.c (line 523) case FS_FAT32:
dlls/msvcp90/exception.c (line 528) case FS_FAT32:
dlls/msvcp90/exception.c (line 533) case FS_FAT32:

dlls/msvcrtd/debug.c (line 61) case FS_FAT32:

dlls/netapi32/access.c (line 950) case FS_FAT32:

dlls/ntdll/reg.c (line 943) case FS_FAT32:

dlls/oleaut32/variant.c (line 4424) case VT_NULL:
dlls/oleaut32/variant.c (line 5512) case VT_NULL:
dlls/oleaut32/variant.c (line 5519) case VT_NULL:
dlls/oleaut32/variant.c (line 5533) case VT_NULL:
dlls/oleaut32/variant.c (line 5542) case VT_BSTR:

dlls/oledb32/convert.c (line 873) case VT_BSTR:

dlls/oledlg/insobjdlg.c (line 199) case IDC_OBJTYPELIST:

dlls/rsaenh/rsaenh.c (line 3780) case ERROR_SUCCESS:

dlls/sane.ds/ui.c (line 1013) case WM_COMMAND:

dlls/serialui/confdlg.c (line 405) default:

dlls/setupapi/queue.c (line 243) case SPFILENOTIFY_STARTQUEUE:

dlls/shell32/shelldispatch.c (line 709) default:

dlls/shell32/shellole.c (line 696) case OPEN_ALWAYS:
dlls/shell32/shellole.c (line 702) case OPEN_EXISTING:

dlls/shell32/shlmenu.c (line 1226) default:

dlls/shlwapi/reg.c (line 730) default:
dlls/shlwapi/reg.c (line 790) default:

dlls/urlmon/axinstall.c (line 258) case WM_TIMER:

dlls/urlmon/internet.c (line 789) default:

dlls/user32/edit.c (line 3163) case '\n':

dlls/user32/mdi.c (line 1329) case SC_SIZE:

dlls/user32/spy.c (line 2463) case WM_WINDOWPOSCHANGING:
dlls/user32/spy.c (line 2473) case WM_STYLECHANGING:
dlls/user32/spy.c (line 2523) default:

dlls/usp10/bidi.c (line 773) default:

dlls/usp10/breaking.c (line 131) case b_LF:

dlls/wined3d/glsl_shader.c (line 467) case HEAP_NODE_TRAVERSE_RIGHT:
dlls/wined3d/glsl_shader.c (line 483) case HEAP_NODE_POP:
dlls/wined3d/glsl_shader.c (line 541) case HEAP_NODE_TRAVERSE_RIGHT:
dlls/wined3d/glsl_shader.c (line 556) case HEAP_NODE_POP:
dlls/wined3d/glsl_shader.c (line 5129) case WINED3D_TOP_BUMPENVMAP:
dlls/wined3d/glsl_shader.c (line 5131) case WINED3D_TOP_BLEND_TEXTURE_ALPHA:

dlls/wined3d/nvidia_texture_shader.c (line 466) default:

dlls/wined3d/state.c (line 2632) default:
dlls/wined3d/state.c (line 3093) default:

dlls/wined3d/surface.c (line 1183) case WINED3D_TEXF_NONE:
dlls/wined3d/surface.c (line 3022) case WINED3D_CONTAINER_NONE:
dlls/wined3d/surface.c (line 3050) case WINED3D_CONTAINER_NONE:

dlls/winex11.drv/palette.c (line 149) case GrayScale:
dlls/winex11.drv/palette.c (line 178) case StaticColor:

dlls/winex11.drv/xrender.c (line 1115) case AA_None:

dlls/wininet/internet.c (line 3485) case INTERNET_SCHEME_GOPHER:

dlls/msvcp60/exception.c (line 613) case EXCEPTION:
dlls/msvcp60/exception.c (line 618) case EXCEPTION_BAD_ALLOC:
dlls/msvcp60/exception.c (line 623) case EXCEPTION_LOGIC_ERROR:
dlls/msvcp60/exception.c (line 628) case EXCEPTION_LENGTH_ERROR:
dlls/msvcp60/exception.c (line 633) case EXCEPTION_OUT_OF_RANGE:
dlls/msvcp60/exception.c (line 638) case EXCEPTION_INVALID_ARGUMENT:
dlls/msvcp60/exception.c (line 643) case EXCEPTION_RUNTIME_ERROR:
dlls/msvcp60/exception.c (line 648) case EXCEPTION_FAILURE:
dlls/msvcp60/exception.c (line 653) default:

dlls/msvcp71/exception.c (line 498) case EXCEPTION:
dlls/msvcp71/exception.c (line 503) case EXCEPTION_BAD_ALLOC:
dlls/msvcp71/exception.c (line 508) case EXCEPTION_LOGIC_ERROR:
dlls/msvcp71/exception.c (line 513) case EXCEPTION_LENGTH_ERROR:
dlls/msvcp71/exception.c (line 518) case EXCEPTION_OUT_OF_RANGE:
dlls/msvcp71/exception.c (line 523) case EXCEPTION_INVALID_ARGUMENT:
dlls/msvcp71/exception.c (line 528) case EXCEPTION_RUNTIME_ERROR:
dlls/msvcp71/exception.c (line 533) case EXCEPTION_FAILURE:

-- 
Andy.

[Andrew Talbot]

Perhaps I should add that the list is of caseS/defaultS that may be fallen
through to, rather than out from.

Unfortunately,because I produced it in a hurry, it does contain the odd
copy-and-paste error for case names (e.g., the case for dmusic/collection.c
line 409 should be default:, not case 8:, and that for
dlls/oledb32/convert.c line 873 should be case DBTYPE_NUMERIC: not case
VT_BSTR:), but I think the line numbers are more trustworthy.

-- 
Andy.