[PATCH 0/5] [media] s2255drv: Fine-tuning for some function implementations

[PATCH 0/5] [media] s2255drv: Fine-tuning for some function implementations

[SF Markus Elfring]

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

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

Markus Elfring (5):
  Delete three error messages for a failed memory allocation in s2255_probe()
  Adjust 13 checks for null pointers
  Improve two size determinations in s2255_probe()
  Use common error handling code in read_pipe_completion()
  Delete an unnecessary return statement in five functions

 drivers/media/usb/s2255/s2255drv.c | 64 ++++++++++++++++----------------------
 1 file changed, 26 insertions(+), 38 deletions(-)

-- 
2.14.1

[PATCH 1/5] [media] s2255drv: Delete three error messages for a failed memory allocation in s2255_probe()

[SF Markus Elfring]

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

Omit extra messages for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/media/usb/s2255/s2255drv.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/media/usb/s2255/s2255drv.c b/drivers/media/usb/s2255/s2255drv.c
index b2f239c4ba42..29285e8cd742 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -2242,13 +2242,9 @@ static int s2255_probe(struct usb_interface *interface,
-	if (dev == NULL) {
-		s2255_dev_err(&interface->dev, "out of memory\n");
+	if (!dev)
 		return -ENOMEM;
-	}
 
 	dev->cmdbuf = kzalloc(S2255_CMDBUF_SIZE, GFP_KERNEL);
-	if (dev->cmdbuf == NULL) {
-		s2255_dev_err(&interface->dev, "out of memory\n");
+	if (!dev->cmdbuf)
 		goto errorFWDATA1;
-	}
 
 	atomic_set(&dev->num_channels, 0);
 	dev->pid = id->idProduct;
@@ -2303,7 +2299,6 @@ static int s2255_probe(struct usb_interface *interface,
-	if (!dev->fw_data->pfw_data) {
-		dev_err(&interface->dev, "out of memory!\n");
+	if (!dev->fw_data->pfw_data)
 		goto errorFWDATA2;
-	}
+
 	/* load the first chunk */
 	if (request_firmware(&dev->fw_data->fw,
 			     FIRMWARE_FILE_NAME, &dev->udev->dev)) {
-- 
2.14.1

[PATCH 2/5] [media] s2255drv: Adjust 13 checks for null pointers

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 20 Sep 2017 16:46:19 +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/s2255/s2255drv.c | 29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/drivers/media/usb/s2255/s2255drv.c b/drivers/media/usb/s2255/s2255drv.c
index 29285e8cd742..aee83bf6fa94 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -516,6 +516,6 @@ static void s2255_fwchunk_complete(struct urb *urb)
 		wake_up(&data->wait_fw);
 		return;
 	}
-	if (data->fw_urb == NULL) {
+	if (!data->fw_urb) {
 		s2255_dev_err(&udev->dev, "disconnected\n");
 		atomic_set(&data->fw_state, S2255_FW_FAILED);
@@ -680,5 +680,5 @@ static int buffer_prepare(struct vb2_buffer *vb)
 	dprintk(vc->dev, 4, "%s\n", __func__);
-	if (vc->fmt == NULL)
+	if (!vc->fmt)
 		return -EINVAL;
 
 	if ((w < norm_minw(vc)) ||
@@ -785,6 +785,5 @@ static int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
 	fmt = format_by_fourcc(f->fmt.pix.pixelformat);
-
-	if (fmt == NULL)
+	if (!fmt)
 		return -EINVAL;
 
 	field = f->fmt.pix.field;
@@ -853,6 +852,5 @@ static int vidioc_s_fmt_vid_cap(struct file *file, void *priv,
 	fmt = format_by_fourcc(f->fmt.pix.pixelformat);
-
-	if (fmt == NULL)
+	if (!fmt)
 		return -EINVAL;
 
 	if (vb2_is_busy(q)) {
@@ -936,6 +934,6 @@ static u32 get_transfer_size(struct s2255_mode *mode)
 	unsigned int mask_mult;
 
-	if (mode == NULL)
+	if (!mode)
 		return 0;
 
 	if (mode->format == FORMAT_NTSC) {
@@ -1390,4 +1388,4 @@ static int vidioc_enum_framesizes(struct file *file, void *priv,
 	fmt = format_by_fourcc(fe->pixel_format);
-	if (fmt == NULL)
+	if (!fmt)
 		return -EINVAL;
 	fe->type = V4L2_FRMSIZE_TYPE_DISCRETE;
@@ -1412,5 +1410,5 @@ static int vidioc_enum_frameintervals(struct file *file, void *priv,
 	fmt = format_by_fourcc(fe->pixel_format);
-	if (fmt == NULL)
+	if (!fmt)
 		return -EINVAL;
 
 	sizes = is_ntsc ? ntsc_sizes : pal_sizes;
@@ -1834,6 +1832,5 @@ static int save_frame(struct s2255_dev *dev, struct s2255_pipeinfo *pipe_info)
 	psrc = (u8 *)pipe_info->transfer_buffer + offset;
 
-
-	if (frm->lpvbits == NULL) {
+	if (!frm->lpvbits) {
 		dprintk(dev, 1, "s2255 frame buffer == NULL.%p %p %d %d",
 			frm, dev, dev->cc, idx);
@@ -1965,6 +1962,6 @@ static int s2255_create_sys_buffers(struct s2255_vc *vc)
 		vc->buffer.frame[i].lpvbits = vmalloc(reqsize);
 		vc->buffer.frame[i].size = reqsize;
-		if (vc->buffer.frame[i].lpvbits == NULL) {
+		if (!vc->buffer.frame[i].lpvbits) {
 			pr_info("out of memory.  using less frames\n");
 			vc->buffer.dwFrames = i;
 			break;
@@ -2007,6 +2004,6 @@ static int s2255_board_init(struct s2255_dev *dev)
 	pipe->transfer_buffer = kzalloc(pipe->max_transfer_size,
 					GFP_KERNEL);
-	if (pipe->transfer_buffer == NULL) {
+	if (!pipe->transfer_buffer) {
 		dprintk(dev, 1, "out of memory!\n");
 		return -ENOMEM;
 	}
@@ -2068,8 +2065,8 @@ static void read_pipe_completion(struct urb *purb)
 	pipe_info = purb->context;
-	if (pipe_info == NULL) {
+	if (!pipe_info) {
 		dev_err(&purb->dev->dev, "no context!\n");
 		return;
 	}
 	dev = pipe_info->dev;
-	if (dev == NULL) {
+	if (!dev) {
 		dev_err(&purb->dev->dev, "no context!\n");
@@ -2257,5 +2254,5 @@ static int s2255_probe(struct usb_interface *interface,
 	dev->udev = usb_get_dev(interface_to_usbdev(interface));
-	if (dev->udev == NULL) {
+	if (!dev->udev) {
 		dev_err(&interface->dev, "null usb device\n");
 		retval = -ENODEV;
 		goto errorUDEV;
-- 
2.14.1

Re: [PATCH 2/5] [media] s2255drv: Adjust 13 checks for null pointers

[Dan Carpenter]

On Wed, Sep 20, 2017 at 06:58:56PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 20 Sep 2017 16:46:19 +0200
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 

You've been told several times that this stuff doesn't work.  Try
applying this patch with `git am` and you'll see why.

regards,
dan carpenter

Re: [media] s2255drv: Adjust 13 checks for null pointers

[SF Markus Elfring]

>> MIME-Version: 1.0
>> Content-Type: text/plain; charset=UTF-8
>> Content-Transfer-Encoding: 8bit
>>
> 
> You've been told several times that this stuff doesn't work.

This functionality might not exactly work in the way that you expect so far.


> Try applying this patch with `git am` and you'll see why.

I find that these extra message fields work in the way that was designed
by the Git software developers.

elfring@Sonne:~/Projekte/Linux/next-patched> LANG=C git checkout -b next_deletion_of_oom_messages_in_s2255drv_test_20170921 next_deletion_of_oom_messages-20170905 && LANG=C git am '../[PATCH 2_5] [media] s2255drv: Adjust 13 checks for null pointers.eml'
Switched to a new branch 'next_deletion_of_oom_messages_in_s2255drv_test_20170921'
Applying: s2255drv: Adjust 13 checks for null pointers


Would you like to clarify corresponding concerns any more?

Regards,
Markus

Re: [media] s2255drv: Adjust 13 checks for null pointers

[Dan Carpenter]

On Thu, Sep 21, 2017 at 10:12:56AM +0200, SF Markus Elfring wrote:
> >> MIME-Version: 1.0
> >> Content-Type: text/plain; charset=UTF-8
> >> Content-Transfer-Encoding: 8bit
> >>
> > 
> > You've been told several times that this stuff doesn't work.
> 
> This functionality might not exactly work in the way that you expect so far.
> 
> 
> > Try applying this patch with `git am` and you'll see why.
> 
> I find that these extra message fields work in the way that was designed
> by the Git software developers.
> 
> elfring@Sonne:~/Projekte/Linux/next-patched> LANG=C git checkout -b next_deletion_of_oom_messages_in_s2255drv_test_20170921 next_deletion_of_oom_messages-20170905 && LANG=C git am '../[PATCH 2_5] [media] s2255drv: Adjust 13 checks for null pointers.eml'
> Switched to a new branch 'next_deletion_of_oom_messages_in_s2255drv_test_20170921'
> Applying: s2255drv: Adjust 13 checks for null pointers
> 
> 
> Would you like to clarify corresponding concerns any more?
> 

Look at the `git log` and it just copies those lines:

commit 2a47170a824697783d8c2d28355a806f075c76e4 (HEAD)
Author: Markus Elfring <elfring@users.sourceforge.net>
Date:   Wed Sep 20 16:46:19 2017 +0200

    s2255drv: Adjust 13 checks for null pointers

    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.


regards,
dan carpenter

Re: [media] s2255drv: Adjust 13 checks for null pointers

[SF Markus Elfring]

>> Would you like to clarify corresponding concerns any more?
>>
> 
> Look at the `git log`

I did this also for a moment.


> and it just copies those lines:

The Git software preserves these three message fields
(when special characters were used in the commit message).

Can you accept such software functionality?

Regards,
Markus

[PATCH 3/5] [media] s2255drv: Improve two size determinations in s2255_probe()

[SF Markus Elfring]

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

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 <elfring@users.sourceforge.net>
---
 drivers/media/usb/s2255/s2255drv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/s2255/s2255drv.c b/drivers/media/usb/s2255/s2255drv.c
index aee83bf6fa94..29bc73ad7d8a 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -2237,4 +2237,4 @@ static int s2255_probe(struct usb_interface *interface,
 	/* allocate memory for our device state and initialize it to zero */
-	dev = kzalloc(sizeof(struct s2255_dev), GFP_KERNEL);
+	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
 	if (!dev)
 		return -ENOMEM;
@@ -2247,4 +2247,4 @@ static int s2255_probe(struct usb_interface *interface,
 	dev->pid = id->idProduct;
-	dev->fw_data = kzalloc(sizeof(struct s2255_fw), GFP_KERNEL);
+	dev->fw_data = kzalloc(sizeof(*dev->fw_data), GFP_KERNEL);
 	if (!dev->fw_data)
 		goto errorFWDATA1;
-- 
2.14.1

[PATCH 4/5] [media] s2255drv: Use common error handling code in read_pipe_completion()

[SF Markus Elfring]

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

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

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/media/usb/s2255/s2255drv.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/media/usb/s2255/s2255drv.c b/drivers/media/usb/s2255/s2255drv.c
index 29bc73ad7d8a..5a5d5ae833ff 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -2065,11 +2065,9 @@ static void read_pipe_completion(struct urb *purb)
 	pipe_info = purb->context;
-	if (!pipe_info) {
-		dev_err(&purb->dev->dev, "no context!\n");
-		return;
-	}
+	if (!pipe_info)
+		goto report_failure;
+
 	dev = pipe_info->dev;
-	if (!dev) {
-		dev_err(&purb->dev->dev, "no context!\n");
-		return;
-	}
+	if (!dev)
+		goto report_failure;
+
 	status = purb->status;
@@ -2107,6 +2105,9 @@ static void read_pipe_completion(struct urb *purb)
 		dprintk(dev, 2, "%s :complete state 0\n", __func__);
 	}
 	return;
+
+report_failure:
+	dev_err(&purb->dev->dev, "no context!\n");
 }
 
 static int s2255_start_readpipe(struct s2255_dev *dev)
-- 
2.14.1

[PATCH 5/5] [media] s2255drv: Delete an unnecessary return statement in five functions

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 20 Sep 2017 17:50:36 +0200

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

WARNING: void function return statements are not generally useful

Thus remove such a statement in the affected functions.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/media/usb/s2255/s2255drv.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/media/usb/s2255/s2255drv.c b/drivers/media/usb/s2255/s2255drv.c
index 5a5d5ae833ff..2f0e0fafc4e2 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -471,6 +471,5 @@ static void planar422p_to_yuv_packed(const unsigned char *in,
 		out[i + 3] = (fmt == V4L2_PIX_FMT_YUYV) ? *pCb++ : *pY++;
 	}
-	return;
 }
 
 static void s2255_reset_dsppower(struct s2255_dev *dev)
@@ -482,5 +481,4 @@ static void s2255_reset_dsppower(struct s2255_dev *dev)
 	s2255_vendor_req(dev, 0x10, 0x0000, 0x0000, NULL, 0, 1);
-	return;
 }
 
 /* kickstarts the firmware loading. from probe
@@ -1586,6 +1584,5 @@ static void s2255_video_device_release(struct video_device *vdev)
 	if (atomic_dec_and_test(&dev->num_channels))
 		s2255_destroy(dev);
-	return;
 }
 
 static const struct video_device template = {
@@ -1890,5 +1887,4 @@ static void s2255_read_video_callback(struct s2255_dev *dev,
 	dprintk(dev, 50, "callback read video done\n");
-	return;
 }
 
 static long s2255_vendor_req(struct s2255_dev *dev, unsigned char Request,
@@ -2205,5 +2201,4 @@ static void s2255_stop_readpipe(struct s2255_dev *dev)
 	dprintk(dev, 4, "%s", __func__);
-	return;
 }
 
 static void s2255_fwload_start(struct s2255_dev *dev, int reset)
-- 
2.14.1