Re: [PATCH v4 01/15] staging: gpib: Modify gpib_register_driver() to return error if it fails

[Nihar Chaithanya <niharchaithanya@xxxxxxxxx>]

On 27/12/24 02:11, Shuah Khan wrote:
> On 12/26/24 12:36, Nihar Chaithanya wrote:
> > The function gpib_register_driver() can fail if kmalloc() fails,
> > but it doesn't return any error if that happens.
> >
> > Modify the function to return error i.e int. Return the appropriate
> > error code if it fails.
> >
> > Signed-off-by: Nihar Chaithanya <niharchaithanya@xxxxxxxxx>
> > ---
> >   drivers/staging/gpib/common/gpib_os.c | 7 ++++---
> >   drivers/staging/gpib/include/gpibP.h  | 2 +-
> >   2 files changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/staging/gpib/common/gpib_os.c 
> > b/drivers/staging/gpib/common/gpib_os.c
> > index 405237d8cb47..07795df3b721 100644
> > --- a/drivers/staging/gpib/common/gpib_os.c
> > +++ b/drivers/staging/gpib/common/gpib_os.c
> > @@ -2094,18 +2094,19 @@ void init_gpib_descriptor(gpib_descriptor_t 
> > *desc)
> >       atomic_set(&desc->io_in_progress, 0);
> >   }
> >   -void gpib_register_driver(gpib_interface_t *interface, struct 
> > module *provider_module)
> > +int gpib_register_driver(gpib_interface_t *interface, struct module 
> > *provider_module)
> >   {
> >       struct gpib_interface_list_struct *entry;
> >         entry = kmalloc(sizeof(*entry), GFP_KERNEL);
> >       if (!entry)
> > -        return;
> > +        return -ENOMEM;
> >         entry->interface = interface;
> >       entry->module = provider_module;
> >       list_add(&entry->list, &registered_drivers);
> > -    pr_info("gpib: registered %s interface\n", interface->name);
>
> Did you mean to delete this message? - looks like an useful message.
> Could you make this dev_info() instead?

Greg had informed that gpib_register_driver() should not be calling 
pr_info(),
I'll add the dev_* debug statements instead of the pr_* statements wherever
the driver registration is completed or fails in this patch series and send
the next version.

> > +
> > +    return 0;
> >   }
> >   EXPORT_SYMBOL(gpib_register_driver);
> >   diff --git a/drivers/staging/gpib/include/gpibP.h 
> > b/drivers/staging/gpib/include/gpibP.h
> > index 5fc42b645ab7..d0cd42c1a0ad 100644
> > --- a/drivers/staging/gpib/include/gpibP.h
> > +++ b/drivers/staging/gpib/include/gpibP.h
> > @@ -17,7 +17,7 @@
> >   #include <linux/fs.h>
> >   #include <linux/interrupt.h>
> >   -void gpib_register_driver(gpib_interface_t *interface, struct 
> > module *mod);
> > +int gpib_register_driver(gpib_interface_t *interface, struct module 
> > *mod);
> >   void gpib_unregister_driver(gpib_interface_t *interface);
> >   struct pci_dev *gpib_pci_get_device(const gpib_board_config_t 
> > *config, unsigned int vendor_id,
> >                       unsigned int device_id, struct pci_dev *from);
>
> thanks,
> -- Shuah

Thanks,
Nihar