Messages in this thread | | | Subject | Re: [PATCH] staging: comedi: comedi_test: return error when comedi_test_init fails | From | Ian Abbott <> | Date | Tue, 10 Dec 2019 10:58:40 +0000 |
| |
On 10/12/2019 09:14, Chuhong Yuan wrote: > comedi_test_init() forgets to return error when it fails. > Modify the return value to fix it. > > Fixes: 9ff7400bd38c ("Staging: comedi: drivers: comedi_test: Add auto-configuration capability") > Signed-off-by: Chuhong Yuan <hslester96@gmail.com> > --- > drivers/staging/comedi/drivers/comedi_test.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/comedi/drivers/comedi_test.c b/drivers/staging/comedi/drivers/comedi_test.c > index ef4c7c8a2b71..133ed99a89f1 100644 > --- a/drivers/staging/comedi/drivers/comedi_test.c > +++ b/drivers/staging/comedi/drivers/comedi_test.c > @@ -799,12 +799,14 @@ static int __init comedi_test_init(void) > if (!config_mode) { > ctcls = class_create(THIS_MODULE, CLASS_NAME); > if (IS_ERR(ctcls)) { > + ret = PTR_ERR(ctcls); > pr_warn("comedi_test: unable to create class\n"); > goto clean3; > } > > ctdev = device_create(ctcls, NULL, MKDEV(0, 0), NULL, DEV_NAME); > if (IS_ERR(ctdev)) { > + ret = PTR_ERR(ctdev); > pr_warn("comedi_test: unable to create device\n"); > goto clean2; > } > @@ -826,7 +828,7 @@ static int __init comedi_test_init(void) > clean3: > ctcls = NULL; > > - return 0; > + return ret; > } > module_init(comedi_test_init); > >
I suspect the current behaviour was by design because the module is not completely broken if it fails to create the "comedi_test" device. The module will just behave as if the "noauto" module parameter (which sets the 'config_mode' variable) was set to "true" if there is an error during class or device creation. That's probably why the kernel log messages are written with 'pr_warn()' rather than 'pr_err()'.
As long as the core "comedi" module has been loaded with the "comedi_num_legacy_minors" module parameter set to a non-zero value, it would still be possible to configure one of the "legacy" comedi devices to use the "comedi_test" driver via the COMEDI_DEVCONFIG ioctl.
However, returning an error might be preferable here, especially since the comedi core module's "comedi_num_legacy_minors" parameter defaults to zero.
I'm happy with the change of behaviour, but would suggest changing the 'pr_warn()' calls to 'pr_err()' to match this change.
-- -=( Ian Abbott <abbotti@mev.co.uk> || Web: www.mev.co.uk )=- -=( MEV Ltd. is a company registered in England & Wales. )=- -=( Registered number: 02862268. Registered address: )=- -=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
| |