Annotating fall through case statements in a switch
Accidental fall throughs in a switch statement can lead to some nasty bugs. I have used the following banner for quite a long time to indicate that the fall through is intentional and not an oversite (this banner is also a part of the KMDF coding guidelines). It has definitely helped me debug coding mistakes over the years.
case IRP_MJ_DEVICE_CONTROL:
case IRP_MJ_INTERNAL_DEVICE_CONTROL:
// || || Fall through || ||
// \/ \/ \/ \/
default:
return NULL;
}
While the banner is certainly human readable, it is not apparent to static analysis tools that the fall through was intentional. Today I discovered that there is a SAL annotation, __fallthrough, which can indicate the intentional fall through. This means you can annotate the code and tools like PreFAST and PreFAST for Drivers can better understand and analyze the code. I have not yet decided if I want to use the annotation by itself without the current banner or addition to my current banner. I am leaning towards just using the annotation though.
Comments
Anonymous
October 25, 2006
There is also the "old style" Unix annotation, just a /* FALL THROUGH */, which supportedly is recognized by lint, too. Why is the wheel re-invented again?Anonymous
October 25, 2006
Because everything that is old is new again, and besides, I like my ASCII art arrows ;P. Seriously though, PreFAST sees the code and the annotations, it does not even see comments, so something new must be used so the the static analysis tools can see the intent. dAnonymous
October 25, 2006
Admittedly, the ASCII art looks very good. ;) Anyhow, this means, in portable code, one might be forced to use three annotations (__fallthrough, ASCII art and /* FALL THROUGH */) if one writes for more than one platform. In fact, other platforms might decide to use even another form for this, adding to the confusion. What a pity.Anonymous
October 26, 2006
You're complaining about portable static analysis annotations. I'd say that's asking quite a lot. Anyway, you could probably get by with some clever #defines for the non-comment stuff without breaking things. PREfast and SAL are so far beyond what UNIX lint can do these days (as much as I love pc-lint) that I don't think it makes sense to write them off as non-portable.Anonymous
October 29, 2006
Steve, I like the tools, too. Additionally, I did not want to tell that the Windows tool should stay at the same level as lint was. This never was my intention. Anyway, having code (parts) which have to work with Unix based systems AND Windows, it is a pity that there are incompatibilities even in such "low-level" things. Especially since it "feels" as if it would not have been too much work to make it compatible somehow. Regards the #define approach, I don't know for sure how the tools feel about it, if this approach would work. If it does, this would be cool.Anonymous
January 03, 2007
I just stumbled across the __reserved (as in reserved for future or system use) annotation today. Unlike