mupen64plus / mupen64plus-core

Core module of the Mupen64Plus project
1.32k stars 258 forks source link

Fallthrough in switches of interpreter code #91

Open ghost opened 9 years ago

ghost commented 9 years ago

There are three defects in the coverity report which are about switch "case" without break and also no /* fall through */ comment.

There are 38098, 38099 and 38100.Does anyone know if the break is actually missing or just the comment is missing to point out that this is intentionally?

Nebuleon commented 9 years ago

Without further information, I assume the report refers to things like lines 304..309 of pure_interp.c as of commit 16b2793, where multiple case statements use the NI (Not Implemented) interpreter function. Those are intentional, and there's not even any code between the case statements that could confuse the reader.

If there are others with a legitimately missing break; statement, please provide line numbers or a new commit like in #92. :)

ghost commented 9 years ago

What information is missing? The line number can be found in the coverity reports I mentioned. I don't have more informations than that. Here is is a copy of the line number from the reports:

38098: interpreter_cop0.def 30 38099: interpreter_cop0.def 28 38100: cic.c 54 (the default part of the switch)

Nebuleon commented 9 years ago

I can view nothing, including the mapping between Coverity's error numbers and line numbers. To view them, I presume I have to join Coverity, and then be added to the project. Thus, the opening message of the bug report has no context for pretty much anyone other than yourself and whoever has already joined Coverity and been added to the project. I've received 13 rapid-fire invitation emails, but none of the emails had any information about the source of the invitation, so I marked them all as spam and trashed them thinking Coverity's email systems were broken - I guess one of them is for this, though. Try looking here, while signed out, and see if you can find anything that I don't: https://scan.coverity.com/projects/4381

Anyway, the new information refers to lines 28..30 of src/r4300/interpreter_cop0.def and line 54 of src/si/cic.c as of commit f648835.

interpreter_cop0.def:

cic.c: According to the message in the call to DebugMessage, the next value (1 = CIC_X102) is used if the CIC checksum is unrecognised. To human readers, the call indicates that the case below will be used and that a fall-through is expected, but not to this static analyser.