On Mon, 2020-11-23 at 16:10 +0000, David Howells wrote: > Joe Perches <j...@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.
