Re: [PATCH 1/2] staging: dgnc: make error codes uniform

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Mar 16, 2017 at 03:46:58PM +1100, Tobin C. Harding wrote:
> On Thu, Mar 16, 2017 at 11:45:17AM +0900, Greg Kroah-Hartman wrote:
> > On Wed, Mar 15, 2017 at 10:44:28AM +1100, Tobin C. Harding wrote:
> > > Driver code is non-uniform in its use of error return codes, identical
> > > failures are returning different error codes. Return is on failure
> > > when checking struct magic numbers. Error codes used include -ENODEV,
> > > -ENXIO, -EIO, and -EFAULT.
> > > 
> > > Use -ENXIO. Justification is that usual call includes a check that the
> > > struct is non-NULL OR'd with check on the magic number.
> > > 
> > > #define ENXIO 6 /* No such device or address */
> > > 
> > > Seems like a good fit. Also this choice results in the minimum number
> > > of files touched.
> > > 
> > > Pick one error code, ENXIO. Change all functions to use the same error
> > > return code for the cases of failed magic number checks.
> > > 
> > > Signed-off-by: Tobin C. Harding <me@xxxxxxxx>
> > > ---
> > >  drivers/staging/dgnc/dgnc_mgmt.c |  2 +-
> > >  drivers/staging/dgnc/dgnc_tty.c  | 44 ++++++++++++++++++++--------------------
> > >  2 files changed, 23 insertions(+), 23 deletions(-)
> > > 
> > > diff --git a/drivers/staging/dgnc/dgnc_mgmt.c b/drivers/staging/dgnc/dgnc_mgmt.c
> > > index ee8a626..f134222 100644
> > > --- a/drivers/staging/dgnc/dgnc_mgmt.c
> > > +++ b/drivers/staging/dgnc/dgnc_mgmt.c
> > > @@ -187,7 +187,7 @@ long dgnc_mgmt_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> > >  		ch = dgnc_board[board]->channels[channel];
> > >  
> > >  		if (!ch || ch->magic != DGNC_CHANNEL_MAGIC)
> > > -			return -ENODEV;
> > > +			return -ENXIO;
> > 
> > No, this really means the device is gone.  And the whole ->magic stuff
> > needs to just be deleted, it's not needed at all.
> 
> Can I please confirm, all these magic number checks are useless?
> 
>  Module: drivers/staging/dgnc
> 
>         if (!tty || tty->magic != TTY_MAGIC)
> 

Checking for tty is fine, right?

>         un = tty->driver_data;
>         if (!un || un->magic != DGNC_UNIT_MAGIC)

Any "magic" check should be removed, it's a _very_ old pattern (before
Linux 1.0) that somehow was done because the developer was worried about
memory corruption.  It's not an issue anymore.

thanks,

greg k-h
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux