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

[Tobin C. Harding]

On Thu, Mar 16, 2017 at 03:32:50PM +0900, Greg Kroah-Hartman wrote:
> 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 at tobin.cc>
> > > > ---
> > > >  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.

Awesome, thanks for clarifying.

Tobin.