[PATCH v5 00/15] staging: gpib: Handle gpib_register_driver() errors

[PATCH v5 00/15] staging: gpib: Handle gpib_register_driver() errors

[Nihar Chaithanya]

While reviewing the patch for fixing Coverity scan issue from
gpib: cb7210 [1], Greg pointed out that the gpib_register_driver()
function can fail and must return an error value if it fails.

This patch series fixes the above issue by changing the return type
of gpib_register_driver() function and returning the error value if
it fails.

This fix also makes sure that wherever the gpib_register_driver()
is called and if it fails - the previous driver register functions
are unregistered(if they exist) and the error value is returned.

Thanks,
Nihar

v4 --> v5 : Added pr_* statements indicating when the driver register
            functions fail or if the module is initialized without errors.

v3 --> v4 : Made the v3 patch into series and also fixed an error in
            v3 [2]. Each patch in this series is a fix for particular
            driver handling the gpib_register_driver() error.

v2 --> v3 : Modified the gpib_register_driver() to return error code,
            added error handling to all the places that the
            gpib_register_driver() is called.

v1 --> v2 : Replaced the redundant cb_pcmcia_init_module() with
            pcmcia_register_driver().

Link to v4 : https://lore.kernel.org/all/20241226193637.241049-16-niharchaithanya@gmail.com/
Link to v3 : https://lore.kernel.org/all/20241221214822.163667-1-niharchaithanya@gmail.com/
Link to v2 : https://lore.kernel.org/all/20241221091121.35476-1-niharchaithanya@gmail.com/
Link to v1 : https://lore.kernel.org/all/20241221022632.20931-1-niharchaithanya@gmail.com/

[1] https://lore.kernel.org/all/2024122133-outskirts-causing-099a@gregkh/
[2] https://lore.kernel.org/all/202412221424.WIjKFIPf-lkp@intel.com/

Nihar Chaithanya (15):
  staging: gpib: Modify gpib_register_driver() to return error if it
    fails
  staging: gpib: agilent_82350b: Handle gpib_register_driver() errors
  staging: gpib: agilent_82357a: Handle gpib_register_driver() errors
  staging: gpib: cb7210: Handle gpib_register_driver() errors
  staging: gpib: cec: Handle gpib_register_driver() errors
  staging: gpib: fluke: Handle gpib_register_driver() errors
  staging: gpib: fmh: Handle gpib_register_driver() errors
  staging: gpib: gpio: Return error value from gpib_register_driver()
  staging: gpib: hp_82335: Return error value from
    gpib_register_driver()
  staging: gpib: hp_82341: Handle gpib_register_driver() errors
  staging: gpib: ines: Handle gpib_register_driver() errors
  staging: gpib: lpvo_usb: Return error value from
    gpib_register_driver()
  staging: gpib: ni_usb: Handle gpib_register_driver() errors
  staging: gpib: pc2: Handle gpib_register_driver() errors
  staging: gpib: tnt4882: Handle gpib_register_driver() errors

 .../gpib/agilent_82350b/agilent_82350b.c      | 21 +++-
 .../gpib/agilent_82357a/agilent_82357a.c      | 18 +++-
 drivers/staging/gpib/cb7210/cb7210.c          | 99 ++++++++++++++-----
 drivers/staging/gpib/cec/cec_gpib.c           |  9 +-
 drivers/staging/gpib/common/gpib_os.c         |  7 +-
 drivers/staging/gpib/eastwood/fluke_gpib.c    | 28 +++++-
 drivers/staging/gpib/fmh_gpib/fmh_gpib.c      | 41 ++++++--
 drivers/staging/gpib/gpio/gpib_bitbang.c      |  7 +-
 drivers/staging/gpib/hp_82335/hp82335.c       |  9 +-
 drivers/staging/gpib/hp_82341/hp_82341.c      | 18 +++-
 drivers/staging/gpib/include/gpibP.h          |  2 +-
 drivers/staging/gpib/ines/ines_gpib.c         | 81 +++++++++++----
 .../gpib/lpvo_usb_gpib/lpvo_usb_gpib.c        |  6 +-
 drivers/staging/gpib/ni_usb/ni_usb_gpib.c     | 17 +++-
 drivers/staging/gpib/pc2/pc2_gpib.c           | 34 ++++++-
 drivers/staging/gpib/tnt4882/tnt4882_gpib.c   | 87 +++++++++++++---
 16 files changed, 387 insertions(+), 97 deletions(-)

-- 
2.34.1

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

[Nihar Chaithanya]

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 v5 01/15] staging: gpib: Modify gpib_register_driver() to return error if it fails

[Greg KH]

On Sat, Dec 28, 2024 at 11:37:49AM +0530, 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.

You forgot to mention that you removed the pr_info() line.

thanks,

greg k-h

[PATCH v5 02/15] staging: gpib: agilent_82350b: Handle gpib_register_driver() errors

[Nihar Chaithanya]

The function gpib_register_driver() can fail which can result in
semi-registered module.

In case gpib_register_driver() fails unregister the previous
gpib and pci registering functions, return the error value.

Signed-off-by: Nihar Chaithanya <niharchaithanya@gmail.com>
---
 .../gpib/agilent_82350b/agilent_82350b.c      | 21 ++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/gpib/agilent_82350b/agilent_82350b.c b/drivers/staging/gpib/agilent_82350b/agilent_82350b.c
index 53006d0cc79c..6f077ede86a6 100644
--- a/drivers/staging/gpib/agilent_82350b/agilent_82350b.c
+++ b/drivers/staging/gpib/agilent_82350b/agilent_82350b.c
@@ -910,13 +910,28 @@ static int __init agilent_82350b_init_module(void)
 
 	result = pci_register_driver(&agilent_82350b_pci_driver);
 	if (result) {
-		pr_err("agilent_82350b: pci_driver_register failed!\n");
+		pr_err("agilent_82350b: pci_driver_register failed: error = %d\n", result);
 		return result;
 	}
 
-	gpib_register_driver(&agilent_82350b_unaccel_interface, THIS_MODULE);
-	gpib_register_driver(&agilent_82350b_interface, THIS_MODULE);
+	result = gpib_register_driver(&agilent_82350b_unaccel_interface, THIS_MODULE);
+	if (result)
+		goto err_unaccel;
+
+	result = gpib_register_driver(&agilent_82350b_interface, THIS_MODULE);
+	if (result)
+		goto err_interface;
+
+	pr_info("agilent_82350b: module init is complete\n");
 	return 0;
+
+err_interface:
+	gpib_unregister_driver(&agilent_82350b_unaccel_interface);
+err_unaccel:
+	pci_unregister_driver(&agilent_82350b_pci_driver);
+
+	pr_err("agilent_82350b: gpib_register_driver failed!\n");
+	return result;
 }
 
 static void __exit agilent_82350b_exit_module(void)
-- 
2.34.1

Re: [PATCH v5 02/15] staging: gpib: agilent_82350b: Handle gpib_register_driver() errors

[Greg KH]

On Sat, Dec 28, 2024 at 11:37:50AM +0530, Nihar Chaithanya wrote:
> The function gpib_register_driver() can fail which can result in
> semi-registered module.
> 
> In case gpib_register_driver() fails unregister the previous
> gpib and pci registering functions, return the error value.
> 
> Signed-off-by: Nihar Chaithanya <niharchaithanya@gmail.com>
> ---
>  .../gpib/agilent_82350b/agilent_82350b.c      | 21 ++++++++++++++++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/gpib/agilent_82350b/agilent_82350b.c b/drivers/staging/gpib/agilent_82350b/agilent_82350b.c
> index 53006d0cc79c..6f077ede86a6 100644
> --- a/drivers/staging/gpib/agilent_82350b/agilent_82350b.c
> +++ b/drivers/staging/gpib/agilent_82350b/agilent_82350b.c
> @@ -910,13 +910,28 @@ static int __init agilent_82350b_init_module(void)
>  
>  	result = pci_register_driver(&agilent_82350b_pci_driver);
>  	if (result) {
> -		pr_err("agilent_82350b: pci_driver_register failed!\n");
> +		pr_err("agilent_82350b: pci_driver_register failed: error = %d\n", result);
>  		return result;
>  	}
>  
> -	gpib_register_driver(&agilent_82350b_unaccel_interface, THIS_MODULE);
> -	gpib_register_driver(&agilent_82350b_interface, THIS_MODULE);
> +	result = gpib_register_driver(&agilent_82350b_unaccel_interface, THIS_MODULE);
> +	if (result)
> +		goto err_unaccel;
> +
> +	result = gpib_register_driver(&agilent_82350b_interface, THIS_MODULE);
> +	if (result)
> +		goto err_interface;
> +
> +	pr_info("agilent_82350b: module init is complete\n");

Why are you adding noise to drivers like this?  When drivers work
properly, they are quiet.


>  	return 0;
> +
> +err_interface:
> +	gpib_unregister_driver(&agilent_82350b_unaccel_interface);
> +err_unaccel:
> +	pci_unregister_driver(&agilent_82350b_pci_driver);
> +
> +	pr_err("agilent_82350b: gpib_register_driver failed!\n");

No error number here?

Same for all the other changes in this series.

thanks,

greg k-h

[PATCH v5 03/15] staging: gpib: agilent_82357a: Handle gpib_register_driver() errors

[Nihar Chaithanya]

The usb_register() function can fail and returns an error value which
is not returned. The function gpib_register_driver() can also fail
which can result in semi-registered module.

In case gpib_register_driver() fails unregister the previous usb driver
registering function. Return the error value if gpib_register_driver()
or usb_register() functions fail.

Signed-off-by: Nihar Chaithanya <niharchaithanya@gmail.com>
---
 .../gpib/agilent_82357a/agilent_82357a.c       | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/gpib/agilent_82357a/agilent_82357a.c b/drivers/staging/gpib/agilent_82357a/agilent_82357a.c
index bf05fb4a736b..4bdf6ce54622 100644
--- a/drivers/staging/gpib/agilent_82357a/agilent_82357a.c
+++ b/drivers/staging/gpib/agilent_82357a/agilent_82357a.c
@@ -1691,13 +1691,25 @@ static struct usb_driver agilent_82357a_bus_driver = {
 static int __init agilent_82357a_init_module(void)
 {
 	int i;
+	int ret;
 
-	pr_info("agilent_82357a_gpib driver loading");
 	for (i = 0; i < MAX_NUM_82357A_INTERFACES; ++i)
 		agilent_82357a_driver_interfaces[i] = NULL;
-	usb_register(&agilent_82357a_bus_driver);
-	gpib_register_driver(&agilent_82357a_gpib_interface, THIS_MODULE);
 
+	ret = usb_register(&agilent_82357a_bus_driver);
+	if (ret) {
+		pr_err("agilent_82357a: usb_register failed: error = %d\n", ret);
+		return ret;
+	}
+
+	ret = gpib_register_driver(&agilent_82357a_gpib_interface, THIS_MODULE);
+	if (ret) {
+		pr_err("agilent_82357a: gpib_register_driver failed!\n");
+		usb_deregister(&agilent_82357a_bus_driver);
+		return ret;
+	}
+
+	pr_info("agilent_82357a: module init is complete\n");
 	return 0;
 }
 
-- 
2.34.1

[PATCH v5 04/15] staging: gpib: cb7210: Handle gpib_register_driver() errors

[Nihar Chaithanya]

The function cb_pcmcia_init_module() can be replaced by calling
pcmcia_register_driver() directly. The error value from this
function is not returned and the previous registering functions
are not unregistered.

The function gpib_register_driver() can fail and similar to
pcmcia_register_driver() function failing, the previous registering
functions are not unregistered.

Replace cb_pcmcia_init_module() with pcmcia_register_driver().
Unregister the gpib and pci register functions if the subsequent
gpib or pcmcia register functions fail and return the error value.

Signed-off-by: Nihar Chaithanya <niharchaithanya@gmail.com>
---
 drivers/staging/gpib/cb7210/cb7210.c | 99 ++++++++++++++++++++--------
 1 file changed, 73 insertions(+), 26 deletions(-)

diff --git a/drivers/staging/gpib/cb7210/cb7210.c b/drivers/staging/gpib/cb7210/cb7210.c
index 63df7f3eb3f3..99041ee007bb 100644
--- a/drivers/staging/gpib/cb7210/cb7210.c
+++ b/drivers/staging/gpib/cb7210/cb7210.c
@@ -1351,12 +1351,6 @@ static struct pcmcia_driver cb_gpib_cs_driver = {
 	.resume		= cb_gpib_resume,
 };
 
-int cb_pcmcia_init_module(void)
-{
-	pcmcia_register_driver(&cb_gpib_cs_driver);
-	return 0;
-}
-
 void cb_pcmcia_cleanup_module(void)
 {
 	DEBUG(0, "cb_gpib_cs: unloading\n");
@@ -1506,32 +1500,85 @@ void cb_pcmcia_detach(gpib_board_t *board)
 
 static int __init cb7210_init_module(void)
 {
-	int err = 0;
-	int result;
+	int ret;
 
-	result = pci_register_driver(&cb7210_pci_driver);
-	if (result) {
-		pr_err("cb7210: pci_driver_register failed!\n");
-		return result;
+	ret = pci_register_driver(&cb7210_pci_driver);
+	if (ret) {
+		pr_err("cb7210: pci_driver_register failed: error = %d\n", ret);
+		return ret;
 	}
 
-	gpib_register_driver(&cb_pci_interface, THIS_MODULE);
-	gpib_register_driver(&cb_isa_interface, THIS_MODULE);
-	gpib_register_driver(&cb_pci_accel_interface, THIS_MODULE);
-	gpib_register_driver(&cb_pci_unaccel_interface, THIS_MODULE);
-	gpib_register_driver(&cb_isa_accel_interface, THIS_MODULE);
-	gpib_register_driver(&cb_isa_unaccel_interface, THIS_MODULE);
-
-#ifdef GPIB__PCMCIA
-	gpib_register_driver(&cb_pcmcia_interface, THIS_MODULE);
-	gpib_register_driver(&cb_pcmcia_accel_interface, THIS_MODULE);
-	gpib_register_driver(&cb_pcmcia_unaccel_interface, THIS_MODULE);
-	err += cb_pcmcia_init_module();
+	ret = gpib_register_driver(&cb_pci_interface, THIS_MODULE);
+	if (ret)
+		goto err_pci;
+
+	ret = gpib_register_driver(&cb_isa_interface, THIS_MODULE);
+	if (ret)
+		goto err_isa;
+
+	ret = gpib_register_driver(&cb_pci_accel_interface, THIS_MODULE);
+	if (ret)
+		goto err_pci_accel;
+
+	ret = gpib_register_driver(&cb_pci_unaccel_interface, THIS_MODULE);
+	if (ret)
+		goto err_pci_unaccel;
+
+	ret = gpib_register_driver(&cb_isa_accel_interface, THIS_MODULE);
+	if (ret)
+		goto err_isa_accel;
+
+	ret = gpib_register_driver(&cb_isa_unaccel_interface, THIS_MODULE);
+	if (ret)
+		goto err_isa_unaccel;
+
+#ifdef GPIB_PCMCIA
+	ret = gpib_register_driver(&cb_pcmcia_interface, THIS_MODULE);
+	if (ret)
+		goto err_pcmcia;
+
+	ret = gpib_register_driver(&cb_pcmcia_accel_interface, THIS_MODULE);
+	if (ret)
+		goto err_pcmcia_accel;
+
+	ret = gpib_register_driver(&cb_pcmcia_unaccel_interface, THIS_MODULE);
+	if (ret)
+		goto err_pcmcia_unaccel;
+
+	ret = pcmcia_register_driver(&cb_gpib_cs_driver);
+	if (ret)
+		goto err_pcmcia_driver;
 #endif
-	if (err)
-		return -1;
 
+	pr_info("cb7210: module init is complete\n");
 	return 0;
+
+#ifdef GPIB_PCMCIA
+err_pcmcia_driver:
+	pr_err("ines: pcmcia_register_driver failed: error = %d\n", ret);
+	gpib_unregister_driver(&cb_pcmcia_unaccel_interface);
+err_pcmcia_unaccel:
+	gpib_unregister_driver(&cb_pcmcia_accel_interface);
+err_pcmcia_accel:
+	gpib_unregister_driver(&cb_pcmcia_interface);
+err_pcmcia:
+#endif
+	gpib_unregister_driver(&cb_isa_unaccel_interface);
+err_isa_unaccel:
+	gpib_unregister_driver(&cb_isa_accel_interface);
+err_isa_accel:
+	gpib_unregister_driver(&cb_pci_unaccel_interface);
+err_pci_unaccel:
+	gpib_unregister_driver(&cb_pci_accel_interface);
+err_pci_accel:
+	gpib_unregister_driver(&cb_isa_interface);
+err_isa:
+	gpib_unregister_driver(&cb_pci_interface);
+err_pci:
+	pci_unregister_driver(&cb7210_pci_driver);
+
+	pr_err("cb7210: gpib_register_driver failed!\n");
+	return ret;
 }
 
 static void __exit cb7210_exit_module(void)
-- 
2.34.1

[PATCH v5 05/15] staging: gpib: cec: Handle gpib_register_driver() errors

[Nihar Chaithanya]

The function gpib_register_driver() can fail and result in a
semi-registered module and does not return an error value if it
fails.

Unregister the pci registering function in case gpib_register_driver()
fails and return the error value.

Signed-off-by: Nihar Chaithanya <niharchaithanya@gmail.com>
---
 drivers/staging/gpib/cec/cec_gpib.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/gpib/cec/cec_gpib.c b/drivers/staging/gpib/cec/cec_gpib.c
index 3dc933deb401..5e887c649ddb 100644
--- a/drivers/staging/gpib/cec/cec_gpib.c
+++ b/drivers/staging/gpib/cec/cec_gpib.c
@@ -365,12 +365,17 @@ static int __init cec_init_module(void)
 
 	result = pci_register_driver(&cec_pci_driver);
 	if (result) {
-		pr_err("cec_gpib: pci_driver_register failed!\n");
+		pr_err("cec_gpib: pci_driver_register failed: error = %d\n", result);
 		return result;
 	}
 
-	gpib_register_driver(&cec_pci_interface, THIS_MODULE);
+	result = gpib_register_driver(&cec_pci_interface, THIS_MODULE);
+	if (result) {
+		pr_err("cec_gpib: gpib_register_driver failed!\n");
+		return result;
+	}
 
+	pr_info("cec_gpib: module init is complete\n");
 	return 0;
 }
 
-- 
2.34.1

[PATCH v5 06/15] staging: gpib: fluke: Handle gpib_register_driver() errors

[Nihar Chaithanya]

The function gpib_register_driver() can fail, resulting in a
semi-registered module and does not return an error value if it
fails.

Unregister the previous platform driver and gpib registering
functions if subsequent gpib_register_driver() fail and return the
error value.

Signed-off-by: Nihar Chaithanya <niharchaithanya@gmail.com>
---
 drivers/staging/gpib/eastwood/fluke_gpib.c | 28 ++++++++++++++++++----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/gpib/eastwood/fluke_gpib.c b/drivers/staging/gpib/eastwood/fluke_gpib.c
index 3f938ab0c84d..285dd5e5ebfc 100644
--- a/drivers/staging/gpib/eastwood/fluke_gpib.c
+++ b/drivers/staging/gpib/eastwood/fluke_gpib.c
@@ -1155,16 +1155,34 @@ static int __init fluke_init_module(void)
 
 	result = platform_driver_register(&fluke_gpib_platform_driver);
 	if (result) {
-		pr_err("fluke_gpib: platform_driver_register failed!\n");
+		pr_err("fluke_gpib: platform_driver_register failed: error = %d\n", result);
 		return result;
 	}
 
-	gpib_register_driver(&fluke_unaccel_interface, THIS_MODULE);
-	gpib_register_driver(&fluke_hybrid_interface, THIS_MODULE);
-	gpib_register_driver(&fluke_interface, THIS_MODULE);
+	result = gpib_register_driver(&fluke_unaccel_interface, THIS_MODULE);
+	if (result)
+		goto err_unaccel;
 
-	pr_info("fluke_gpib\n");
+	result = gpib_register_driver(&fluke_hybrid_interface, THIS_MODULE);
+	if (result)
+		goto err_hybrid;
+
+	result = gpib_register_driver(&fluke_interface, THIS_MODULE);
+	if (result)
+		goto err_interface;
+
+	pr_info("fluke_gpib: module init is complete\n");
 	return 0;
+
+err_interface:
+	gpib_unregister_driver(&fluke_hybrid_interface);
+err_hybrid:
+	gpib_unregister_driver(&fluke_unaccel_interface);
+err_unaccel:
+	platform_driver_unregister(&fluke_gpib_platform_driver);
+
+	pr_err("fluke_gpib: gpib_register_driver failed!\n");
+	return result;
 }
 
 static void __exit fluke_exit_module(void)
-- 
2.34.1

[PATCH v5 07/15] staging: gpib: fmh: Handle gpib_register_driver() errors

[Nihar Chaithanya]

The function gpib_register_driver() can fail, resulting in a
semi-registered module and does not return an error value if it
fails.

Unregister the previous platform driver, pci and gpib registering
functions if subsequent gpib_register_driver() fail and return the
error value.

Signed-off-by: Nihar Chaithanya <niharchaithanya@gmail.com>
---
 drivers/staging/gpib/fmh_gpib/fmh_gpib.c | 41 +++++++++++++++++++-----
 1 file changed, 33 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/gpib/fmh_gpib/fmh_gpib.c b/drivers/staging/gpib/fmh_gpib/fmh_gpib.c
index 62791db1c34a..dbade677391b 100644
--- a/drivers/staging/gpib/fmh_gpib/fmh_gpib.c
+++ b/drivers/staging/gpib/fmh_gpib/fmh_gpib.c
@@ -1691,23 +1691,48 @@ static int __init fmh_gpib_init_module(void)
 
 	result = platform_driver_register(&fmh_gpib_platform_driver);
 	if (result) {
-		pr_err("fmh_gpib: platform_driver_register failed!\n");
+		pr_err("fmh_gpib: platform_driver_register failed: error = %d\n", result);
 		return result;
 	}
 
 	result = pci_register_driver(&fmh_gpib_pci_driver);
 	if (result) {
-		pr_err("fmh_gpib: pci_driver_register failed!\n");
-		return result;
+		pr_err("fmh_gpib: pci_register_driver failed: error = %d\n", result);
+		goto err_pci_driver;
 	}
 
-	gpib_register_driver(&fmh_gpib_unaccel_interface, THIS_MODULE);
-	gpib_register_driver(&fmh_gpib_interface, THIS_MODULE);
-	gpib_register_driver(&fmh_gpib_pci_unaccel_interface, THIS_MODULE);
-	gpib_register_driver(&fmh_gpib_pci_interface, THIS_MODULE);
+	result = gpib_register_driver(&fmh_gpib_unaccel_interface, THIS_MODULE);
+	if (result)
+		goto err_unaccel;
+
+	result = gpib_register_driver(&fmh_gpib_interface, THIS_MODULE);
+	if (result)
+		goto err_interface;
+
+	result = gpib_register_driver(&fmh_gpib_pci_unaccel_interface, THIS_MODULE);
+	if (result)
+		goto err_pci_unaccel;
 
-	pr_info("fmh_gpib\n");
+	result = gpib_register_driver(&fmh_gpib_pci_interface, THIS_MODULE);
+	if (result)
+		goto err_pci;
+
+	pr_info("fmh_gpib: module init is complete\n");
 	return 0;
+
+err_pci:
+	gpib_unregister_driver(&fmh_gpib_pci_unaccel_interface);
+err_pci_unaccel:
+	gpib_unregister_driver(&fmh_gpib_interface);
+err_interface:
+	gpib_unregister_driver(&fmh_gpib_unaccel_interface);
+err_unaccel:
+	pci_unregister_driver(&fmh_gpib_pci_driver);
+err_pci_driver:
+	platform_driver_unregister(&fmh_gpib_platform_driver);
+
+	pr_err("fmh_gpib: gpib_register_driver failed\n");
+	return result;
 }
 
 static void __exit fmh_gpib_exit_module(void)
-- 
2.34.1

Re: [PATCH v5 07/15] staging: gpib: fmh: Handle gpib_register_driver() errors

[kernel test robot]

Hi Nihar,

kernel test robot noticed the following build errors:

[auto build test ERROR on staging/staging-testing]

url:    https://github.com/intel-lab-lkp/linux/commits/Nihar-Chaithanya/staging-gpib-Modify-gpib_register_driver-to-return-error-if-it-fails/20241228-142936
base:   staging/staging-testing
patch link:    https://lore.kernel.org/r/20241228060800.107653-8-niharchaithanya%40gmail.com
patch subject: [PATCH v5 07/15] staging: gpib: fmh: Handle gpib_register_driver() errors
config: sparc-randconfig-002-20241228 (https://download.01.org/0day-ci/archive/20241228/202412281702.kUUcfCcI-lkp@intel.com/config)
compiler: sparc-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241228/202412281702.kUUcfCcI-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202412281702.kUUcfCcI-lkp@intel.com/

All errors (new ones prefixed by >>):

   sparc-linux-ld: drivers/staging/gpib/fmh_gpib/fmh_gpib.o: in function `fmh_gpib_t1_delay':
   fmh_gpib.c:(.text+0x280): undefined reference to `nec7210_t1_delay'
   sparc-linux-ld: drivers/staging/gpib/fmh_gpib/fmh_gpib.o: in function `fmh_gpib_serial_poll_status':
   fmh_gpib.c:(.text+0x2ec): undefined reference to `nec7210_serial_poll_status'
   sparc-linux-ld: drivers/staging/gpib/fmh_gpib/fmh_gpib.o: in function `fmh_gpib_secondary_address':
   fmh_gpib.c:(.text+0x3f0): undefined reference to `nec7210_secondary_address'
   sparc-linux-ld: drivers/staging/gpib/fmh_gpib/fmh_gpib.o: in function `fmh_gpib_primary_address':
   fmh_gpib.c:(.text+0x404): undefined reference to `nec7210_primary_address'
   sparc-linux-ld: drivers/staging/gpib/fmh_gpib/fmh_gpib.o: in function `fmh_gpib_update_status':
   fmh_gpib.c:(.text+0x418): undefined reference to `nec7210_update_status'
   sparc-linux-ld: drivers/staging/gpib/fmh_gpib/fmh_gpib.o: in function `fmh_gpib_parallel_poll_response':
   fmh_gpib.c:(.text+0x42c): undefined reference to `nec7210_parallel_poll_response'
   sparc-linux-ld: drivers/staging/gpib/fmh_gpib/fmh_gpib.o: in function `fmh_gpib_parallel_poll':
   fmh_gpib.c:(.text+0x440): undefined reference to `nec7210_parallel_poll'
   sparc-linux-ld: drivers/staging/gpib/fmh_gpib/fmh_gpib.o: in function `fmh_gpib_parallel_poll_configure':
   fmh_gpib.c:(.text+0x454): undefined reference to `nec7210_parallel_poll_configure'
   sparc-linux-ld: drivers/staging/gpib/fmh_gpib/fmh_gpib.o: in function `fmh_gpib_disable_eos':
   fmh_gpib.c:(.text+0x464): undefined reference to `nec7210_disable_eos'
   sparc-linux-ld: drivers/staging/gpib/fmh_gpib/fmh_gpib.o: in function `fmh_gpib_enable_eos':
   fmh_gpib.c:(.text+0x47c): undefined reference to `nec7210_enable_eos'
   sparc-linux-ld: drivers/staging/gpib/fmh_gpib/fmh_gpib.o: in function `fmh_gpib_remote_enable':
   fmh_gpib.c:(.text+0x490): undefined reference to `nec7210_remote_enable'
   sparc-linux-ld: drivers/staging/gpib/fmh_gpib/fmh_gpib.o: in function `fmh_gpib_interface_clear':
   fmh_gpib.c:(.text+0x4a4): undefined reference to `nec7210_interface_clear'
   sparc-linux-ld: drivers/staging/gpib/fmh_gpib/fmh_gpib.o: in function `fmh_gpib_request_system_control':
   fmh_gpib.c:(.text+0x4b8): undefined reference to `nec7210_request_system_control'
   sparc-linux-ld: drivers/staging/gpib/fmh_gpib/fmh_gpib.o: in function `fmh_gpib_go_to_standby':
   fmh_gpib.c:(.text+0x4c8): undefined reference to `nec7210_go_to_standby'
   sparc-linux-ld: drivers/staging/gpib/fmh_gpib/fmh_gpib.o: in function `fmh_gpib_take_control':
   fmh_gpib.c:(.text+0x4dc): undefined reference to `nec7210_take_control'
   sparc-linux-ld: drivers/staging/gpib/fmh_gpib/fmh_gpib.o: in function `fmh_gpib_command':
   fmh_gpib.c:(.text+0x4f8): undefined reference to `nec7210_command'
   sparc-linux-ld: drivers/staging/gpib/fmh_gpib/fmh_gpib.o: in function `fmh_gpib_write':
   fmh_gpib.c:(.text+0x518): undefined reference to `nec7210_write'
   sparc-linux-ld: drivers/staging/gpib/fmh_gpib/fmh_gpib.o: in function `fmh_gpib_read':
   fmh_gpib.c:(.text+0x538): undefined reference to `nec7210_read'
   sparc-linux-ld: drivers/staging/gpib/fmh_gpib/fmh_gpib.o: in function `fmh_gpib_detach':
   fmh_gpib.c:(.text+0x720): undefined reference to `nec7210_board_reset'
   sparc-linux-ld: drivers/staging/gpib/fmh_gpib/fmh_gpib.o: in function `fmh_gpib_internal_interrupt':
   fmh_gpib.c:(.text+0x82c): undefined reference to `push_gpib_event'
   sparc-linux-ld: fmh_gpib.c:(.text+0x840): undefined reference to `nec7210_interrupt_have_status'
   sparc-linux-ld: drivers/staging/gpib/fmh_gpib/fmh_gpib.o: in function `fmh_gpib_device_match':
   fmh_gpib.c:(.text+0xae4): undefined reference to `gpib_match_device_path'
   sparc-linux-ld: drivers/staging/gpib/fmh_gpib/fmh_gpib.o: in function `fmh_gpib_dma_callback':
   fmh_gpib.c:(.text+0x1058): undefined reference to `nec7210_set_reg_bits'
   sparc-linux-ld: drivers/staging/gpib/fmh_gpib/fmh_gpib.o: in function `fmh_gpib_fifo_write_countable':
   fmh_gpib.c:(.text+0x10e8): undefined reference to `nec7210_set_reg_bits'
   sparc-linux-ld: fmh_gpib.c:(.text+0x1100): undefined reference to `nec7210_set_reg_bits'
   sparc-linux-ld: fmh_gpib.c:(.text+0x1164): undefined reference to `nec7210_set_reg_bits'
   sparc-linux-ld: fmh_gpib.c:(.text+0x1290): undefined reference to `nec7210_set_reg_bits'
   sparc-linux-ld: drivers/staging/gpib/fmh_gpib/fmh_gpib.o:fmh_gpib.c:(.text+0x12a4): more undefined references to `nec7210_set_reg_bits' follow
   sparc-linux-ld: drivers/staging/gpib/fmh_gpib/fmh_gpib.o: in function `fmh_gpib_pci_detach':
   fmh_gpib.c:(.text+0x1820): undefined reference to `nec7210_board_reset'
   sparc-linux-ld: drivers/staging/gpib/fmh_gpib/fmh_gpib.o: in function `fmh_gpib_init.isra.0':
   fmh_gpib.c:(.text+0x1ae8): undefined reference to `nec7210_board_reset'
   sparc-linux-ld: fmh_gpib.c:(.text+0x1b08): undefined reference to `nec7210_set_handshake_mode'
   sparc-linux-ld: fmh_gpib.c:(.text+0x1b44): undefined reference to `nec7210_board_online'
   sparc-linux-ld: drivers/staging/gpib/fmh_gpib/fmh_gpib.o: in function `fmh_gpib_dma_write':
   fmh_gpib.c:(.text+0x1f24): undefined reference to `nec7210_set_reg_bits'
   sparc-linux-ld: fmh_gpib.c:(.text+0x1f38): undefined reference to `nec7210_set_reg_bits'
   sparc-linux-ld: fmh_gpib.c:(.text+0x2094): undefined reference to `nec7210_set_reg_bits'
   sparc-linux-ld: drivers/staging/gpib/fmh_gpib/fmh_gpib.o: in function `fmh_gpib_pci_attach_impl':
   fmh_gpib.c:(.text+0x2388): undefined reference to `gpib_pci_get_device'
   sparc-linux-ld: drivers/staging/gpib/fmh_gpib/fmh_gpib.o: in function `fmh_gpib_dma_read':
   fmh_gpib.c:(.text+0x26b0): undefined reference to `nec7210_set_reg_bits'
   sparc-linux-ld: fmh_gpib.c:(.text+0x26c4): undefined reference to `nec7210_set_reg_bits'
   sparc-linux-ld: fmh_gpib.c:(.text+0x27f0): undefined reference to `nec7210_set_reg_bits'
   sparc-linux-ld: drivers/staging/gpib/fmh_gpib/fmh_gpib.o: in function `fmh_gpib_exit_module':
   fmh_gpib.c:(.exit.text+0x8): undefined reference to `gpib_unregister_driver'
   sparc-linux-ld: fmh_gpib.c:(.exit.text+0x14): undefined reference to `gpib_unregister_driver'
   sparc-linux-ld: fmh_gpib.c:(.exit.text+0x20): undefined reference to `gpib_unregister_driver'
   sparc-linux-ld: fmh_gpib.c:(.exit.text+0x2c): undefined reference to `gpib_unregister_driver'
   sparc-linux-ld: drivers/staging/gpib/fmh_gpib/fmh_gpib.o: in function `fmh_gpib_init_module':
   fmh_gpib.c:(.init.text+0x4c): undefined reference to `gpib_register_driver'
   sparc-linux-ld: fmh_gpib.c:(.init.text+0x68): undefined reference to `gpib_register_driver'
   sparc-linux-ld: fmh_gpib.c:(.init.text+0x84): undefined reference to `gpib_register_driver'
   sparc-linux-ld: fmh_gpib.c:(.init.text+0xa0): undefined reference to `gpib_register_driver'
>> sparc-linux-ld: fmh_gpib.c:(.init.text+0xb4): undefined reference to `gpib_unregister_driver'
   sparc-linux-ld: fmh_gpib.c:(.init.text+0xbc): undefined reference to `gpib_unregister_driver'
   sparc-linux-ld: fmh_gpib.c:(.init.text+0xc4): undefined reference to `gpib_unregister_driver'

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH v5 08/15] staging: gpib: gpio: Return error value from gpib_register_driver()

[Nihar Chaithanya]

The function gpib_register_driver() can fail and does not return an
error value if it fails.

Return the error value if gpib_register_driver() fails.

Signed-off-by: Nihar Chaithanya <niharchaithanya@gmail.com>
---
 drivers/staging/gpib/gpio/gpib_bitbang.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/gpib/gpio/gpib_bitbang.c b/drivers/staging/gpib/gpio/gpib_bitbang.c
index a2d562cbd65b..0fb654757540 100644
--- a/drivers/staging/gpib/gpio/gpib_bitbang.c
+++ b/drivers/staging/gpib/gpio/gpib_bitbang.c
@@ -1341,7 +1341,12 @@ return_to_local : bb_return_to_local,
 
 static int __init bb_init_module(void)
 {
-	gpib_register_driver(&bb_interface, THIS_MODULE);
+	int result = gpib_register_driver(&bb_interface, THIS_MODULE);
+
+	if (result) {
+		pr_err("gpib_bitbang: gpib_register_driver failed\n");
+		return result;
+	}
 
 	dbg_printk(0, "module loaded with pin map \"%s\"%s\n",
 		   pin_map, (sn7516x_used) ? " and SN7516x driver support" : "");
-- 
2.34.1

[PATCH v5 09/15] staging: gpib: hp_82335: Return error value from gpib_register_driver()

[Nihar Chaithanya]

The function gpib_register_driver() can fail and does not return an
error value if it fails.

Return the error value if gpib_register_driver() fails.

Signed-off-by: Nihar Chaithanya <niharchaithanya@gmail.com>
---
 drivers/staging/gpib/hp_82335/hp82335.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/gpib/hp_82335/hp82335.c b/drivers/staging/gpib/hp_82335/hp82335.c
index 40afe42aea47..af8ddc173d12 100644
--- a/drivers/staging/gpib/hp_82335/hp82335.c
+++ b/drivers/staging/gpib/hp_82335/hp82335.c
@@ -325,7 +325,14 @@ void hp82335_detach(gpib_board_t *board)
 
 static int __init hp82335_init_module(void)
 {
-	gpib_register_driver(&hp82335_interface, THIS_MODULE);
+	int result = gpib_register_driver(&hp82335_interface, THIS_MODULE);
+
+	if (result) {
+		pr_err("hp82335: gpib_register_driver failed!\n");
+		return result;
+	}
+
+	pr_info("hp82335: module init is complete\n");
 	return 0;
 }
 
-- 
2.34.1

[PATCH v5 10/15] staging: gpib: hp_82341: Handle gpib_register_driver() errors

[Nihar Chaithanya]

The function gpib_register_driver() can fail, resulting in a
semi-registered module and does not return an error value if it
fails.

Return the error value if the first gpib_register_driver() fails
and if the second gpib_register_driver() fails unregister the first
gpib_register_driver() and return the error value.

Signed-off-by: Nihar Chaithanya <niharchaithanya@gmail.com>
---
 drivers/staging/gpib/hp_82341/hp_82341.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/gpib/hp_82341/hp_82341.c b/drivers/staging/gpib/hp_82341/hp_82341.c
index 8ad1c885a9fb..eaeed8fb77c3 100644
--- a/drivers/staging/gpib/hp_82341/hp_82341.c
+++ b/drivers/staging/gpib/hp_82341/hp_82341.c
@@ -807,8 +807,22 @@ MODULE_DEVICE_TABLE(pnp, hp_82341_pnp_table);
 
 static int __init hp_82341_init_module(void)
 {
-	gpib_register_driver(&hp_82341_unaccel_interface, THIS_MODULE);
-	gpib_register_driver(&hp_82341_interface, THIS_MODULE);
+	int ret;
+
+	ret = gpib_register_driver(&hp_82341_unaccel_interface, THIS_MODULE);
+	if (ret) {
+		pr_err("hp_82341: gpib_register_driver failed!\n");
+		return ret;
+	}
+
+	ret = gpib_register_driver(&hp_82341_interface, THIS_MODULE);
+	if (ret) {
+		pr_err("hp_82341: gpib_register_driver failed!\n");
+		gpib_unregister_driver(&hp_82341_unaccel_interface);
+		return ret;
+	}
+
+	pr_info("hp_82341: module init is complete\n");
 	return 0;
 }
 
-- 
2.34.1

[PATCH v5 11/15] staging: gpib: ines: Handle gpib_register_driver() errors

[Nihar Chaithanya]

The function ines_pcmcia_init_module() can be replaced by calling
pcmcia_register_driver() directly. The error value from this
function is not returned and the previous registering functions
are not unregistered.

The function gpib_register_driver() can fail and similar to
pcmcia_register_driver() function failing, the previous registering
functions are not unregistered.

Replace cb_pcmcia_init_module() with pcmcia_register_driver().
Unregister the gpib and pci register functions if the subsequent
gpib or pcmcia register functions fail and return the error value.

Signed-off-by: Nihar Chaithanya <niharchaithanya@gmail.com>
---
 drivers/staging/gpib/ines/ines_gpib.c | 81 ++++++++++++++++++++-------
 1 file changed, 60 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/gpib/ines/ines_gpib.c b/drivers/staging/gpib/ines/ines_gpib.c
index 9d8387c3bf01..73df4cd16926 100644
--- a/drivers/staging/gpib/ines/ines_gpib.c
+++ b/drivers/staging/gpib/ines/ines_gpib.c
@@ -1227,12 +1227,6 @@ static struct pcmcia_driver ines_gpib_cs_driver = {
 	.resume		= ines_gpib_resume,
 };
 
-int ines_pcmcia_init_module(void)
-{
-	pcmcia_register_driver(&ines_gpib_cs_driver);
-	return 0;
-}
-
 void ines_pcmcia_cleanup_module(void)
 {
 	DEBUG(0, "ines_cs: unloading\n");
@@ -1420,28 +1414,73 @@ void ines_pcmcia_detach(gpib_board_t *board)
 
 static int __init ines_init_module(void)
 {
-	int err = 0;
+	int ret;
 
-	err = pci_register_driver(&ines_pci_driver);
-	if (err) {
-		pr_err("ines_gpib: pci_driver_register failed!\n");
-		return err;
+	ret = pci_register_driver(&ines_pci_driver);
+	if (ret) {
+		pr_err("ines_gpib: pci_driver_register failed: error = %d\n", ret);
+		return ret;
 	}
 
-	gpib_register_driver(&ines_pci_interface, THIS_MODULE);
-	gpib_register_driver(&ines_pci_unaccel_interface, THIS_MODULE);
-	gpib_register_driver(&ines_pci_accel_interface, THIS_MODULE);
-	gpib_register_driver(&ines_isa_interface, THIS_MODULE);
+	ret = gpib_register_driver(&ines_pci_interface, THIS_MODULE);
+	if (ret)
+		goto err_pci;
+
+	ret = gpib_register_driver(&ines_pci_unaccel_interface, THIS_MODULE);
+	if (ret)
+		goto err_pci_unaccel;
+
+	ret = gpib_register_driver(&ines_pci_accel_interface, THIS_MODULE);
+	if (ret)
+		goto err_pci_accel;
+
+	ret = gpib_register_driver(&ines_isa_interface, THIS_MODULE);
+	if (ret)
+		goto err_isa;
+
 #ifdef GPIB_PCMCIA
-	gpib_register_driver(&ines_pcmcia_interface, THIS_MODULE);
-	gpib_register_driver(&ines_pcmcia_unaccel_interface, THIS_MODULE);
-	gpib_register_driver(&ines_pcmcia_accel_interface, THIS_MODULE);
-	err += ines_pcmcia_init_module();
+	ret = gpib_register_driver(&ines_pcmcia_interface, THIS_MODULE);
+	if (ret)
+		goto err_pcmcia;
+
+	ret = gpib_register_driver(&ines_pcmcia_unaccel_interface, THIS_MODULE);
+	if (ret)
+		goto err_pcmcia_unaccel;
+
+	ret = gpib_register_driver(&ines_pcmcia_accel_interface, THIS_MODULE);
+	if (ret)
+		goto err_pcmcia_accel;
+
+	ret = pcmcia_register_driver(&ines_gpib_cs_driver);
+	if (ret)
+		goto err_pcmcia_driver;
 #endif
-	if (err)
-		return -1;
 
+	pr_info("ines_gpib: module init is complete\n");
 	return 0;
+
+#ifdef GPIB_PCMCIA
+err_pcmcia_driver:
+	pr_err("ines_gpib: pcmcia_register_driver failed: error = %d\n", ret);
+	gpib_unregister_driver(&ines_pcmcia_accel_interface);
+err_pcmcia_accel:
+	gpib_unregister_driver(&ines_pcmcia_unaccel_interface);
+err_pcmcia_unaccel:
+	gpib_unregister_driver(&ines_pcmcia_interface);
+err_pcmcia:
+#endif
+	gpib_unregister_driver(&ines_isa_interface);
+err_isa:
+	gpib_unregister_driver(&ines_pci_accel_interface);
+err_pci_accel:
+	gpib_unregister_driver(&ines_pci_unaccel_interface);
+err_pci_unaccel:
+	gpib_unregister_driver(&ines_pci_interface);
+err_pci:
+	pci_unregister_driver(&ines_pci_driver);
+
+	pr_err("ines_gpib: gpib_register_driver failed!\n");
+	return ret;
 }
 
 static void __exit ines_exit_module(void)
-- 
2.34.1

Re: [PATCH v5 11/15] staging: gpib: ines: Handle gpib_register_driver() errors

[Greg KH]

On Sat, Dec 28, 2024 at 11:37:59AM +0530, Nihar Chaithanya wrote:
> The function ines_pcmcia_init_module() can be replaced by calling
> pcmcia_register_driver() directly. The error value from this
> function is not returned and the previous registering functions
> are not unregistered.
> 
> The function gpib_register_driver() can fail and similar to
> pcmcia_register_driver() function failing, the previous registering
> functions are not unregistered.
> 
> Replace cb_pcmcia_init_module() with pcmcia_register_driver().
> Unregister the gpib and pci register functions if the subsequent
> gpib or pcmcia register functions fail and return the error value.
> 
> Signed-off-by: Nihar Chaithanya <niharchaithanya@gmail.com>
> ---
>  drivers/staging/gpib/ines/ines_gpib.c | 81 ++++++++++++++++++++-------
>  1 file changed, 60 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/staging/gpib/ines/ines_gpib.c b/drivers/staging/gpib/ines/ines_gpib.c
> index 9d8387c3bf01..73df4cd16926 100644
> --- a/drivers/staging/gpib/ines/ines_gpib.c
> +++ b/drivers/staging/gpib/ines/ines_gpib.c
> @@ -1227,12 +1227,6 @@ static struct pcmcia_driver ines_gpib_cs_driver = {
>  	.resume		= ines_gpib_resume,
>  };
>  
> -int ines_pcmcia_init_module(void)
> -{
> -	pcmcia_register_driver(&ines_gpib_cs_driver);
> -	return 0;
> -}
> -
>  void ines_pcmcia_cleanup_module(void)
>  {
>  	DEBUG(0, "ines_cs: unloading\n");
> @@ -1420,28 +1414,73 @@ void ines_pcmcia_detach(gpib_board_t *board)
>  
>  static int __init ines_init_module(void)
>  {
> -	int err = 0;
> +	int ret;
>  
> -	err = pci_register_driver(&ines_pci_driver);
> -	if (err) {
> -		pr_err("ines_gpib: pci_driver_register failed!\n");
> -		return err;
> +	ret = pci_register_driver(&ines_pci_driver);
> +	if (ret) {
> +		pr_err("ines_gpib: pci_driver_register failed: error = %d\n", ret);
> +		return ret;
>  	}
>  
> -	gpib_register_driver(&ines_pci_interface, THIS_MODULE);
> -	gpib_register_driver(&ines_pci_unaccel_interface, THIS_MODULE);
> -	gpib_register_driver(&ines_pci_accel_interface, THIS_MODULE);
> -	gpib_register_driver(&ines_isa_interface, THIS_MODULE);
> +	ret = gpib_register_driver(&ines_pci_interface, THIS_MODULE);
> +	if (ret)
> +		goto err_pci;
> +
> +	ret = gpib_register_driver(&ines_pci_unaccel_interface, THIS_MODULE);
> +	if (ret)
> +		goto err_pci_unaccel;
> +
> +	ret = gpib_register_driver(&ines_pci_accel_interface, THIS_MODULE);
> +	if (ret)
> +		goto err_pci_accel;
> +
> +	ret = gpib_register_driver(&ines_isa_interface, THIS_MODULE);
> +	if (ret)
> +		goto err_isa;
> +
>  #ifdef GPIB_PCMCIA
> -	gpib_register_driver(&ines_pcmcia_interface, THIS_MODULE);
> -	gpib_register_driver(&ines_pcmcia_unaccel_interface, THIS_MODULE);
> -	gpib_register_driver(&ines_pcmcia_accel_interface, THIS_MODULE);
> -	err += ines_pcmcia_init_module();
> +	ret = gpib_register_driver(&ines_pcmcia_interface, THIS_MODULE);
> +	if (ret)
> +		goto err_pcmcia;
> +
> +	ret = gpib_register_driver(&ines_pcmcia_unaccel_interface, THIS_MODULE);
> +	if (ret)
> +		goto err_pcmcia_unaccel;
> +
> +	ret = gpib_register_driver(&ines_pcmcia_accel_interface, THIS_MODULE);
> +	if (ret)
> +		goto err_pcmcia_accel;
> +
> +	ret = pcmcia_register_driver(&ines_gpib_cs_driver);
> +	if (ret)
> +		goto err_pcmcia_driver;
>  #endif
> -	if (err)
> -		return -1;
>  
> +	pr_info("ines_gpib: module init is complete\n");

Why did you add this debugging line here that was not here at all?

>  	return 0;
> +
> +#ifdef GPIB_PCMCIA
> +err_pcmcia_driver:
> +	pr_err("ines_gpib: pcmcia_register_driver failed: error = %d\n", ret);

Why just this error message for only this one failure?

> +	gpib_unregister_driver(&ines_pcmcia_accel_interface);
> +err_pcmcia_accel:
> +	gpib_unregister_driver(&ines_pcmcia_unaccel_interface);
> +err_pcmcia_unaccel:
> +	gpib_unregister_driver(&ines_pcmcia_interface);
> +err_pcmcia:
> +#endif
> +	gpib_unregister_driver(&ines_isa_interface);
> +err_isa:
> +	gpib_unregister_driver(&ines_pci_accel_interface);
> +err_pci_accel:
> +	gpib_unregister_driver(&ines_pci_unaccel_interface);
> +err_pci_unaccel:
> +	gpib_unregister_driver(&ines_pci_interface);
> +err_pci:
> +	pci_unregister_driver(&ines_pci_driver);
> +
> +	pr_err("ines_gpib: gpib_register_driver failed!\n");

Why not provide the error number here too?

thanks,

greg k-h

Re: [PATCH v5 11/15] staging: gpib: ines: Handle gpib_register_driver() errors

[Nihar Chaithanya]

On 29/12/24 15:58, Greg KH wrote:
> On Sat, Dec 28, 2024 at 11:37:59AM +0530, Nihar Chaithanya wrote:
>> The function ines_pcmcia_init_module() can be replaced by calling
>> pcmcia_register_driver() directly. The error value from this
>> function is not returned and the previous registering functions
>> are not unregistered.
>>
>> The function gpib_register_driver() can fail and similar to
>> pcmcia_register_driver() function failing, the previous registering
>> functions are not unregistered.
>>
>> Replace cb_pcmcia_init_module() with pcmcia_register_driver().
>> Unregister the gpib and pci register functions if the subsequent
>> gpib or pcmcia register functions fail and return the error value.
>>
>> Signed-off-by: Nihar Chaithanya <niharchaithanya@gmail.com>
>> ---
>>   drivers/staging/gpib/ines/ines_gpib.c | 81 ++++++++++++++++++++-------
>>   1 file changed, 60 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/staging/gpib/ines/ines_gpib.c b/drivers/staging/gpib/ines/ines_gpib.c
>> index 9d8387c3bf01..73df4cd16926 100644
>> --- a/drivers/staging/gpib/ines/ines_gpib.c
>> +++ b/drivers/staging/gpib/ines/ines_gpib.c
>> @@ -1227,12 +1227,6 @@ static struct pcmcia_driver ines_gpib_cs_driver = {
>>   	.resume		= ines_gpib_resume,
>>   };
>>   
>> -int ines_pcmcia_init_module(void)
>> -{
>> -	pcmcia_register_driver(&ines_gpib_cs_driver);
>> -	return 0;
>> -}
>> -
>>   void ines_pcmcia_cleanup_module(void)
>>   {
>>   	DEBUG(0, "ines_cs: unloading\n");
>> @@ -1420,28 +1414,73 @@ void ines_pcmcia_detach(gpib_board_t *board)
>>   
>>   static int __init ines_init_module(void)
>>   {
>> -	int err = 0;
>> +	int ret;
>>   
>> -	err = pci_register_driver(&ines_pci_driver);
>> -	if (err) {
>> -		pr_err("ines_gpib: pci_driver_register failed!\n");
>> -		return err;
>> +	ret = pci_register_driver(&ines_pci_driver);
>> +	if (ret) {
>> +		pr_err("ines_gpib: pci_driver_register failed: error = %d\n", ret);
>> +		return ret;
>>   	}
>>   
>> -	gpib_register_driver(&ines_pci_interface, THIS_MODULE);
>> -	gpib_register_driver(&ines_pci_unaccel_interface, THIS_MODULE);
>> -	gpib_register_driver(&ines_pci_accel_interface, THIS_MODULE);
>> -	gpib_register_driver(&ines_isa_interface, THIS_MODULE);
>> +	ret = gpib_register_driver(&ines_pci_interface, THIS_MODULE);
>> +	if (ret)
>> +		goto err_pci;
>> +
>> +	ret = gpib_register_driver(&ines_pci_unaccel_interface, THIS_MODULE);
>> +	if (ret)
>> +		goto err_pci_unaccel;
>> +
>> +	ret = gpib_register_driver(&ines_pci_accel_interface, THIS_MODULE);
>> +	if (ret)
>> +		goto err_pci_accel;
>> +
>> +	ret = gpib_register_driver(&ines_isa_interface, THIS_MODULE);
>> +	if (ret)
>> +		goto err_isa;
>> +
>>   #ifdef GPIB_PCMCIA
>> -	gpib_register_driver(&ines_pcmcia_interface, THIS_MODULE);
>> -	gpib_register_driver(&ines_pcmcia_unaccel_interface, THIS_MODULE);
>> -	gpib_register_driver(&ines_pcmcia_accel_interface, THIS_MODULE);
>> -	err += ines_pcmcia_init_module();
>> +	ret = gpib_register_driver(&ines_pcmcia_interface, THIS_MODULE);
>> +	if (ret)
>> +		goto err_pcmcia;
>> +
>> +	ret = gpib_register_driver(&ines_pcmcia_unaccel_interface, THIS_MODULE);
>> +	if (ret)
>> +		goto err_pcmcia_unaccel;
>> +
>> +	ret = gpib_register_driver(&ines_pcmcia_accel_interface, THIS_MODULE);
>> +	if (ret)
>> +		goto err_pcmcia_accel;
>> +
>> +	ret = pcmcia_register_driver(&ines_gpib_cs_driver);
>> +	if (ret)
>> +		goto err_pcmcia_driver;
>>   #endif
>> -	if (err)
>> -		return -1;
>>   
>> +	pr_info("ines_gpib: module init is complete\n");
> Why did you add this debugging line here that was not here at all?
>

Hi Greg, I thought to keep all the *_init_module consistent in gpib, so 
added

this. If they were not present earlier I won't add them in the next version.

>>   	return 0;
>> +
>> +#ifdef GPIB_PCMCIA
>> +err_pcmcia_driver:
>> +	pr_err("ines_gpib: pcmcia_register_driver failed: error = %d\n", ret);
> Why just this error message for only this one failure?

This failure is when pcmcia_register_driver fails, it could give multiple

error values.

>
>> +	gpib_unregister_driver(&ines_pcmcia_accel_interface);
>> +err_pcmcia_accel:
>> +	gpib_unregister_driver(&ines_pcmcia_unaccel_interface);
>> +err_pcmcia_unaccel:
>> +	gpib_unregister_driver(&ines_pcmcia_interface);
>> +err_pcmcia:
>> +#endif
>> +	gpib_unregister_driver(&ines_isa_interface);
>> +err_isa:
>> +	gpib_unregister_driver(&ines_pci_accel_interface);
>> +err_pci_accel:
>> +	gpib_unregister_driver(&ines_pci_unaccel_interface);
>> +err_pci_unaccel:
>> +	gpib_unregister_driver(&ines_pci_interface);
>> +err_pci:
>> +	pci_unregister_driver(&ines_pci_driver);
>> +
>> +	pr_err("ines_gpib: gpib_register_driver failed!\n");
> Why not provide the error number here too?

When gpib_register_driver fails it would always be ENOMEM error value,

so should I mention this number?

>
> thanks,
>
> greg k-h

Thanks,

Nihar

[PATCH v5 12/15] staging: gpib: lpvo_usb: Return error value from gpib_register_driver()

[Nihar Chaithanya]

The function gpib_register_driver() can fail and does not return an
error value if it fails.

Return the error value if gpib_register_driver() fails.

Signed-off-by: Nihar Chaithanya <niharchaithanya@gmail.com>
---
 drivers/staging/gpib/lpvo_usb_gpib/lpvo_usb_gpib.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/gpib/lpvo_usb_gpib/lpvo_usb_gpib.c b/drivers/staging/gpib/lpvo_usb_gpib/lpvo_usb_gpib.c
index 796c3a5be545..839e89615214 100644
--- a/drivers/staging/gpib/lpvo_usb_gpib/lpvo_usb_gpib.c
+++ b/drivers/staging/gpib/lpvo_usb_gpib/lpvo_usb_gpib.c
@@ -1181,7 +1181,11 @@ static int usb_gpib_init_module(struct usb_interface *interface)
 		return rv;
 
 	if (!assigned_usb_minors) {
-		gpib_register_driver(&usb_gpib_interface, THIS_MODULE);
+		rv = gpib_register_driver(&usb_gpib_interface, THIS_MODULE);
+		if (rv) {
+			pr_err("lpvo_usb_gpib: gpib_register_driver failed\n");
+			goto exit;
+		}
 	} else {
 		/* check if minor is already registered - maybe useless, but if
 		 *  it happens the code is inconsistent somewhere
-- 
2.34.1

[PATCH v5 13/15] staging: gpib: ni_usb: Handle gpib_register_driver() errors

[Nihar Chaithanya]

The usb_register() function can fail and returns an error value which
is not returned. The function gpib_register_driver() can also fail
which can result in semi-registered module.

In case gpib_register_driver() fails unregister the previous usb driver
registering function. Return the error value if gpib_register_driver()
or usb_register() functions fail.

Signed-off-by: Nihar Chaithanya <niharchaithanya@gmail.com>
---
 drivers/staging/gpib/ni_usb/ni_usb_gpib.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/gpib/ni_usb/ni_usb_gpib.c b/drivers/staging/gpib/ni_usb/ni_usb_gpib.c
index b7b6fb1be379..c21e35bef3ea 100644
--- a/drivers/staging/gpib/ni_usb/ni_usb_gpib.c
+++ b/drivers/staging/gpib/ni_usb/ni_usb_gpib.c
@@ -2619,13 +2619,24 @@ static struct usb_driver ni_usb_bus_driver = {
 static int __init ni_usb_init_module(void)
 {
 	int i;
+	int ret;
 
-	pr_info("ni_usb_gpib driver loading\n");
 	for (i = 0; i < MAX_NUM_NI_USB_INTERFACES; i++)
 		ni_usb_driver_interfaces[i] = NULL;
-	usb_register(&ni_usb_bus_driver);
-	gpib_register_driver(&ni_usb_gpib_interface, THIS_MODULE);
 
+	ret = usb_register(&ni_usb_bus_driver);
+	if (ret) {
+		pr_err("ni_usb_gpib: usb_register failed: error = %d\n", ret);
+		return ret;
+	}
+
+	ret = gpib_register_driver(&ni_usb_gpib_interface, THIS_MODULE);
+	if (ret) {
+		pr_err("ni_usb: gpib_register_driver failed\n");
+		return ret;
+	}
+
+	pr_info("ni_usb: module init is complete\n");
 	return 0;
 }
 
-- 
2.34.1

[PATCH v5 14/15] staging: gpib: pc2: Handle gpib_register_driver() errors

[Nihar Chaithanya]

The function gpib_register_driver() can fail, resulting in a
semi-registered module and does not return an error value if it
fails.

Unregister the previous gpib registering functions if subsequent
gpib_register_driver() fail and return the error value.

Signed-off-by: Nihar Chaithanya <niharchaithanya@gmail.com>
---
 drivers/staging/gpib/pc2/pc2_gpib.c | 34 +++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/gpib/pc2/pc2_gpib.c b/drivers/staging/gpib/pc2/pc2_gpib.c
index 7b3b34f47341..4441939265e4 100644
--- a/drivers/staging/gpib/pc2/pc2_gpib.c
+++ b/drivers/staging/gpib/pc2/pc2_gpib.c
@@ -635,12 +635,38 @@ void pc2_2a_detach(gpib_board_t *board)
 
 static int __init pc2_init_module(void)
 {
-	gpib_register_driver(&pc2_interface, THIS_MODULE);
-	gpib_register_driver(&pc2a_interface, THIS_MODULE);
-	gpib_register_driver(&pc2a_cb7210_interface, THIS_MODULE);
-	gpib_register_driver(&pc2_2a_interface, THIS_MODULE);
+	int ret;
 
+	ret = gpib_register_driver(&pc2_interface, THIS_MODULE);
+	if (ret) {
+		pr_err("pc2_gpib: gpib_register_driver failed!\n");
+		return ret;
+	}
+
+	ret = gpib_register_driver(&pc2a_interface, THIS_MODULE);
+	if (ret)
+		goto err_pc2a;
+
+	ret = gpib_register_driver(&pc2a_cb7210_interface, THIS_MODULE);
+	if (ret)
+		goto err_cb7210;
+
+	ret = gpib_register_driver(&pc2_2a_interface, THIS_MODULE);
+	if (ret)
+		goto err_pc2_2a;
+
+	pr_info("pc2_gpib: module init is complete\n");
 	return 0;
+
+err_pc2_2a:
+	gpib_unregister_driver(&pc2a_cb7210_interface);
+err_cb7210:
+	gpib_unregister_driver(&pc2a_interface);
+err_pc2a:
+	gpib_unregister_driver(&pc2_interface);
+
+	pr_err("pc2gpib: gpib_register_driver failed!\n");
+	return ret;
 }
 
 static void __exit pc2_exit_module(void)
-- 
2.34.1

[PATCH v5 15/15] staging: gpib: tnt4882: Handle gpib_register_driver() errors

[Nihar Chaithanya]

The error value from the init_ni_gpib_cs() function is not
returned and the previous registering functions are not unregistered.

The function gpib_register_driver() can fail and similar to
pcmcia_register_driver() function failing, the previous registering
functions are not unregistered.

Unregister the gpib and pci register functions if the subsequent
gpib or pcmcia register functions fail and return the error value.

Signed-off-by: Nihar Chaithanya <niharchaithanya@gmail.com>
---
 drivers/staging/gpib/tnt4882/tnt4882_gpib.c | 87 ++++++++++++++++++---
 1 file changed, 74 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/gpib/tnt4882/tnt4882_gpib.c b/drivers/staging/gpib/tnt4882/tnt4882_gpib.c
index e49a952fa0d8..eab7fd121f0c 100644
--- a/drivers/staging/gpib/tnt4882/tnt4882_gpib.c
+++ b/drivers/staging/gpib/tnt4882/tnt4882_gpib.c
@@ -1524,29 +1524,90 @@ static int __init tnt4882_init_module(void)
 
 	result = pci_register_driver(&tnt4882_pci_driver);
 	if (result) {
-		pr_err("tnt4882: pci_driver_register failed!\n");
+		pr_err("tnt4882_gpib: pci_driver_register failed: error = %d\n", result);
 		return result;
 	}
 
-	gpib_register_driver(&ni_isa_interface, THIS_MODULE);
-	gpib_register_driver(&ni_isa_accel_interface, THIS_MODULE);
-	gpib_register_driver(&ni_nat4882_isa_interface, THIS_MODULE);
-	gpib_register_driver(&ni_nat4882_isa_accel_interface, THIS_MODULE);
-	gpib_register_driver(&ni_nec_isa_interface, THIS_MODULE);
-	gpib_register_driver(&ni_nec_isa_accel_interface, THIS_MODULE);
-	gpib_register_driver(&ni_pci_interface, THIS_MODULE);
-	gpib_register_driver(&ni_pci_accel_interface, THIS_MODULE);
+	result = gpib_register_driver(&ni_isa_interface, THIS_MODULE);
+	if (result)
+		goto err_isa;
+
+	result = gpib_register_driver(&ni_isa_accel_interface, THIS_MODULE);
+	if (result)
+		goto err_isa_accel;
+
+	result = gpib_register_driver(&ni_nat4882_isa_interface, THIS_MODULE);
+	if (result)
+		goto err_nat4882_isa;
+
+	result = gpib_register_driver(&ni_nat4882_isa_accel_interface, THIS_MODULE);
+	if (result)
+		goto err_nat4882_isa_accel;
+
+	result = gpib_register_driver(&ni_nec_isa_interface, THIS_MODULE);
+	if (result)
+		goto err_nec_isa;
+
+	result = gpib_register_driver(&ni_nec_isa_accel_interface, THIS_MODULE);
+	if (result)
+		goto err_nec_isa_accel;
+
+	result = gpib_register_driver(&ni_pci_interface, THIS_MODULE);
+	if (result)
+		goto err_pci;
+
+	result = gpib_register_driver(&ni_pci_accel_interface, THIS_MODULE);
+	if (result)
+		goto err_pci_accel;
+
 #ifdef GPIB_PCMCIA
-	gpib_register_driver(&ni_pcmcia_interface, THIS_MODULE);
-	gpib_register_driver(&ni_pcmcia_accel_interface, THIS_MODULE);
-	if (init_ni_gpib_cs() < 0)
-		return -1;
+	result = gpib_register_driver(&ni_pcmcia_interface, THIS_MODULE);
+	if (result)
+		goto err_pcmcia;
+
+	result = gpib_register_driver(&ni_pcmcia_accel_interface, THIS_MODULE);
+	if (result)
+		goto err_pcmcia_accel;
+
+	result = init_ni_gpib_cs();
+	if (result)
+		goto err_pcmcia_driver;
 #endif
 
 	mite_init();
 	mite_list_devices();
 
+	pr_info("tnt4882_gpib: module init is complete\n");
 	return 0;
+
+#ifdef GPIB_PCMCIA
+err_pcmcia_driver:
+	pr_err("tnt4882_gpib: pcmcia_register_driver failed: error = %d\n", result);
+	gpib_unregister_driver(&ni_pcmcia_accel_interface);
+err_pcmcia_accel:
+	gpib_unregister_driver(&ni_pcmcia_interface);
+err_pcmcia:
+#endif
+	gpib_unregister_driver(&ni_pci_accel_interface);
+err_pci_accel:
+	gpib_unregister_driver(&ni_pci_interface);
+err_pci:
+	gpib_unregister_driver(&ni_nec_isa_accel_interface);
+err_nec_isa_accel:
+	gpib_unregister_driver(&ni_nec_isa_interface);
+err_nec_isa:
+	gpib_unregister_driver(&ni_nat4882_isa_accel_interface);
+err_nat4882_isa_accel:
+	gpib_unregister_driver(&ni_nat4882_isa_interface);
+err_nat4882_isa:
+	gpib_unregister_driver(&ni_isa_accel_interface);
+err_isa_accel:
+	gpib_unregister_driver(&ni_isa_interface);
+err_isa:
+	pci_unregister_driver(&tnt4882_pci_driver);
+
+	pr_err("tnt4882_gpib: gpib_register_driver failed\n");
+	return result;
 }
 
 static void __exit tnt4882_exit_module(void)
-- 
2.34.1