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

[Nihar Chaithanya <niharchaithanya@gmail.com>]

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@gmail.com>
>> ---
>>   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