Christian Couder <christian.couder@xxxxxxxxx> writes:
>> + default:
>> + BUG("Unhandled rebase type %d", opts->type);
>> + break;
>
> Nit: I think the "break;" line could be removed as the BUG() should always exit.
>
> A quick grep shows that there are other places where there is a
> "break;" line after a BUG() though. Maybe one of the #leftoverbits
> could be about removing those "break;" lines.
I do not see the above as nit, though. "could" is often quite a
different thing from "should", and in this case (and all the other
cases you incorrectly mark with %leftoverbits) removal of these
lines does not improve readability. In fact, if there were another
case arm after this one, having "break" there would help those who
scan the code, as the person who wants to ensure what that next case
arm does is correct is given a strong hint by "break;" immediately
before it that nothing in the previous case arm matters and does not
have to even be looked at. By removing it you force such a reader
to notice BUG() and reason that it does not matter because it does
not return control to us (hence no fallthru).
