Messages in this thread |  | | Subject | Re: [PATCH 001/141] afs: Fix fall-through warnings for Clang | From | Joe Perches <> | Date | Mon, 23 Nov 2020 08:51:14 -0800 |
| |
On Mon, 2020-11-23 at 16:10 +0000, David Howells wrote: > Joe Perches <joe@perches.com> wrote: > > > > call->unmarshall++; > > > + > > > + fallthrough; > > > > My preference would be to change these to break and not fallthrough; > > > > > case 5: > > > break; > > > } > > My preference would be to fall through. The case number is the state machine > state, as indexed by call->unmarshall.
Then ideally the state machine states should be enums and not numbers and the compiler should use a default block for unhandled states right?
Is code like call->marshall++ a common style for kernel state machines? Perhaps not.
Does it work? Sure.
Is it obvious what the transitions are? No.
> All the other cases in the switch fall through. > > I would also generally prefer that the fallthrough statement occur before the > blank line, not after it, since it belongs with the previous clause, and not > between a comment about a case statement and its associated case statement, > i.e.: > > afs_extract_to_tmp(call); > call->unmarshall++; > > /* extract the callback array and its count in two steps */ > fallthrough; > case 3: > > would be better written as: > > afs_extract_to_tmp(call); > call->unmarshall++; > fallthrough; > > /* extract the callback array and its count in two steps */ > case 3:
I agree completely.
|  |