On Tue, Sep 11, 2018 at 06:48:27AM +0000, Nicholas Mc Guire wrote:
> On Mon, Sep 10, 2018 at 08:30:38PM +0200, Hans de Goede wrote:
> > Commit 1daddbc8dec5 ("staging: vboxvideo: Update driver to use
> > drm_dev_register.") replaced the obsolere drm_get_pci_dev() with
> > normal pci probe and remove functions.
> >
> > But the new vbox_pci_probe() is missing a pci_enable_device() call,
> > causing interrupts to not be delivered. This causes resizes of the
> > vm window to not get seen by the drm/kms code.
> >
> > This commit adds the missing pci_enable_device() call, fixing this.
>
> pci_enable_device is the wrapper to pci_enable_device_flags() the later
> return < 0 on error - so while the check for if(ret) will do the right
> think here I think it would be prefereable to explicidly use if (ret < 0)
> as all error values pci_enable_device_flags() returns are negative.
>
It could go either way, I think. "if (ret) " is sort of as explicit as
"if (ret < 0) " when you consider the false side. When I see "if (ret)"
then I know the code returns negative error codes and zero, but when I
see "if (ret < 0)" then I think maybe this returns positive non-zero
values as well.
As a static analysis person, the "if (ret)" style is easier for me.
Sometimes Smatch doesn't know what a function returns. Maybe the error
code comes from a different thread and Smatch doesn't understand
threads. So then when people use "if (ret)" Smatch knows that non-zero
means *param1 is not initialized. Then the caller does "if (ret < 0)"
that means that positive non-zero values are not handled so let's print
an uninitialized variable warning. Just to spell it out a little more,
the error code won't be printed for "if (ret)" because negatives are a
subset of non-zero.
Of course, if you do it consistently there won't be a warning message.
I never see the consistent subsystems, so I don't know if they exist.
regards,
dan carpenter
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
