Skip to contentSkip to conversation

TTUSB DVB Budget: Fine-tuning for three function implementations

PATCH New SF Markus Elfring 1 participant 4 messages Sep 20, 2017

Subsystems (2)
Media (V4L/DVB) Kernel janitors
lore
Share & tools
lore

SF Markus Elfring

author
#0 lore raw

cover TTUSB DVB Budget: Fine-tuning for three function implementations

From: Markus Elfring <[email protected]>
Date: Wed, 20 Sep 2017 21:03:45 +0200

Three update suggestions were taken into account
from static source code analysis.

Markus Elfring (3):
Use common error handling code in ttusb_probe() Improve two size determinations in ttusb_probe() Adjust eight checks for null pointers

 drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c | 34 +++++++++++------------
 1 file changed, 17 insertions(+), 17 deletions(-)
Signature · 1 line
2.14.1
author
#1 lore raw

PATCH 1/3 dvb-ttusb-budget: Use common error handling code in ttusb_probe()

From: Markus Elfring <[email protected]>
Date: Wed, 20 Sep 2017 20:25:24 +0200

Add two jump targets so that a bit of exception handling can be better reused at the end of this function.

This issue was detected by using the Coccinelle software.

Signed-off-by Markus Elfring <[email protected]>
---
 drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)
drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c +6 -7
index b842f367249f..38394c9ecc67 100644
@@ -1675,8 +1675,7 @@ static int ttusb_probe(struct usb_interface *intf, const struct usb_device_id *i
 	if (result < 0) {
 		dprintk("%s: ttusb_alloc_iso_urbs - failed\n", __func__);
 		mutex_unlock(&ttusb->semi2c);
-		kfree(ttusb);
-		return result;
+		goto err_free_usb;
 	}
 
 	if (ttusb_init_controller(ttusb))

@@ -1687,11 +1686,9 @@ static int ttusb_probe(struct usb_interface *intf, const struct usb_device_id *i
 	result = dvb_register_adapter(&ttusb->adapter,
 				      "Technotrend/Hauppauge Nova-USB",
 				      THIS_MODULE, &udev->dev, adapter_nr);
-	if (result < 0) {
-		ttusb_free_iso_urbs(ttusb);
-		kfree(ttusb);
-		return result;
-	}
+	if (result < 0)
+		goto err_free_iso_urbs;
+
 	ttusb->adapter.priv = ttusb;
 
 	/* i2c */

@@ -1762,7 +1759,9 @@ static int ttusb_probe(struct usb_interface *intf, const struct usb_device_id *i
 	i2c_del_adapter(&ttusb->i2c_adap);
 err_unregister_adapter:
 	dvb_unregister_adapter (&ttusb->adapter);
+err_free_iso_urbs:
 	ttusb_free_iso_urbs(ttusb);
+err_free_usb:
 	kfree(ttusb);
 	return result;
 }
Signature · 1 line
2.14.1
author
#2 lore raw

PATCH 2/3 dvb-ttusb-budget: Improve two size determinations in ttusb_probe()

In reply to SF Markus Elfring

From: Markus Elfring <[email protected]>
Date: Wed, 20 Sep 2017 20:46:11 +0200

* The script "checkpatch.pl" pointed information out like the following.

ERROR: do not use assignment in if condition

Thus fix an affected source code place.

* Replace the specification of data structures by variable references as the parameter for the operator "sizeof" to make the corresponding size determination a bit safer according to the Linux coding style convention.

Signed-off-by Markus Elfring <[email protected]>
---
 drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c +3 -2
index 38394c9ecc67..fef3c8554e91 100644
@@ -1657,7 +1657,8 @@ static int ttusb_probe(struct usb_interface *intf, const struct usb_device_id *i
 
 	if (intf->altsetting->desc.bInterfaceNumber != 1) return -ENODEV;
 
-	if (!(ttusb = kzalloc(sizeof(struct ttusb), GFP_KERNEL)))
+	ttusb = kzalloc(sizeof(*ttusb), GFP_KERNEL);
+	if (!ttusb)
 		return -ENOMEM;
 
 	ttusb->dev = udev;

@@ -1692,7 +1693,7 @@ static int ttusb_probe(struct usb_interface *intf, const struct usb_device_id *i
 	ttusb->adapter.priv = ttusb;
 
 	/* i2c */
-	memset(&ttusb->i2c_adap, 0, sizeof(struct i2c_adapter));
+	memset(&ttusb->i2c_adap, 0, sizeof(ttusb->i2c_adap));
 	strcpy(ttusb->i2c_adap.name, "TTUSB DEC");
 
 	i2c_set_adapdata(&ttusb->i2c_adap, ttusb);
Signature · 1 line
2.14.1
author
#3 lore raw

PATCH 3/3 dvb-ttusb-budget: Adjust eight checks for null pointers

In reply to SF Markus Elfring

From: Markus Elfring <[email protected]>
Date: Wed, 20 Sep 2017 20:53:13 +0200
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The script “checkpatch.pl” pointed information out like the following.

Comparison to NULL could be written …

Thus fix the affected source code places.

Signed-off-by Markus Elfring <[email protected]>
---
 drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c +8 -8
index fef3c8554e91..2e97b1e64249 100644
@@ -1572,7 +1572,7 @@ static void frontend_init(struct ttusb* ttusb)
 	case 0x1003: // Hauppauge/TT Nova-USB-S budget (stv0299/ALPS BSRU6|BSBE1(tsa5059))
 		// try the stv0299 based first
 		ttusb->fe = dvb_attach(stv0299_attach, &alps_stv0299_config, &ttusb->i2c_adap);
-		if (ttusb->fe != NULL) {
+		if (ttusb->fe) {
 			ttusb->fe->ops.tuner_ops.set_params = philips_tsa5059_tuner_set_params;
 
 			if(ttusb->revision = TTUSB_REV_2_2) { // ALPS BSBE1

@@ -1586,7 +1586,7 @@ static void frontend_init(struct ttusb* ttusb)
 
 		// Grundig 29504-491
 		ttusb->fe = dvb_attach(tda8083_attach, &ttusb_novas_grundig_29504_491_config, &ttusb->i2c_adap);
-		if (ttusb->fe != NULL) {
+		if (ttusb->fe) {
 			ttusb->fe->ops.tuner_ops.set_params = ttusb_novas_grundig_29504_491_tuner_set_params;
 			ttusb->fe->ops.set_voltage = ttusb_set_voltage;
 			break;

@@ -1595,13 +1595,13 @@ static void frontend_init(struct ttusb* ttusb)
 
 	case 0x1004: // Hauppauge/TT DVB-C budget (ves1820/ALPS TDBE2(sp5659))
 		ttusb->fe = dvb_attach(ves1820_attach, &alps_tdbe2_config, &ttusb->i2c_adap, read_pwm(ttusb));
-		if (ttusb->fe != NULL) {
+		if (ttusb->fe) {
 			ttusb->fe->ops.tuner_ops.set_params = alps_tdbe2_tuner_set_params;
 			break;
 		}
 
 		ttusb->fe = dvb_attach(stv0297_attach, &dvbc_philips_tdm1316l_config, &ttusb->i2c_adap);
-		if (ttusb->fe != NULL) {
+		if (ttusb->fe) {
 			ttusb->fe->ops.tuner_ops.set_params = dvbc_philips_tdm1316l_tuner_set_params;
 			break;
 		}

@@ -1610,14 +1610,14 @@ static void frontend_init(struct ttusb* ttusb)
 	case 0x1005: // Hauppauge/TT Nova-USB-t budget (tda10046/Philips td1316(tda6651tt) OR cx22700/ALPS TDMB7(??))
 		// try the ALPS TDMB7 first
 		ttusb->fe = dvb_attach(cx22700_attach, &alps_tdmb7_config, &ttusb->i2c_adap);
-		if (ttusb->fe != NULL) {
+		if (ttusb->fe) {
 			ttusb->fe->ops.tuner_ops.set_params = alps_tdmb7_tuner_set_params;
 			break;
 		}
 
 		// Philips td1316
 		ttusb->fe = dvb_attach(tda10046_attach, &philips_tdm1316l_config, &ttusb->i2c_adap);
-		if (ttusb->fe != NULL) {
+		if (ttusb->fe) {
 			ttusb->fe->ops.tuner_ops.init = philips_tdm1316l_tuner_init;
 			ttusb->fe->ops.tuner_ops.set_params = philips_tdm1316l_tuner_set_params;
 			break;

@@ -1625,7 +1625,7 @@ static void frontend_init(struct ttusb* ttusb)
 		break;
 	}
 
-	if (ttusb->fe = NULL) {
+	if (!ttusb->fe) {
 		printk("dvb-ttusb-budget: A frontend driver was not found for device [%04x:%04x]\n",
 		       le16_to_cpu(ttusb->dev->descriptor.idVendor),
 		       le16_to_cpu(ttusb->dev->descriptor.idProduct));

@@ -1781,7 +1781,7 @@ static void ttusb_disconnect(struct usb_interface *intf)
 	dvb_net_release(&ttusb->dvbnet);
 	dvb_dmxdev_release(&ttusb->dmxdev);
 	dvb_dmx_release(&ttusb->dvb_demux);
-	if (ttusb->fe != NULL) {
+	if (ttusb->fe) {
 		dvb_unregister_frontend(ttusb->fe);
 		dvb_frontend_detach(ttusb->fe);
 	}
Signature · 1 line
2.14.1