On Wed, 8 Jul 2015, David Binderman wrote: > Hello there, > > ---------------------------------------- > > An incorrect definition of CCR_PM_USBPW3 in ohci-tmio.c is a perennial > > source of invalid diagnoses from static scanners, such as in > > <http://marc.info/?l=linux-usb&m=143634574527641&w=2>. This patch > > fixes the definition. > > Sure the patch changes the value of one of the macros used in the switch statement, > but the code still has fallthrough from one case to another. That's > what the static analyser is complaining about, not the values of the macros. Are you certain of that? The analyzer output you posted says "Redundant bitwise operation on 'pm' in 'switch' statement" -- that "redundant" seems to refer to the fact that the macro values are the same. > I''m not sure if this fallthrough is intentional or not. It is. It's a not-uncommon idiom. > If it is, it might be better etiquette to have a comment nearby with the word "fallthrough" in it, > to give the rest of us a clue as to the code's intention. > > Something like > ������ case 3: > ����������� pm |= CCR_PM_USBPW3; > /* fallthrough */ > ������ case 2: > ����������� pm |= CCR_PM_USBPW2; > /* fallthrough */ > ������ case 1: > ����������� pm |= CCR_PM_USBPW1; > /* fallthrough */ I'm open to a patch for this, but it doesn't seem necessary. Have you tried running the patched code through the static analyzer? > On the other hand, if someone has blundered, and so the fallthrough is unintended, > then I'd expect to see code something like this: > > ������ case 3: > ����������� > ���������������� pm |= CCR_PM_USBPW3; > ���������������� break; > ������ case 2: > ����������� > ���������������� pm |= CCR_PM_USBPW2; > � � � � � � � �� break; ������� > ������ case 1: > ����������� > ���������������� pm |= CCR_PM_USBPW1; > ���������������� break; > > Just an idea. > > Regards > > David Binderman Perhaps a better solution would be like this: case 1: pm |= CCR_PM_USBPW1; break; case 2: pm |= CCR_PM_USBPW1 | CCR_PM_USBPW2; break; case 3: pm |= CCR_PM_USBPW1 | CCR_PM_USBPW2 | CCR_PM_USBPW3; break; This is okay with only three cases, but it starts to get unwieldy. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
