[v4] staging: gpib: Handle gpib_register_driver() errors

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

Posted by Nihar Chaithanya 1 year, 1 month ago

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);
+
+	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);
-- 
2.34.1

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

Posted by Shuah Khan 1 year, 1 month ago

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?

> +
> +	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

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

Posted by Nihar Chaithanya 1 year, 1 month ago

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