On 03/30, Ævar Arnfjörð Bjarmason wrote: > 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@xxxxxxxxxxxxxx/ > 2. https://public-inbox.org/git/20170513231509.7834-19-avarab@xxxxxxxxx/ 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 :) > > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> > --- > fetch-pack.c | 4 ++-- > upload-pack.c | 4 ++-- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/fetch-pack.c b/fetch-pack.c > index 216d1368be..3a16b4bc1a 100644 > --- a/fetch-pack.c > +++ b/fetch-pack.c > @@ -1393,8 +1393,8 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args, > > state = FETCH_DONE; > break; > - case FETCH_DONE: > - continue; > + default: > + BUG("Added a new fetch_state without updating switch"); > } > } > > diff --git a/upload-pack.c b/upload-pack.c > index 87b4d32a6e..b7a7601c83 100644 > --- a/upload-pack.c > +++ b/upload-pack.c > @@ -1416,8 +1416,8 @@ int upload_pack_v2(struct repository *r, struct argv_array *keys, > create_pack_file(); > state = FETCH_DONE; > break; > - case FETCH_DONE: > - continue; > + default: > + BUG("Added a new fetch_state without updating switch"); > } > } > > -- > 2.16.2.804.g6dcf76e118 > -- Brandon Williams
