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
