On Mon, Jan 12, 2015 at 12:51:49PM +0100, Marc Kleine-Budde wrote:
> On 01/08/2015 04:19 PM, Ahmed S. Darwish wrote:
>
> [...]
>
> >>> MODULE_DEVICE_TABLE(usb, kvaser_usb_table);
> >>> @@ -463,7 +631,18 @@ static int kvaser_usb_get_software_info(struct
> >>> kvaser_usb *dev)
> >>> if (err)
> >>> return err;
> >>>
> >>> - dev->fw_version = le32_to_cpu(msg.u.softinfo.fw_version);
> >>> + switch (dev->family) {
> >>> + case KVASER_LEAF:
> >>> + dev->fw_version = le32_to_cpu(msg.u.leaf.softinfo.fw_version);
> >>> + break;
> >>> + case KVASER_USBCAN:
> >>> + dev->fw_version = le32_to_cpu(msg.u.usbcan.softinfo.fw_version);
> >>> + break;
> >>> + default:
> >>> + dev_err(dev->udev->dev.parent,
> >>> + "Invalid device family (%d)\n", dev->family);
> >>> + return -EINVAL;
> >>
> >> The default case should not happen. I think you can remove it.
>
> > It's true, it _should_ never happen. But I only add such checks if
> > the follow-up code critically depends on a certain `dev->family`
> > behavior. So it's kind of a defensive check against any possible
> > bug in driver or memory.
> >
> > What do you think?
>
> The kernel is full of callback functions, if you have a bit flip there
> you're in trouble anyways. A bug in the driver (or other parts of the
> kernel) might overwrite the memory of dev->family, but if this happens,
> more things will break.
>
I see. Thanks for the explanation.
Most of them are now removed in latest submission, except two to
avoid GCC warnings of variables that "may be used uninitialized".
Regards,
Darwish
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
