[PATCH 4.19 regression fix 1/2] staging: vboxvideo: Fix IRQs no longer working

[Dan Carpenter]

On Tue, Sep 11, 2018 at 07:41:10AM +0000, Nicholas Mc Guire wrote:
> On Tue, Sep 11, 2018 at 08:53:35AM +0200, Hans de Goede wrote:
> > Hi,
> > 
> > On 11-09-18 08:48, 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.
> > 
> > The use of "if (ret)" checks for functions which return 0 on success
> > negative value on error is a standard pattern in the kernel, so I would
> > prefer to keep this as is.
> >
> 
> Well as noted it does the right thing - just checking the use of 
> pci_enable_device() in the existing drivers it seems that it is roughly
> balanced between checking < 0 and !0. The rational for < 0 would be that
> the negative return values mandate a signed type, whilc !0 does not and
> if someone then uses and unsigned type the error case would return as
> success while the < 0 would be detected at compile time (or other static
> code checkers). 

If the function returns int but ret is an unsigned int and we do
"if (ret)", then yes we don't print a static checker warning.  But the
code works perfectly so it doesn't matter.

I'm going to publish some code soon which will complain if a function
returns specific negatives and you save it to an unsigned type which is
smaller than an int.

regards,
dan carpenter