s2255drv: Fine-tuning for some function implementations
Subsystems (2)
cover s2255drv: Fine-tuning for some function implementations
From: Markus Elfring <[email protected]>
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(-)
Signature · 1 line
2.14.1
PATCH 1/5 s2255drv: Delete three error messages for a failed memory allocation in s2255_probe()
From: Markus Elfring <[email protected]>
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.
--- drivers/media/usb/s2255/s2255drv.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-)
drivers/media/usb/s2255/s2255drv.c +4 -9
@@ -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)) {
Signature · 1 line
2.14.1
PATCH 2/5 s2255drv: Adjust 13 checks for null pointers
In reply to SF Markus ElfringFrom: Markus Elfring <[email protected]>
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.
--- drivers/media/usb/s2255/s2255drv.c | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-)
drivers/media/usb/s2255/s2255drv.c +13 -16
@@ -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;
Signature · 1 line
2.14.1
Re: [PATCH 2/5] [media] s2255drv: Adjust 13 checks for null pointers
Quoted 6 lines · SF Markus Elfring
From: Markus Elfring <[email protected]>
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
Quoted 6 lines
Quoted 4 lines
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bitYou'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.
Quoted 1 line
Try applying this patch with
git amand 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
Quoted 15 lines · SF Markus Elfring
Quoted 6 lines
Quoted 4 lines
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bitYou'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.
Quoted 1 line
Try applying this patch with
git amand you'll see why.I find that these extra message fields work in the way that was designed by the Git software developers.
Quoted 7 lines
elfring@Sonne:~/Projekte/Linux/next-patched> LANG=C git checkout -b
next_deletion_of_oom_messages_in_s2255drv_test_20170921next_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 pointersWould 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 <[email protected]> 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
Quoted 4 lines
Quoted 2 lines
Would you like to clarify corresponding concerns any more?
Look at the
git log
I did this also for a moment.
Quoted 1 line
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 s2255drv: Improve two size determinations in s2255_probe()
In reply to SF Markus ElfringFrom: Markus Elfring <[email protected]>
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.
--- drivers/media/usb/s2255/s2255drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/media/usb/s2255/s2255drv.c +2 -2
@@ -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;
Signature · 1 line
2.14.1
PATCH 4/5 s2255drv: Use common error handling code in read_pipe_completion()
In reply to SF Markus ElfringFrom: Markus Elfring <[email protected]>
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.
--- drivers/media/usb/s2255/s2255drv.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)
drivers/media/usb/s2255/s2255drv.c +9 -8
@@ -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)
Signature · 1 line
2.14.1
PATCH 5/5 s2255drv: Delete an unnecessary return statement in five functions
In reply to SF Markus ElfringFrom: Markus Elfring <[email protected]>
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.
--- drivers/media/usb/s2255/s2255drv.c | 5 ----- 1 file changed, 5 deletions(-)
drivers/media/usb/s2255/s2255drv.c +0 -5
@@ -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)
Signature · 1 line
2.14.1