[PATCH 0/8] [media] ttusb_dec: Fine-tuning for some function implementations

[PATCH 0/8] [media] ttusb_dec: Fine-tuning for some function implementations

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 14 Sep 2017 12:26:24 +0200

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

Markus Elfring (8):
  Use common error handling code in ttusb_dec_init_dvb()
  Adjust five checks for null pointers
  Improve a size determination in three functions
  Delete an error message for a failed memory allocation in ttusb_dec_probe()
  Move an assignment in ttusb_dec_probe()
  Reduce the scope for three variables in ttusb_dec_process_urb()
  Add spaces for better code readability
  Delete four unwanted spaces

 drivers/media/usb/ttusb-dec/ttusb_dec.c | 175 ++++++++++++++------------------
 1 file changed, 79 insertions(+), 96 deletions(-)

-- 
2.14.1

[PATCH 1/8] [media] ttusb_dec: Use common error handling code in ttusb_dec_init_dvb()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 13 Sep 2017 18:08:19 +0200

Add 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 <elfring@users.sourceforge.net>
---
 drivers/media/usb/ttusb-dec/ttusb_dec.c | 43 ++++++++++++---------------------
 1 file changed, 15 insertions(+), 28 deletions(-)

diff --git a/drivers/media/usb/ttusb-dec/ttusb_dec.c b/drivers/media/usb/ttusb-dec/ttusb_dec.c
index cdefb5dfbbdc..0bc80daf6e2e 100644
--- a/drivers/media/usb/ttusb-dec/ttusb_dec.c
+++ b/drivers/media/usb/ttusb-dec/ttusb_dec.c
@@ -1508,10 +1508,7 @@ static int ttusb_dec_init_dvb(struct ttusb_dec *dec)
 	if ((result = dvb_dmx_init(&dec->demux)) < 0) {
 		printk("%s: dvb_dmx_init failed: error %d\n", __func__,
 		       result);
-
-		dvb_unregister_adapter(&dec->adapter);
-
-		return result;
+		goto unregister_adapter;
 	}
 
 	dec->dmxdev.filternum = 32;
@@ -1521,43 +1518,33 @@ static int ttusb_dec_init_dvb(struct ttusb_dec *dec)
 	if ((result = dvb_dmxdev_init(&dec->dmxdev, &dec->adapter)) < 0) {
 		printk("%s: dvb_dmxdev_init failed: error %d\n",
 		       __func__, result);
-
-		dvb_dmx_release(&dec->demux);
-		dvb_unregister_adapter(&dec->adapter);
-
-		return result;
+		goto release_demux;
 	}
 
 	dec->frontend.source = DMX_FRONTEND_0;
 
-	if ((result = dec->demux.dmx.add_frontend(&dec->demux.dmx,
-						  &dec->frontend)) < 0) {
-		printk("%s: dvb_dmx_init failed: error %d\n", __func__,
-		       result);
-
-		dvb_dmxdev_release(&dec->dmxdev);
-		dvb_dmx_release(&dec->demux);
-		dvb_unregister_adapter(&dec->adapter);
-
-		return result;
-	}
+	result = dec->demux.dmx.add_frontend(&dec->demux.dmx, &dec->frontend);
+	if (result < 0)
+		goto report_failure;
 
 	if ((result = dec->demux.dmx.connect_frontend(&dec->demux.dmx,
 						      &dec->frontend)) < 0) {
-		printk("%s: dvb_dmx_init failed: error %d\n", __func__,
-		       result);
-
 		dec->demux.dmx.remove_frontend(&dec->demux.dmx, &dec->frontend);
-		dvb_dmxdev_release(&dec->dmxdev);
-		dvb_dmx_release(&dec->demux);
-		dvb_unregister_adapter(&dec->adapter);
-
-		return result;
+		goto report_failure;
 	}
 
 	dvb_net_init(&dec->adapter, &dec->dvb_net, &dec->demux.dmx);
 
 	return 0;
+
+report_failure:
+	printk("%s: dvb_dmx_init failed: error %d\n", __func__, result);
+	dvb_dmxdev_release(&dec->dmxdev);
+release_demux:
+	dvb_dmx_release(&dec->demux);
+unregister_adapter:
+	dvb_unregister_adapter(&dec->adapter);
+	return result;
 }
 
 static void ttusb_dec_exit_dvb(struct ttusb_dec *dec)
-- 
2.14.1

[PATCH 2/8] [media] ttusb_dec: Adjust five checks for null pointers

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 13 Sep 2017 18:22:35 +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 <elfring@users.sourceforge.net>
---
 drivers/media/usb/ttusb-dec/ttusb_dec.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/media/usb/ttusb-dec/ttusb_dec.c b/drivers/media/usb/ttusb-dec/ttusb_dec.c
index 0bc80daf6e2e..901cb221aad2 100644
--- a/drivers/media/usb/ttusb-dec/ttusb_dec.c
+++ b/drivers/media/usb/ttusb-dec/ttusb_dec.c
@@ -406,15 +406,15 @@ static int ttusb_dec_get_stb_state (struct ttusb_dec *dec, unsigned int *mode,
 		return result;
 
 	if (c_length >= 0x0c) {
-		if (mode != NULL) {
+		if (mode) {
 			memcpy(&tmp, c, 4);
 			*mode = ntohl(tmp);
 		}
-		if (model != NULL) {
+		if (model) {
 			memcpy(&tmp, &c[4], 4);
 			*model = ntohl(tmp);
 		}
-		if (version != NULL) {
+		if (version) {
 			memcpy(&tmp, &c[8], 4);
 			*version = ntohl(tmp);
 		}
@@ -1393,7 +1393,7 @@ static int ttusb_dec_boot_dsp(struct ttusb_dec *dec)
 	j = 0;
 
 	b = kmalloc(ARM_PACKET_SIZE, GFP_KERNEL);
-	if (b == NULL) {
+	if (!b) {
 		release_firmware(fw_entry);
 		return -ENOMEM;
 	}
@@ -1703,18 +1703,17 @@ static int ttusb_dec_probe(struct usb_interface *intf,
 		break;
 	}
 
-	if (dec->fe == NULL) {
+	if (!dec->fe)
 		printk("dvb-ttusb-dec: A frontend driver was not found for device [%04x:%04x]\n",
 		       le16_to_cpu(dec->udev->descriptor.idVendor),
 		       le16_to_cpu(dec->udev->descriptor.idProduct));
-	} else {
+	else
 		if (dvb_register_frontend(&dec->adapter, dec->fe)) {
 			printk("budget-ci: Frontend registration failed!\n");
 			if (dec->fe->ops.release)
 				dec->fe->ops.release(dec->fe);
 			dec->fe = NULL;
 		}
-	}
 
 	ttusb_dec_init_v_pes(dec);
 	ttusb_dec_init_filters(dec);
-- 
2.14.1

[PATCH 3/8] [media] ttusb_dec: Improve a size determination in three functions

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 13 Sep 2017 19:56:38 +0200

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

  ERROR: do not use assignment in if condition

  Thus fix a few source code places.

* Replace the specification of data structures by pointer dereferences
  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 <elfring@users.sourceforge.net>
---
 drivers/media/usb/ttusb-dec/ttusb_dec.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/media/usb/ttusb-dec/ttusb_dec.c b/drivers/media/usb/ttusb-dec/ttusb_dec.c
index 901cb221aad2..76070da3b7c7 100644
--- a/drivers/media/usb/ttusb-dec/ttusb_dec.c
+++ b/drivers/media/usb/ttusb-dec/ttusb_dec.c
@@ -821,8 +821,8 @@ static void ttusb_dec_process_urb(struct urb *urb)
 			b = urb->transfer_buffer + d->offset;
 			length = d->actual_length;
 
-			if ((frame = kmalloc(sizeof(struct urb_frame),
-					     GFP_ATOMIC))) {
+			frame = kmalloc(sizeof(*frame), GFP_ATOMIC);
+			if (frame) {
 				unsigned long flags;
 
 				memcpy(frame->data, b, length);
@@ -1073,8 +1073,8 @@ static int ttusb_dec_start_sec_feed(struct dvb_demux_feed *dvbdmxfeed)
 
 	if (!result) {
 		if (c_length == 2) {
-			if (!(finfo = kmalloc(sizeof(struct filter_info),
-					      GFP_ATOMIC)))
+			finfo = kmalloc(sizeof(*finfo), GFP_ATOMIC);
+			if (!finfo)
 				return -ENOMEM;
 
 			finfo->stream_id = c[1];
@@ -1658,7 +1658,8 @@ static int ttusb_dec_probe(struct usb_interface *intf,
 
 	udev = interface_to_usbdev(intf);
 
-	if (!(dec = kzalloc(sizeof(struct ttusb_dec), GFP_KERNEL))) {
+	dec = kzalloc(sizeof(*dec), GFP_KERNEL);
+	if (!dec) {
 		printk("%s: couldn't allocate memory.\n", __func__);
 		return -ENOMEM;
 	}
-- 
2.14.1

[PATCH 4/8] [media] ttusb_dec: Delete an error message for a failed memory allocation in ttusb_dec_probe()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 13 Sep 2017 20:10:39 +0200

Omit an extra message for a memory allocation failure in this function.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/media/usb/ttusb-dec/ttusb_dec.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/media/usb/ttusb-dec/ttusb_dec.c b/drivers/media/usb/ttusb-dec/ttusb_dec.c
index 76070da3b7c7..26d637684b30 100644
--- a/drivers/media/usb/ttusb-dec/ttusb_dec.c
+++ b/drivers/media/usb/ttusb-dec/ttusb_dec.c
@@ -1659,10 +1659,8 @@ static int ttusb_dec_probe(struct usb_interface *intf,
 	udev = interface_to_usbdev(intf);
 
 	dec = kzalloc(sizeof(*dec), GFP_KERNEL);
-	if (!dec) {
-		printk("%s: couldn't allocate memory.\n", __func__);
+	if (!dec)
 		return -ENOMEM;
-	}
 
 	usb_set_intfdata(intf, (void *)dec);
 
-- 
2.14.1

[PATCH 5/8] [media] ttusb_dec: Move an assignment in ttusb_dec_probe()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 13 Sep 2017 20:32:25 +0200

Assign a pointer to a data structure member without using an intermediate
local variable.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/media/usb/ttusb-dec/ttusb_dec.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/media/usb/ttusb-dec/ttusb_dec.c b/drivers/media/usb/ttusb-dec/ttusb_dec.c
index 26d637684b30..7759de653ee9 100644
--- a/drivers/media/usb/ttusb-dec/ttusb_dec.c
+++ b/drivers/media/usb/ttusb-dec/ttusb_dec.c
@@ -1650,14 +1650,10 @@ static const struct ttusbdecfe_config fe_config = {
 static int ttusb_dec_probe(struct usb_interface *intf,
 			   const struct usb_device_id *id)
 {
-	struct usb_device *udev;
 	struct ttusb_dec *dec;
 	int result;
 
 	dprintk("%s\n", __func__);
-
-	udev = interface_to_usbdev(intf);
-
 	dec = kzalloc(sizeof(*dec), GFP_KERNEL);
 	if (!dec)
 		return -ENOMEM;
@@ -1678,7 +1674,7 @@ static int ttusb_dec_probe(struct usb_interface *intf,
 		break;
 	}
 
-	dec->udev = udev;
+	dec->udev = interface_to_usbdev(intf);
 
 	result = ttusb_dec_init_usb(dec);
 	if (result)
-- 
2.14.1

[PATCH 6/8] [media] ttusb_dec: Reduce the scope for three variables in ttusb_dec_process_urb()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 13 Sep 2017 21:23:29 +0200

* Move the definition for the local variables "b", "d" and "length" into
  an if branch so that the corresponding setting will only be performed
  if a memory allocation succeeded in this function.

* Adjust their data types.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/media/usb/ttusb-dec/ttusb_dec.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/media/usb/ttusb-dec/ttusb_dec.c b/drivers/media/usb/ttusb-dec/ttusb_dec.c
index 7759de653ee9..e9fe4c6142a5 100644
--- a/drivers/media/usb/ttusb-dec/ttusb_dec.c
+++ b/drivers/media/usb/ttusb-dec/ttusb_dec.c
@@ -812,19 +812,18 @@ static void ttusb_dec_process_urb(struct urb *urb)
 		int i;
 
 		for (i = 0; i < FRAMES_PER_ISO_BUF; i++) {
-			struct usb_iso_packet_descriptor *d;
-			u8 *b;
-			int length;
 			struct urb_frame *frame;
 
-			d = &urb->iso_frame_desc[i];
-			b = urb->transfer_buffer + d->offset;
-			length = d->actual_length;
-
 			frame = kmalloc(sizeof(*frame), GFP_ATOMIC);
 			if (frame) {
 				unsigned long flags;
+				struct usb_iso_packet_descriptor const *d;
+				u8 const *b;
+				unsigned int length;
 
+				d = &urb->iso_frame_desc[i];
+				b = urb->transfer_buffer + d->offset;
+				length = d->actual_length;
 				memcpy(frame->data, b, length);
 				frame->length = length;
 
-- 
2.14.1

[PATCH 7/8] [media] ttusb_dec: Add spaces for better code readability

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 13 Sep 2017 22:12:07 +0200

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

ERROR: spaces required around that '=' (ctx:VxV)

Thus fix the affected source code places.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/media/usb/ttusb-dec/ttusb_dec.c | 79 +++++++++++++++++----------------
 1 file changed, 41 insertions(+), 38 deletions(-)

diff --git a/drivers/media/usb/ttusb-dec/ttusb_dec.c b/drivers/media/usb/ttusb-dec/ttusb_dec.c
index e9fe4c6142a5..58256d518fa6 100644
--- a/drivers/media/usb/ttusb-dec/ttusb_dec.c
+++ b/drivers/media/usb/ttusb-dec/ttusb_dec.c
@@ -209,47 +209,49 @@ static void dvb_filter_pes2ts_init(struct dvb_filter_pes2ts *p2ts,
 				   unsigned short pid,
 				   dvb_filter_pes2ts_cb_t *cb, void *priv)
 {
-	unsigned char *buf=p2ts->buf;
-
-	buf[0]=0x47;
-	buf[1]=(pid>>8);
-	buf[2]=pid&0xff;
-	p2ts->cc=0;
-	p2ts->cb=cb;
-	p2ts->priv=priv;
+	unsigned char *buf = p2ts->buf;
+
+	buf[0] = 0x47;
+	buf[1] = pid >> 8;
+	buf[2] = pid & 0xff;
+	p2ts->cc = 0;
+	p2ts->cb = cb;
+	p2ts->priv = priv;
 }
 
 static int dvb_filter_pes2ts(struct dvb_filter_pes2ts *p2ts,
 			     unsigned char *pes, int len, int payload_start)
 {
-	unsigned char *buf=p2ts->buf;
-	int ret=0, rest;
+	unsigned char *buf = p2ts->buf;
+	int ret = 0, rest;
 
 	//len=6+((pes[4]<<8)|pes[5]);
 
 	if (payload_start)
-		buf[1]|=0x40;
+		buf[1] |= 0x40;
 	else
-		buf[1]&=~0x40;
-	while (len>=184) {
-		buf[3]=0x10|((p2ts->cc++)&0x0f);
-		memcpy(buf+4, pes, 184);
-		if ((ret=p2ts->cb(p2ts->priv, buf)))
+		buf[1] &= ~0x40;
+	while (len >= 184) {
+		buf[3] = 0x10 | ((p2ts->cc++) & 0x0f);
+		memcpy(buf + 4, pes, 184);
+		ret = p2ts->cb(p2ts->priv, buf);
+		if (ret)
 			return ret;
-		len-=184; pes+=184;
-		buf[1]&=~0x40;
+		len -= 184;
+		pes += 184;
+		buf[1] &= ~0x40;
 	}
 	if (!len)
 		return 0;
-	buf[3]=0x30|((p2ts->cc++)&0x0f);
-	rest=183-len;
+	buf[3] = 0x30 | ((p2ts->cc++) & 0x0f);
+	rest = 183 - len;
 	if (rest) {
-		buf[5]=0x00;
-		if (rest-1)
-			memset(buf+6, 0xff, rest-1);
+		buf[5] = 0x00;
+		if (rest - 1)
+			memset(buf + 6, 0xff, rest - 1);
 	}
-	buf[4]=rest;
-	memcpy(buf+5+rest, pes, len);
+	buf[4] = rest;
+	memcpy(buf + 5 + rest, pes, len);
 	return p2ts->cb(p2ts->priv, buf);
 }
 
@@ -262,7 +264,7 @@ static void ttusb_dec_handle_irq( struct urb *urb)
 	char *buffer = dec->irq_buffer;
 	int retval;
 
-	switch(urb->status) {
+	switch (urb->status) {
 		case 0: /*success*/
 			break;
 		case -ECONNRESET:
@@ -275,7 +277,7 @@ static void ttusb_dec_handle_irq( struct urb *urb)
 			return;
 		default:
 			dprintk("%s:nonzero status received: %d\n",
-					__func__,urb->status);
+				__func__, urb->status);
 			goto exit;
 	}
 
@@ -1263,10 +1265,9 @@ static int ttusb_init_rc( struct ttusb_dec *dec)
 
 	dec->rc_input_dev = input_dev;
 	if (usb_submit_urb(dec->irq_urb, GFP_KERNEL))
-		printk("%s: usb_submit_urb failed\n",__func__);
+		printk("%s: usb_submit_urb failed\n", __func__);
 	/* enable irq pipe */
-	ttusb_dec_send_command(dec,0xb0,sizeof(b),b,NULL,NULL);
-
+	ttusb_dec_send_command(dec, 0xb0, sizeof(b), b, NULL, NULL);
 	return 0;
 }
 
@@ -1295,18 +1296,20 @@ static int ttusb_dec_init_usb(struct ttusb_dec *dec)
 	dec->out_pipe = usb_sndisocpipe(dec->udev, OUT_PIPE);
 	dec->irq_pipe = usb_rcvintpipe(dec->udev, IRQ_PIPE);
 
-	if(enable_rc) {
+	if (enable_rc) {
 		dec->irq_urb = usb_alloc_urb(0, GFP_KERNEL);
-		if(!dec->irq_urb) {
+		if (!dec->irq_urb)
 			return -ENOMEM;
-		}
-		dec->irq_buffer = usb_alloc_coherent(dec->udev,IRQ_PACKET_SIZE,
-					GFP_KERNEL, &dec->irq_dma_handle);
-		if(!dec->irq_buffer) {
+
+		dec->irq_buffer = usb_alloc_coherent(dec->udev,
+						     IRQ_PACKET_SIZE,
+						     GFP_KERNEL,
+						     &dec->irq_dma_handle);
+		if (!dec->irq_buffer) {
 			usb_free_urb(dec->irq_urb);
 			return -ENOMEM;
 		}
-		usb_fill_int_urb(dec->irq_urb, dec->udev,dec->irq_pipe,
+		usb_fill_int_urb(dec->irq_urb, dec->udev, dec->irq_pipe,
 				 dec->irq_buffer, IRQ_PACKET_SIZE,
 				 ttusb_dec_handle_irq, dec, 1);
 		dec->irq_urb->transfer_dma = dec->irq_dma_handle;
@@ -1739,7 +1742,7 @@ static void ttusb_dec_disconnect(struct usb_interface *intf)
 	if (dec->active) {
 		ttusb_dec_exit_tasklet(dec);
 		ttusb_dec_exit_filters(dec);
-		if(enable_rc)
+		if (enable_rc)
 			ttusb_dec_exit_rc(dec);
 		ttusb_dec_exit_usb(dec);
 		ttusb_dec_exit_dvb(dec);
-- 
2.14.1

[PATCH 8/8] [media] ttusb_dec: Delete four unwanted spaces

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 13 Sep 2017 22:22:41 +0200

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

ERROR: space prohibited after that open parenthesis '('

Thus fix affected source code places.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/media/usb/ttusb-dec/ttusb_dec.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/usb/ttusb-dec/ttusb_dec.c b/drivers/media/usb/ttusb-dec/ttusb_dec.c
index 58256d518fa6..b05f83eac95d 100644
--- a/drivers/media/usb/ttusb-dec/ttusb_dec.c
+++ b/drivers/media/usb/ttusb-dec/ttusb_dec.c
@@ -258,7 +258,7 @@ static int dvb_filter_pes2ts(struct dvb_filter_pes2ts *p2ts,
 static void ttusb_dec_set_model(struct ttusb_dec *dec,
 				enum ttusb_dec_model model);
 
-static void ttusb_dec_handle_irq( struct urb *urb)
+static void ttusb_dec_handle_irq(struct urb *urb)
 {
 	struct ttusb_dec *dec = urb->context;
 	char *buffer = dec->irq_buffer;
@@ -393,8 +393,8 @@ static int ttusb_dec_send_command(struct ttusb_dec *dec, const u8 command,
 	}
 }
 
-static int ttusb_dec_get_stb_state (struct ttusb_dec *dec, unsigned int *mode,
-				    unsigned int *model, unsigned int *version)
+static int ttusb_dec_get_stb_state(struct ttusb_dec *dec, unsigned int *mode,
+				   unsigned int *model, unsigned int *version)
 {
 	u8 c[COMMAND_PACKET_SIZE];
 	int c_length;
@@ -1233,7 +1233,7 @@ static void ttusb_dec_init_tasklet(struct ttusb_dec *dec)
 		     (unsigned long)dec);
 }
 
-static int ttusb_init_rc( struct ttusb_dec *dec)
+static int ttusb_init_rc(struct ttusb_dec *dec)
 {
 	struct input_dev *input_dev;
 	u8 b[] = { 0x00, 0x01 };
-- 
2.14.1