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=p...@mail.gmail.com/
> 2. https://public-inbox.org/git/20170513231509.7834-19-ava...@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 :)
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <ava...@gmail.com>
> ---
> 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
