On Fri, Mar 30, 2018 at 12:03 PM, Brandon Williams [off-list ref] wrote:

On 03/30, Ævar Arnfjörð Bjarmason wrote:

quoted

Change the switch statement driving upload_pack_v2() and
do_fetch_pack_v2() to clearly indicate that the FETCH_DONE case is
being handled implicitly by other code, instead of giving the reader
the impression that the "continue" statement is needed.

This issue was flagged as DEADCODE by Coverity[1]. Simply removing the
"case FETCH_DONE" would make -Wswitch warn. Instead implement the same
solution discussed for my "[PATCH v2 18/29] grep: catch a missing enum
in switch statement" patch[2] (which never made it into git.git).

1. https://public-inbox.org/git/CAGZ79kbAOcwaRzjuMtZ_HVsYvUr_7UAPbOcnrmPgsdE19q=PrQ@mail.gmail.com/
2. https://public-inbox.org/git/20170513231509.7834-19-avarab@gmail.com/

I understand why you want this change, but I dislike it because it
removes the ability to have the compiler tell you that your switch
statements are exhaustive.  Of course it should be noticed rather
quickly by the addition of those BUG statements :)

I think coverity doesn't flag empty sections, i.e.

    case FETCH_DONE:
    default:
        BUG(...)

would do for coverity. Not sure if we want to add a /*fall thru */
comment, that would aid other compilers to not warn about it.

This would cover both the compiler as well as coverity.

Stefan