Re: [v3] [media] Use common error handling code in 19 functions

[SF Markus Elfring]

> @@ -656,18 +656,18 @@  static int dvb_dmxdev_start_feed(struct dmxdev *dmxdev,
>   tsfeed->priv = filter;
>  
>   ret = tsfeed->set(tsfeed, feed->pid, ts_type, ts_pes, timeout);
> - if (ret < 0) {
> - dmxdev->demux->release_ts_feed(dmxdev->demux, tsfeed);
> - return ret;
> - }
> + if (ret < 0)
> + goto release_feed;
>  
>   ret = tsfeed->start_filtering(tsfeed);
> - if (ret < 0) {
> - dmxdev->demux->release_ts_feed(dmxdev->demux, tsfeed);
> - return ret;
> - }
> + if (ret < 0)
> + goto release_feed;
>  
>   return 0;
> +
> +release_feed:
> + dmxdev->demux->release_ts_feed(dmxdev->demux, tsfeed);
> + return ret;
>  }
> 
> There's *nothing* wrong with the above. It works fine,

I can agree to this view in principle according to the required control flow.


> avoids goto

How does this wording fit to information from the section
“7) Centralized exiting of functions” in the document “coding-style.rst”?


> and probably even produce the same code, as gcc will likely optimize it.

Would you like to clarify the current situation around supported
software optimisations any more?


> It is also easier to review, as the error handling is closer
> to the code.

Do we stumble on different coding style preferences once more?


> On the other hand, there's nothing wrong on taking the approach
> you're proposing.

Thanks for another bit of positive feedback.


> In the end, using goto or not on error handling like the above is 
> a matter of personal taste - and taste changes with time

Do Linux guidelines need any adjustments?


> and with developer. I really don't have time to keep reviewing patches
> that are just churning the code just due to someone's personal taste.

I tried to apply another general source code transformation pattern.


> I'm pretty sure if I start accepting things like that,
> someone else would be on some future doing patches just reverting it,
> and I would be likely having to apply them too.

Why?

I hope also that the source code can be kept consistent to some degree.


> So, except if the patch is really fixing something - e.g. a broken
> error handling code, I'll just ignore such patches and mark as
> rejected without further notice/comments from now on.

I would find such a communication style questionable.
Do you distinguish between bug fixes and possible corrections for
other error categories (or software weaknesses)?

Regards,
Markus

Re: [v3] [media] Use common error handling code in 19 functions

[SF Markus Elfring]

> Adjust jump targets so that a bit of exception handling can be better
> reused at the end of these functions.

Why was this update suggestion rejected once more a moment ago?

https://patchwork.linuxtv.org/patch/47827/
lkml.kernel.org/r/<57ef3a56-2578-1d5f-1268-348b49b0c...@users.sourceforge.net>
https://lkml.org/lkml/2018/3/9/823

Would you like to integrate such a source code transformation after any further 
adjustments?

Regards,
Markus

Re: [media] ov5645: Move an error code assignment in ov5645_probe()

[SF Markus Elfring]

>> Move an assignment for a specific error code so that it is stored only once
>> in this function implementation.
>>
>> This issue was detected by using the Coccinelle software.
> 
> How?

Would you like to experiment a bit more with the following approach
for the semantic patch language?

show_same_statements3.cocci:

@duplicated_code@
identifier work;
statement s1, s2;
type T;
@@
 T work(...)
 {
 ... when any
*if ((...) < 0)
*{
...
*   s1
*   s2
*}
 ... when any
*if ((...) < 0)
*{
...
*   s1
*   s2
*}
 ... when any
 }


>> @@ -1334,6 +1329,7 @@ static int ov5645_probe(struct i2c_client *client,
>>  
>>  power_down:
>>  ov5645_s_power(>sd, false);
>> +ret = -ENODEV;
> 
> I don't think this is where people would expect you to set the error code
> in general.

This can be. - The view depends on some factors.


> It should rather take place before goto, not after it.

I proposed another software design direction.


> That'd mean another variable,

To which detail do you refer here?


> and I'm not convinced the result would improve the driver.

Can you see the relevance of a small code reduction in this function?

Regards,
Markus

[PATCH] [media] ov5645: Move an error code assignment in ov5645_probe()

[SF Markus Elfring]

From: Markus Elfring 
Date: Wed, 14 Mar 2018 22:02:52 +0100

Move an assignment for a specific error code so that it is stored only once
in this function implementation.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/media/i2c/ov5645.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/media/i2c/ov5645.c b/drivers/media/i2c/ov5645.c
index d28845f7356f..374576380fd4 100644
--- a/drivers/media/i2c/ov5645.c
+++ b/drivers/media/i2c/ov5645.c
@@ -1284,13 +1284,11 @@ static int ov5645_probe(struct i2c_client *client,
ret = ov5645_read_reg(ov5645, OV5645_CHIP_ID_HIGH, _id_high);
if (ret < 0 || chip_id_high != OV5645_CHIP_ID_HIGH_BYTE) {
dev_err(dev, "could not read ID high\n");
-   ret = -ENODEV;
goto power_down;
}
ret = ov5645_read_reg(ov5645, OV5645_CHIP_ID_LOW, _id_low);
if (ret < 0 || chip_id_low != OV5645_CHIP_ID_LOW_BYTE) {
dev_err(dev, "could not read ID low\n");
-   ret = -ENODEV;
goto power_down;
}
 
@@ -1300,7 +1298,6 @@ static int ov5645_probe(struct i2c_client *client,
  >aec_pk_manual);
if (ret < 0) {
dev_err(dev, "could not read AEC/AGC mode\n");
-   ret = -ENODEV;
goto power_down;
}
 
@@ -1308,7 +1305,6 @@ static int ov5645_probe(struct i2c_client *client,
  >timing_tc_reg20);
if (ret < 0) {
dev_err(dev, "could not read vflip value\n");
-   ret = -ENODEV;
goto power_down;
}
 
@@ -1316,7 +1312,6 @@ static int ov5645_probe(struct i2c_client *client,
  >timing_tc_reg21);
if (ret < 0) {
dev_err(dev, "could not read hflip value\n");
-   ret = -ENODEV;
goto power_down;
}
 
@@ -1334,6 +1329,7 @@ static int ov5645_probe(struct i2c_client *client,
 
 power_down:
ov5645_s_power(>sd, false);
+   ret = -ENODEV;
 free_entity:
media_entity_cleanup(>sd.entity);
 free_ctrl:
-- 
2.16.2

[PATCH v3] [media] Use common error handling code in 19 functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Fri, 9 Mar 2018 21:00:12 +0100

Adjust jump targets so that a bit of exception handling can be better
reused at the end of these functions.

This issue was partly detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---

v3:
Laurent Pinchart and Todor Tomov requested a few adjustments.
Updates were rebased on source files from Linux next-20180308.

v2:
Hans Verkuil insisted on patch squashing. Thus several changes
were recombined based on source files from Linux next-20180216.

The implementation of the function "tda8261_set_params" was improved
after a notification by Christoph Böhmwalder on 2017-09-26.

 drivers/media/dvb-core/dmxdev.c| 16 
 drivers/media/dvb-frontends/tda1004x.c | 20 ++
 drivers/media/dvb-frontends/tda8261.c  | 19 ++
 drivers/media/pci/bt8xx/dst.c  | 19 ++
 drivers/media/pci/bt8xx/dst_ca.c   | 30 +++
 drivers/media/pci/cx88/cx88-input.c| 17 +
 drivers/media/platform/omap3isp/ispvideo.c | 28 ++
 .../media/platform/qcom/camss-8x16/camss-csid.c| 19 +-
 drivers/media/tuners/tuner-xc2028.c| 30 +++
 drivers/media/usb/cpia2/cpia2_usb.c| 13 ---
 drivers/media/usb/gspca/gspca.c| 17 +
 drivers/media/usb/gspca/sn9c20x.c  | 17 +
 drivers/media/usb/pvrusb2/pvrusb2-ioread.c | 10 +++--
 drivers/media/usb/tm6000/tm6000-cards.c|  7 ++--
 drivers/media/usb/tm6000/tm6000-dvb.c  | 11 --
 drivers/media/usb/tm6000/tm6000-video.c| 13 ---
 drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c  | 13 +++
 drivers/media/usb/ttusb-dec/ttusb_dec.c| 43 --
 18 files changed, 171 insertions(+), 171 deletions(-)

diff --git a/drivers/media/dvb-core/dmxdev.c b/drivers/media/dvb-core/dmxdev.c
index 61a750fae465..17d05b05fa9d 100644
--- a/drivers/media/dvb-core/dmxdev.c
+++ b/drivers/media/dvb-core/dmxdev.c
@@ -656,18 +656,18 @@ static int dvb_dmxdev_start_feed(struct dmxdev *dmxdev,
tsfeed->priv = filter;
 
ret = tsfeed->set(tsfeed, feed->pid, ts_type, ts_pes, timeout);
-   if (ret < 0) {
-   dmxdev->demux->release_ts_feed(dmxdev->demux, tsfeed);
-   return ret;
-   }
+   if (ret < 0)
+   goto release_feed;
 
ret = tsfeed->start_filtering(tsfeed);
-   if (ret < 0) {
-   dmxdev->demux->release_ts_feed(dmxdev->demux, tsfeed);
-   return ret;
-   }
+   if (ret < 0)
+   goto release_feed;
 
return 0;
+
+release_feed:
+   dmxdev->demux->release_ts_feed(dmxdev->demux, tsfeed);
+   return ret;
 }
 
 static int dvb_dmxdev_filter_start(struct dmxdev_filter *filter)
diff --git a/drivers/media/dvb-frontends/tda1004x.c 
b/drivers/media/dvb-frontends/tda1004x.c
index 58e3beff5adc..85ca111fc8c4 100644
--- a/drivers/media/dvb-frontends/tda1004x.c
+++ b/drivers/media/dvb-frontends/tda1004x.c
@@ -1299,20 +1299,22 @@ struct dvb_frontend* tda10045_attach(const struct 
tda1004x_config* config,
id = tda1004x_read_byte(state, TDA1004X_CHIPID);
if (id < 0) {
printk(KERN_ERR "tda10045: chip is not answering. Giving 
up.\n");
-   kfree(state);
-   return NULL;
+   goto free_state;
}
 
if (id != 0x25) {
printk(KERN_ERR "Invalid tda1004x ID = 0x%02x. Can't 
proceed\n", id);
-   kfree(state);
-   return NULL;
+   goto free_state;
}
 
/* create dvb_frontend */
memcpy(>frontend.ops, _ops, sizeof(struct 
dvb_frontend_ops));
state->frontend.demodulator_priv = state;
return >frontend;
+
+free_state:
+   kfree(state);
+   return NULL;
 }
 
 static const struct dvb_frontend_ops tda10046_ops = {
@@ -1369,19 +1371,21 @@ struct dvb_frontend* tda10046_attach(const struct 
tda1004x_config* config,
id = tda1004x_read_byte(state, TDA1004X_CHIPID);
if (id < 0) {
printk(KERN_ERR "tda10046: chip is not answering. Giving 
up.\n");
-   kfree(state);
-   return NULL;
+   goto free_state;
}
if (id != 0x46) {
printk(KERN_ERR "Invalid tda1004x ID = 0x%02x. Can't 
proceed\n", id);
-   kfree(state);
-   return NULL;
+   goto free_state;
}
 
/* create dvb_frontend */
memcpy(>frontend.ops, _ops, sizeof(struct 
dvb_frontend_ops));
state->frontend.demodulator_priv = state;
return >frontend;
+
+free_state:
+   kfree(state);
+   return NULL;
 }
 
 module_param(debug, int, 0644);
diff --git 

Re: [v2] [media] Use common error handling code in 20 functions

[SF Markus Elfring]

I got the following notification.

 - http://patchwork.linuxtv.org/patch/47270/
 - for: Linux Media kernel patches
was: New
now: Rejected


Would you like to clarify still any other variant for this change proposal?

Regards,
Markus

Re: [v2] [media] Use common error handling code in 20 functions

[SF Markus Elfring]

>> +put_isp:
>> +omap3isp_put(video->isp);
>> +delete_fh:
>> +v4l2_fh_del(>vfh);
>> +v4l2_fh_exit(>vfh);
>> +kfree(handle);
> 
> Please prefix the error labels with error_.

How often do you really need such an extra prefix?


>> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
>> @@ -994,10 +994,8 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file,
>> void *fh, struct v4l2_queryctrl qc = { .id = ctrl->id };
>>
>>  ret = uvc_query_v4l2_ctrl(chain, );
>> -if (ret < 0) {
>> -ctrls->error_idx = i;
>> -return ret;
>> -}
>> +if (ret < 0)
>> +goto set_index;
>>
>>  ctrl->value = qc.default_value;
>>  }
>> @@ -1013,14 +1011,17 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file,
>> void *fh, ret = uvc_ctrl_get(chain, ctrl);
>>  if (ret < 0) {
>>  uvc_ctrl_rollback(handle);
>> -ctrls->error_idx = i;
>> -return ret;
>> +goto set_index;
>>  }
>>  }
>>
>>  ctrls->error_idx = 0;
>>
>>  return uvc_ctrl_rollback(handle);
>> +
>> +set_index:
>> +ctrls->error_idx = i;
>> +return ret;
>>  }
> 
> For uvcvideo I find this to hinder readability

I got an other development view.


> without adding much added value.

There can be a small effect for such a function implementation.


> Please drop the uvcvideo change from this patch.

Would it be nice if this source code adjustment could be integrated also?

Regards,
Markus

[PATCH v2] [media] Delete unnecessary variable initialisations in seven functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Thu, 22 Feb 2018 21:45:47 +0100

Some local variables will be set to an appropriate value before usage.
Thus omit explicit initialisations at the beginning of these functions.

Signed-off-by: Markus Elfring 
---

v2:
Hans Verkuil insisted on patch squashing. Thus some changes
were recombined based on source files from Linux next-20180216.

 drivers/media/radio/radio-mr800.c | 2 +-
 drivers/media/radio/radio-wl1273.c| 2 +-
 drivers/media/radio/si470x/radio-si470x-usb.c | 2 +-
 drivers/media/usb/cx231xx/cx231xx-cards.c | 2 +-
 drivers/media/usb/cx231xx/cx231xx-dvb.c   | 2 +-
 drivers/media/usb/go7007/snd-go7007.c | 2 +-
 drivers/media/usb/tm6000/tm6000-cards.c   | 2 +-
 7 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/media/radio/radio-mr800.c 
b/drivers/media/radio/radio-mr800.c
index dc6c4f985911..0f292c6ba338 100644
--- a/drivers/media/radio/radio-mr800.c
+++ b/drivers/media/radio/radio-mr800.c
@@ -511,5 +511,5 @@ static int usb_amradio_probe(struct usb_interface *intf,
const struct usb_device_id *id)
 {
struct amradio_device *radio;
-   int retval = 0;
+   int retval;
 
diff --git a/drivers/media/radio/radio-wl1273.c 
b/drivers/media/radio/radio-wl1273.c
index 58e944591602..8f9f8dfc3497 100644
--- a/drivers/media/radio/radio-wl1273.c
+++ b/drivers/media/radio/radio-wl1273.c
@@ -671,6 +671,6 @@ static int wl1273_fm_start(struct wl1273_device *radio, int 
new_mode)
 static int wl1273_fm_suspend(struct wl1273_device *radio)
 {
struct wl1273_core *core = radio->core;
-   int r = 0;
+   int r;
 
/* Cannot go from OFF to SUSPENDED */
diff --git a/drivers/media/radio/si470x/radio-si470x-usb.c 
b/drivers/media/radio/si470x/radio-si470x-usb.c
index c311f9951d80..2277e850bb5e 100644
--- a/drivers/media/radio/si470x/radio-si470x-usb.c
+++ b/drivers/media/radio/si470x/radio-si470x-usb.c
@@ -578,6 +578,6 @@ static int si470x_usb_driver_probe(struct usb_interface 
*intf,
struct si470x_device *radio;
struct usb_host_interface *iface_desc;
struct usb_endpoint_descriptor *endpoint;
-   int i, int_end_size, retval = 0;
+   int i, int_end_size, retval;
unsigned char version_warning = 0;
 
diff --git a/drivers/media/usb/cx231xx/cx231xx-cards.c 
b/drivers/media/usb/cx231xx/cx231xx-cards.c
index f9ec7fedcd5b..14e3814f55d9 100644
--- a/drivers/media/usb/cx231xx/cx231xx-cards.c
+++ b/drivers/media/usb/cx231xx/cx231xx-cards.c
@@ -1135,6 +1135,6 @@ static void cx231xx_config_tuner(struct cx231xx *dev)
 static int read_eeprom(struct cx231xx *dev, struct i2c_client *client,
   u8 *eedata, int len)
 {
-   int ret = 0;
+   int ret;
u8 start_offset = 0;
int len_todo = len;
diff --git a/drivers/media/usb/cx231xx/cx231xx-dvb.c 
b/drivers/media/usb/cx231xx/cx231xx-dvb.c
index fb5654062b1a..24ca011c49bb 100644
--- a/drivers/media/usb/cx231xx/cx231xx-dvb.c
+++ b/drivers/media/usb/cx231xx/cx231xx-dvb.c
@@ -604,6 +604,6 @@ static void unregister_dvb(struct cx231xx_dvb *dvb)
 
 static int dvb_init(struct cx231xx *dev)
 {
-   int result = 0;
+   int result;
struct cx231xx_dvb *dvb;
struct i2c_adapter *tuner_i2c;
diff --git a/drivers/media/usb/go7007/snd-go7007.c 
b/drivers/media/usb/go7007/snd-go7007.c
index c618764480c6..f84a2130f033 100644
--- a/drivers/media/usb/go7007/snd-go7007.c
+++ b/drivers/media/usb/go7007/snd-go7007.c
@@ -227,7 +227,7 @@ int go7007_snd_init(struct go7007 *go)
 {
static int dev;
struct go7007_snd *gosnd;
-   int ret = 0;
+   int ret;
 
if (dev >= SNDRV_CARDS)
return -ENODEV;
diff --git a/drivers/media/usb/tm6000/tm6000-cards.c 
b/drivers/media/usb/tm6000/tm6000-cards.c
index 4d5f4cc4887e..70939e96b856 100644
--- a/drivers/media/usb/tm6000/tm6000-cards.c
+++ b/drivers/media/usb/tm6000/tm6000-cards.c
@@ -1174,7 +1174,7 @@ static int tm6000_usb_probe(struct usb_interface 
*interface,
 {
struct usb_device *usbdev;
struct tm6000_core *dev;
-   int i, rc = 0;
+   int i, rc;
int nr = 0;
char *speed;
 
-- 
2.16.2

[PATCH v2] [media] Use common error handling code in 20 functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 19 Feb 2018 18:50:40 +0100

Adjust jump targets so that a bit of exception handling can be better
reused at the end of these functions.

This issue was partly detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---

v2:
Hans Verkuil insisted on patch squashing. Thus several changes
were recombined based on source files from Linux next-20180216.

The implementation of the function "tda8261_set_params" was improved
after a notification by Christoph Böhmwalder on 2017-09-26.

 drivers/media/dvb-core/dmxdev.c| 16 
 drivers/media/dvb-frontends/tda1004x.c | 20 ++
 drivers/media/dvb-frontends/tda8261.c  | 19 ++
 drivers/media/pci/bt8xx/dst.c  | 19 ++
 drivers/media/pci/bt8xx/dst_ca.c   | 30 +++
 drivers/media/pci/cx88/cx88-input.c| 17 +
 drivers/media/platform/omap3isp/ispvideo.c | 29 +++
 .../media/platform/qcom/camss-8x16/camss-csid.c| 20 +-
 drivers/media/tuners/tuner-xc2028.c| 30 +++
 drivers/media/usb/cpia2/cpia2_usb.c| 13 ---
 drivers/media/usb/gspca/gspca.c| 17 +
 drivers/media/usb/gspca/sn9c20x.c  | 17 +
 drivers/media/usb/pvrusb2/pvrusb2-ioread.c | 10 +++--
 drivers/media/usb/tm6000/tm6000-cards.c|  7 ++--
 drivers/media/usb/tm6000/tm6000-dvb.c  | 11 --
 drivers/media/usb/tm6000/tm6000-video.c| 13 ---
 drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c  | 13 +++
 drivers/media/usb/ttusb-dec/ttusb_dec.c| 43 --
 drivers/media/usb/uvc/uvc_v4l2.c   | 13 ---
 19 files changed, 180 insertions(+), 177 deletions(-)

diff --git a/drivers/media/dvb-core/dmxdev.c b/drivers/media/dvb-core/dmxdev.c
index 6d53af00190e..6a0411c91195 100644
--- a/drivers/media/dvb-core/dmxdev.c
+++ b/drivers/media/dvb-core/dmxdev.c
@@ -645,18 +645,18 @@ static int dvb_dmxdev_start_feed(struct dmxdev *dmxdev,
tsfeed->priv = filter;
 
ret = tsfeed->set(tsfeed, feed->pid, ts_type, ts_pes, timeout);
-   if (ret < 0) {
-   dmxdev->demux->release_ts_feed(dmxdev->demux, tsfeed);
-   return ret;
-   }
+   if (ret < 0)
+   goto release_feed;
 
ret = tsfeed->start_filtering(tsfeed);
-   if (ret < 0) {
-   dmxdev->demux->release_ts_feed(dmxdev->demux, tsfeed);
-   return ret;
-   }
+   if (ret < 0)
+   goto release_feed;
 
return 0;
+
+release_feed:
+   dmxdev->demux->release_ts_feed(dmxdev->demux, tsfeed);
+   return ret;
 }
 
 static int dvb_dmxdev_filter_start(struct dmxdev_filter *filter)
diff --git a/drivers/media/dvb-frontends/tda1004x.c 
b/drivers/media/dvb-frontends/tda1004x.c
index 58e3beff5adc..85ca111fc8c4 100644
--- a/drivers/media/dvb-frontends/tda1004x.c
+++ b/drivers/media/dvb-frontends/tda1004x.c
@@ -1299,20 +1299,22 @@ struct dvb_frontend* tda10045_attach(const struct 
tda1004x_config* config,
id = tda1004x_read_byte(state, TDA1004X_CHIPID);
if (id < 0) {
printk(KERN_ERR "tda10045: chip is not answering. Giving 
up.\n");
-   kfree(state);
-   return NULL;
+   goto free_state;
}
 
if (id != 0x25) {
printk(KERN_ERR "Invalid tda1004x ID = 0x%02x. Can't 
proceed\n", id);
-   kfree(state);
-   return NULL;
+   goto free_state;
}
 
/* create dvb_frontend */
memcpy(>frontend.ops, _ops, sizeof(struct 
dvb_frontend_ops));
state->frontend.demodulator_priv = state;
return >frontend;
+
+free_state:
+   kfree(state);
+   return NULL;
 }
 
 static const struct dvb_frontend_ops tda10046_ops = {
@@ -1369,19 +1371,21 @@ struct dvb_frontend* tda10046_attach(const struct 
tda1004x_config* config,
id = tda1004x_read_byte(state, TDA1004X_CHIPID);
if (id < 0) {
printk(KERN_ERR "tda10046: chip is not answering. Giving 
up.\n");
-   kfree(state);
-   return NULL;
+   goto free_state;
}
if (id != 0x46) {
printk(KERN_ERR "Invalid tda1004x ID = 0x%02x. Can't 
proceed\n", id);
-   kfree(state);
-   return NULL;
+   goto free_state;
}
 
/* create dvb_frontend */
memcpy(>frontend.ops, _ops, sizeof(struct 
dvb_frontend_ops));
state->frontend.demodulator_priv = state;
return >frontend;
+
+free_state:
+   kfree(state);
+   return NULL;
 }
 
 module_param(debug, int, 0644);
diff --git a/drivers/media/dvb-frontends/tda8261.c 
b/drivers/media/dvb-frontends/tda8261.c
index 

Re: Adjustments for a lot of function implementations

[SF Markus Elfring]

>> Do any contributors get into the mood to take another look at software 
>> updates
>> from my selection of change possibilities in a more constructive way?
>>
>> Do you need any additional development resources?
> 
> One last time: either post per-driver patches with all the cleanups for a 
> driver
> in a single patch,

I find such a change combination unsafe.


> or a per-directory patch (drivers/media/pci, usb, etc) doing the same cleanup
> for all drivers in that directory.

Would you dare to apply any (of my) scripts for the semantic patch language
directly on the whole directory for multi-media software?


> I prefer the first approach, but it's up to you.

Can you handle bigger patches really better than similar patch series?


> We don't have the time to wade through dozens of one-liner cleanup patches.

Are there any further possibilities to consider around consequences
from a general change resistance?

Will any development (or management) tools like “quilt fold” make the regrouping
of possible update steps more convenient and safer? 

Regards,
Markus

Re: Adjustments for a lot of function implementations

[SF Markus Elfring]

> One last time: either post per-driver patches with all the cleanups for a 
> driver
> in a single patch,

I preferred to offer source code adjustments according to specific 
transformation
patterns mostly for each software module separately (also in small patch 
series).


> or a per-directory patch (drivers/media/pci, usb, etc) doing the same cleanup
> for all drivers in that directory.

I am curious if bigger patch packages would be easier to get accepted.

Or would you get frightened still by any other change combination?



> I prefer the first approach,

We have got different preferences for a safe patch granularity.


> but it's up to you.

I imagine that there are more development factors involved.


> We don't have the time to wade through dozens of one-liner cleanup patches.

It is usual that integration of update suggestions will take some time.
How would the situation change if I would dare to regroup possible update steps?


> I don't understand what is so difficult about this.

There are communication difficulties to consider since your terse information
from your conference meeting.

If you would insist on patch squashing, would you dare to use a development tool
like “quilt fold” also on your own once more?

Regards,
Markus

Re: Adjustments for a lot of function implementations

[SF Markus Elfring]

> ??? I did that: either one patch per directory with the same type of change,
> or one patch per driver combining all the changes for that driver.

Do any contributors get into the mood to take another look at software updates
from my selection of change possibilities in a more constructive way?

Do you need any additional development resources?

Regards,
Markus

Re: Adjustments for a lot of function implementations

[SF Markus Elfring]

> ??? I did that: either one patch per directory with the same type of change,
> or one patch per driver combining all the changes for that driver.

Are you going to answer any of my remaining questions in a more constructive 
way?

Regards,
Markus

Re: Adjustments for a lot of function implementations

[SF Markus Elfring]

> ??? I did that: either one patch per directory with the same type of change,
> or one patch per driver combining all the changes for that driver.

Would you like to answer my still remaining questions in any more
constructive ways?

Regards,
Markus

[PATCH] staging/media/davinci_vpfe: Use common error handling code in vpfe_attach_irq()

[SF Markus Elfring]

From: Markus Elfring 
Date: Fri, 3 Nov 2017 10:45:31 +0100

Add a jump target 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 
---
 drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c 
b/drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c
index bffe2153b910..80297d2df31d 100644
--- a/drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c
+++ b/drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c
@@ -309,8 +309,7 @@ static int vpfe_attach_irq(struct vpfe_device *vpfe_dev)
if (ret < 0) {
v4l2_err(_dev->v4l2_dev,
"Error: requesting VINT1 interrupt\n");
-   free_irq(vpfe_dev->ccdc_irq0, vpfe_dev);
-   return ret;
+   goto free_irq;
}
 
ret = request_irq(vpfe_dev->imp_dma_irq, vpfe_imp_dma_isr,
@@ -319,11 +318,14 @@ static int vpfe_attach_irq(struct vpfe_device *vpfe_dev)
v4l2_err(_dev->v4l2_dev,
 "Error: requesting IMP IRQ interrupt\n");
free_irq(vpfe_dev->ccdc_irq1, vpfe_dev);
-   free_irq(vpfe_dev->ccdc_irq0, vpfe_dev);
-   return ret;
+   goto free_irq;
}
 
return 0;
+
+free_irq:
+   free_irq(vpfe_dev->ccdc_irq0, vpfe_dev);
+   return ret;
 }
 
 /*
-- 
2.15.0

Re: Adjustments for a lot of function implementations

[SF Markus Elfring]

 but will reject the others, not just this driver but all of them
 that are currently pending in our patchwork 
 (https://patchwork.linuxtv.org).

I find it very surprising that you rejected 146 useful update suggestions
so easily.


 Feel free to repost, but only if you organize the patch as either fixing
 the same type of issue for a whole subdirectory (media/usb, media/pci, etc)
>>
>> Just for the record, while this may work for media, it won't work for all
>> subsystems.  One will quickly get a complaint that the big patch needs to
>> go into multiple trees.
> 
> For the record: this only applies to drivers/media.

What does this software area make it so special in comparison to
other Linux subsystems?


> We discussed what do to with series like this during our media summit
> last Friday and this was the conclusion of that.

* Have you taken any other solution approaches into account than
  a quick “rejection”?

* Could your reaction have been different if the remarkable number of
  change possibilities were sent by different authors (and not only me)?

* How should possibly remaining disagreements about affected implementation
  details be resolved now?

* Are you looking for further improvements around development tools
  like “patchwork” and “quilt”?

* Will you accept increasing risks because of bigger patch sizes?


 or fixing all issues for a single driver.
>>>
>>> I find that I did this already.

* Can such an information lead to differences in the preferred patch 
granularity?

* How do you think about this detail?


 Actual bug fixes (like the null pointer patch in this series) can still be 
 posted as
 separate patches, but cleanups shouldn't.
>>>
>>> I got an other software development opinion.

How would you ever like to clean up stuff in affected source files
which was accumulated (or preserved somehow) over years?


 Just so you know, I'll reject any future patch series that do not follow 
 these rules.

I guess that this handling will trigger more communication challenges.


 Just use common sense when posting these things in the future.

Our “common sense” seems to be occasionally different in significant ways.


 I would also suggest that your time might be spent more productively
 if you would work on some more useful projects.

I distribute my software development capacity over several areas.
Does your wording indicate a questionable signal for further contributions?

Regards,
Markus

Re: Adjustments for a lot of function implementations

[SF Markus Elfring]

> Yes, and you were told not to do it

I have got an other impression.


> like that again.

I continued with the presentation of suggestions from my selection
of change possibilities.
It seems that there are very different expectations for the
preferred patch granularity.

Can it happen again that you are going to use a development tool
like “quilt” (as a maintainer) for the desired recombination
of possible update steps?

Regards,
Markus

Re: Adjustments for a lot of function implementations

[SF Markus Elfring]

 Feel free to repost, but only if you organize the patch as either fixing 
 the same type of
 issue for a whole subdirectory (media/usb, media/pci, etc)
>>
>> Just for the record, while this may work for media, it won't work for all
>> subsystems.  One will quickly get a complaint that the big patch needs to
>> go into multiple trees.
> 
> For the record: this only applies to drivers/media.

Interesting …


> We discussed what do to with series like this during our media summit last 
> Friday

Would you like to share any more information from this meeting?


> and this was the conclusion of that.

I would appreciate further indications for a corresponding change acceptance.

I found a feedback by Mauro Carvalho Chehab more constructive.

[GIT,PULL,FOR,v4.15] Cleanup fixes
https://patchwork.linuxtv.org/patch/43957/

“…
This time, I was nice and I took some time doing:

$ quilt fold < `quilt next` && quilt delete `quilt next`
…”


Regards,
Markus

Re: Adjustments for a lot of function implementations

[SF Markus Elfring]

> While we do not mind cleanup patches, the way you post them (one fix per file)

I find it safer in this way  while I was browsing through the landscape of Linux
software components.


> is really annoying and takes us too much time to review.

It is just the case that there are so many remaining open issues.


> I'll take the "Fix a possible null pointer" patch since it is an actual bug 
> fix,

Thanks for a bit of change acceptance.


> but will reject the others, not just this driver but all of them that are 
> currently
> pending in our patchwork (https://patchwork.linuxtv.org).

Will any chances evolve to integrate 146 patches in any other combination?


> Feel free to repost, but only if you organize the patch as either fixing the 
> same type of
> issue for a whole subdirectory (media/usb, media/pci, etc)

Can we achieve an agreement on the shown change patterns?

Is a consensus possible for involved update candidates?


> or fixing all issues for a single driver.

I find that I did this already.


> Actual bug fixes (like the null pointer patch in this series) can still be 
> posted as
> separate patches, but cleanups shouldn't.

I got an other software development opinion.


> Just so you know, I'll reject any future patch series that do not follow 
> these rules.
> Just use common sense when posting these things in the future.

Do we need to try any additional communication tools out?


> I would also suggest that your time might be spent more productively if you 
> would
> work on some more useful projects.

I hope that various change possibilities (from my selection) will become useful
for more Linux users.
How will the clarification evolve further?


Regards,
Markus

Re: [PATCH 1/6] [media] tda8261: Use common error handling code in tda8261_set_params()

[SF Markus Elfring]

>> @@ -129,18 +129,18 @@ static int tda8261_set_params(struct dvb_frontend *fe)
>>  
>>  /* Set params */
>>  err = tda8261_write(state, buf);
>> -if (err < 0) {
>> -pr_err("%s: I/O Error\n", __func__);
>> -return err;
>> -}
>> +err = tda8261_get_status(fe, );
>> +if (err < 0)
>> +goto report_failure;
>> +
> 
> Is this change really correct? Doesn't it query the status once more
> often than before?

Thanks for your inquiry.

Unfortunately, I made a copy mistake at this source code place.
When should I send a corrected suggestion for this update step
in the patch series?

Regards,
Markus

[PATCH 2/2] [media] dmxdev: Improve a size determination in dvb_dmxdev_add_pid()

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 26 Sep 2017 15:22:57 +0200

Replace the specification of a data structure by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/media/dvb-core/dmxdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/dvb-core/dmxdev.c b/drivers/media/dvb-core/dmxdev.c
index f8bf7459d5ca..8e0f91775fe4 100644
--- a/drivers/media/dvb-core/dmxdev.c
+++ b/drivers/media/dvb-core/dmxdev.c
@@ -809,7 +809,7 @@ static int dvb_dmxdev_add_pid(struct dmxdev *dmxdev,
(!list_empty(>feed.ts)))
return -EINVAL;
 
-   feed = kzalloc(sizeof(struct dmxdev_feed), GFP_KERNEL);
+   feed = kzalloc(sizeof(*feed), GFP_KERNEL);
if (feed == NULL)
return -ENOMEM;
 
-- 
2.14.1

[PATCH 0/2] [media] dmxdev: Fine-tuning for two function implementations

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 26 Sep 2017 15:30:03 +0200

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

Markus Elfring (2):
  Use common error handling code in dvb_dmxdev_start_feed()
  Improve a size determination in dvb_dmxdev_add_pid()

 drivers/media/dvb-core/dmxdev.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

-- 
2.14.1

[PATCH 1/2] [media] dmxdev: Use common error handling code in dvb_dmxdev_start_feed()

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 26 Sep 2017 15:12:47 +0200

Add a jump target 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 
---
 drivers/media/dvb-core/dmxdev.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/media/dvb-core/dmxdev.c b/drivers/media/dvb-core/dmxdev.c
index 45e91add73ba..f8bf7459d5ca 100644
--- a/drivers/media/dvb-core/dmxdev.c
+++ b/drivers/media/dvb-core/dmxdev.c
@@ -594,18 +594,18 @@ static int dvb_dmxdev_start_feed(struct dmxdev *dmxdev,
tsfeed->priv = filter;
 
ret = tsfeed->set(tsfeed, feed->pid, ts_type, ts_pes, timeout);
-   if (ret < 0) {
-   dmxdev->demux->release_ts_feed(dmxdev->demux, tsfeed);
-   return ret;
-   }
+   if (ret < 0)
+   goto release_feed;
 
ret = tsfeed->start_filtering(tsfeed);
-   if (ret < 0) {
-   dmxdev->demux->release_ts_feed(dmxdev->demux, tsfeed);
-   return ret;
-   }
+   if (ret < 0)
+   goto release_feed;
 
return 0;
+
+release_feed:
+   dmxdev->demux->release_ts_feed(dmxdev->demux, tsfeed);
+   return ret;
 }
 
 static int dvb_dmxdev_filter_start(struct dmxdev_filter *filter)
-- 
2.14.1

[PATCH 6/6] [media] tda8261: Delete an unnecessary variable initialisation in three functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 26 Sep 2017 12:55:16 +0200

The local variable "err" is reassigned by a statement at the beginning.
Thus omit the explicit initialisation in these functions.

Signed-off-by: Markus Elfring 
---
 drivers/media/dvb-frontends/tda8261.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/dvb-frontends/tda8261.c 
b/drivers/media/dvb-frontends/tda8261.c
index 20185ee8f253..33144a34e337 100644
--- a/drivers/media/dvb-frontends/tda8261.c
+++ b/drivers/media/dvb-frontends/tda8261.c
@@ -39,7 +39,7 @@ struct tda8261_state {
 static int tda8261_read(struct tda8261_state *state, u8 *buf)
 {
const struct tda8261_config *config = state->config;
-   int err = 0;
+   int err;
struct i2c_msg msg = { .addr= config->addr, .flags = I2C_M_RD,.buf 
= buf,  .len = 1 };
 
err = i2c_transfer(state->i2c, , 1);
@@ -52,7 +52,7 @@ static int tda8261_read(struct tda8261_state *state, u8 *buf)
 static int tda8261_write(struct tda8261_state *state, u8 *buf)
 {
const struct tda8261_config *config = state->config;
-   int err = 0;
+   int err;
struct i2c_msg msg = { .addr = config->addr, .flags = 0, .buf = buf, 
.len = 4 };
 
err = i2c_transfer(state->i2c, , 1);
@@ -66,7 +66,7 @@ static int tda8261_get_status(struct dvb_frontend *fe, u32 
*status)
 {
struct tda8261_state *state = fe->tuner_priv;
u8 result = 0;
-   int err = 0;
+   int err;
 
*status = 0;
err = tda8261_read(state, );
-- 
2.14.1

[PATCH 4/6] [media] tda8261: Delete an unnecessary variable initialisation in tda8261_attach()

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 26 Sep 2017 12:24:57 +0200

The local variable "state" is reassigned by a statement at the beginning.
Thus omit the explicit initialisation.

Signed-off-by: Markus Elfring 
---
 drivers/media/dvb-frontends/tda8261.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/dvb-frontends/tda8261.c 
b/drivers/media/dvb-frontends/tda8261.c
index e3b4183d00c2..492d8c03a5fa 100644
--- a/drivers/media/dvb-frontends/tda8261.c
+++ b/drivers/media/dvb-frontends/tda8261.c
@@ -183,7 +183,7 @@ struct dvb_frontend *tda8261_attach(struct dvb_frontend *fe,
const struct tda8261_config *config,
struct i2c_adapter *i2c)
 {
-   struct tda8261_state *state = NULL;
+   struct tda8261_state *state;
 
state = kzalloc(sizeof(*state), GFP_KERNEL);
if (!state)
-- 
2.14.1

[PATCH 5/6] [media] tda8261: Adjust three function calls together with a variable assignment

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 26 Sep 2017 12:52:24 +0200

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

ERROR: do not use assignment in if condition

Thus fix the affected source code places.

Signed-off-by: Markus Elfring 
---
 drivers/media/dvb-frontends/tda8261.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/media/dvb-frontends/tda8261.c 
b/drivers/media/dvb-frontends/tda8261.c
index 492d8c03a5fa..20185ee8f253 100644
--- a/drivers/media/dvb-frontends/tda8261.c
+++ b/drivers/media/dvb-frontends/tda8261.c
@@ -42,7 +42,8 @@ static int tda8261_read(struct tda8261_state *state, u8 *buf)
int err = 0;
struct i2c_msg msg = { .addr= config->addr, .flags = I2C_M_RD,.buf 
= buf,  .len = 1 };
 
-   if ((err = i2c_transfer(state->i2c, , 1)) != 1)
+   err = i2c_transfer(state->i2c, , 1);
+   if (err != 1)
pr_err("%s: read error, err=%d\n", __func__, err);
 
return err;
@@ -54,7 +55,8 @@ static int tda8261_write(struct tda8261_state *state, u8 *buf)
int err = 0;
struct i2c_msg msg = { .addr = config->addr, .flags = 0, .buf = buf, 
.len = 4 };
 
-   if ((err = i2c_transfer(state->i2c, , 1)) != 1)
+   err = i2c_transfer(state->i2c, , 1);
+   if (err != 1)
pr_err("%s: write error, err=%d\n", __func__, err);
 
return err;
@@ -67,8 +69,8 @@ static int tda8261_get_status(struct dvb_frontend *fe, u32 
*status)
int err = 0;
 
*status = 0;
-
-   if ((err = tda8261_read(state, )) < 0) {
+   err = tda8261_read(state, );
+   if (err < 0) {
pr_err("%s: I/O Error\n", __func__);
return err;
}
-- 
2.14.1

[PATCH 3/6] [media] tda8261: Return directly after a failed kzalloc() in tda8261_attach()

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 26 Sep 2017 12:20:33 +0200

* Return directly after a call of the function "kzalloc" failed
  at the beginning.

* Delete a call of the function "kfree" and the jump target "exit"
  which became unnecessary with this refactoring.

Signed-off-by: Markus Elfring 
---
 drivers/media/dvb-frontends/tda8261.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/media/dvb-frontends/tda8261.c 
b/drivers/media/dvb-frontends/tda8261.c
index 5269a170c84e..e3b4183d00c2 100644
--- a/drivers/media/dvb-frontends/tda8261.c
+++ b/drivers/media/dvb-frontends/tda8261.c
@@ -187,7 +187,7 @@ struct dvb_frontend *tda8261_attach(struct dvb_frontend *fe,
 
state = kzalloc(sizeof(*state), GFP_KERNEL);
if (!state)
-   goto exit;
+   return NULL;
 
state->config   = config;
state->i2c  = i2c;
@@ -200,10 +200,6 @@ struct dvb_frontend *tda8261_attach(struct dvb_frontend 
*fe,
pr_info("%s: Attaching TDA8261 8PSK/QPSK tuner\n", __func__);
 
return fe;
-
-exit:
-   kfree(state);
-   return NULL;
 }
 
 EXPORT_SYMBOL(tda8261_attach);
-- 
2.14.1

[PATCH 2/6] [media] tda8261: Improve a size determination in tda8261_attach()

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 26 Sep 2017 12:06:19 +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 a data structure by a pointer dereference
  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 
---
 drivers/media/dvb-frontends/tda8261.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/dvb-frontends/tda8261.c 
b/drivers/media/dvb-frontends/tda8261.c
index 5a8a9b6b8107..5269a170c84e 100644
--- a/drivers/media/dvb-frontends/tda8261.c
+++ b/drivers/media/dvb-frontends/tda8261.c
@@ -185,7 +185,8 @@ struct dvb_frontend *tda8261_attach(struct dvb_frontend *fe,
 {
struct tda8261_state *state = NULL;
 
-   if ((state = kzalloc(sizeof (struct tda8261_state), GFP_KERNEL)) == 
NULL)
+   state = kzalloc(sizeof(*state), GFP_KERNEL);
+   if (!state)
goto exit;
 
state->config   = config;
-- 
2.14.1

[PATCH 1/6] [media] tda8261: Use common error handling code in tda8261_set_params()

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 26 Sep 2017 11:01:44 +0200

* Add a jump target 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.

* 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.

Signed-off-by: Markus Elfring 
---
 drivers/media/dvb-frontends/tda8261.c | 20 
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/media/dvb-frontends/tda8261.c 
b/drivers/media/dvb-frontends/tda8261.c
index 4eb294f330bc..5a8a9b6b8107 100644
--- a/drivers/media/dvb-frontends/tda8261.c
+++ b/drivers/media/dvb-frontends/tda8261.c
@@ -129,18 +129,18 @@ static int tda8261_set_params(struct dvb_frontend *fe)
 
/* Set params */
err = tda8261_write(state, buf);
-   if (err < 0) {
-   pr_err("%s: I/O Error\n", __func__);
-   return err;
-   }
+   err = tda8261_get_status(fe, );
+   if (err < 0)
+   goto report_failure;
+
/* sleep for some time */
pr_debug("%s: Waiting to Phase LOCK\n", __func__);
msleep(20);
/* check status */
-   if ((err = tda8261_get_status(fe, )) < 0) {
-   pr_err("%s: I/O Error\n", __func__);
-   return err;
-   }
+   err = tda8261_get_status(fe, );
+   if (err < 0)
+   goto report_failure;
+
if (status == 1) {
pr_debug("%s: Tuner Phase locked: status=%d\n", __func__,
 status);
@@ -150,6 +150,10 @@ static int tda8261_set_params(struct dvb_frontend *fe)
}
 
return 0;
+
+report_failure:
+   pr_err("%s: I/O Error\n", __func__);
+   return err;
 }
 
 static void tda8261_release(struct dvb_frontend *fe)
-- 
2.14.1

[PATCH 0/6] [media] TDA8261: Fine-tuning for five function implementations

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 26 Sep 2017 13:20:12 +0200

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

Markus Elfring (6):
  Use common error handling code in tda8261_set_params()
  Improve a size determination in tda8261_attach()
  Return directly after a failed kzalloc() in tda8261_attach()
  Delete an unnecessary variable initialisation in tda8261_attach()
  Adjust three function calls together with a variable assignment
  Delete an unnecessary variable initialisation in three functions

 drivers/media/dvb-frontends/tda8261.c | 47 +++
 1 file changed, 25 insertions(+), 22 deletions(-)

-- 
2.14.1

[PATCH] [media] bt8xx: Use common error handling code in two functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 25 Sep 2017 22:10:17 +0200

Adjust jump targets so that a bit of exception handling can be better
reused at the end of these functions.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/media/pci/bt8xx/dst.c| 19 +++
 drivers/media/pci/bt8xx/dst_ca.c | 30 +++---
 2 files changed, 26 insertions(+), 23 deletions(-)

diff --git a/drivers/media/pci/bt8xx/dst.c b/drivers/media/pci/bt8xx/dst.c
index 7166d2279465..1290419aca0b 100644
--- a/drivers/media/pci/bt8xx/dst.c
+++ b/drivers/media/pci/bt8xx/dst.c
@@ -134,17 +134,20 @@ EXPORT_SYMBOL(rdc_reset_state);
 static int rdc_8820_reset(struct dst_state *state)
 {
dprintk(3, "Resetting DST\n");
-   if (dst_gpio_outb(state, RDC_8820_RESET, RDC_8820_RESET, 0, NO_DELAY) < 
0) {
-   pr_err("dst_gpio_outb ERROR !\n");
-   return -1;
-   }
+   if (dst_gpio_outb(state, RDC_8820_RESET, RDC_8820_RESET, 0, NO_DELAY)
+   < 0)
+   goto report_failure;
+
udelay(1000);
-   if (dst_gpio_outb(state, RDC_8820_RESET, RDC_8820_RESET, 
RDC_8820_RESET, DELAY) < 0) {
-   pr_err("dst_gpio_outb ERROR !\n");
-   return -1;
-   }
+   if (dst_gpio_outb(state, RDC_8820_RESET, RDC_8820_RESET,
+ RDC_8820_RESET, DELAY) < 0)
+   goto report_failure;
 
return 0;
+
+report_failure:
+   pr_err("dst_gpio_outb ERROR !\n");
+   return -1;
 }
 
 static int dst_pio_enable(struct dst_state *state)
diff --git a/drivers/media/pci/bt8xx/dst_ca.c b/drivers/media/pci/bt8xx/dst_ca.c
index 90f4263452d3..5ea0a9c9a590 100644
--- a/drivers/media/pci/bt8xx/dst_ca.c
+++ b/drivers/media/pci/bt8xx/dst_ca.c
@@ -97,33 +97,33 @@ static int dst_ci_command(struct dst_state* state, u8 * 
data, u8 *ca_string, u8
 
if (write_dst(state, data, len)) {
dprintk(verbose, DST_CA_INFO, 1, " Write not successful, trying 
to recover");
-   dst_error_recovery(state);
-   goto error;
+   goto error_recovery;
}
if ((dst_pio_disable(state)) < 0) {
dprintk(verbose, DST_CA_ERROR, 1, " DST PIO disable failed.");
-   goto error;
-   }
-   if (read_dst(state, , GET_ACK) < 0) {
-   dprintk(verbose, DST_CA_INFO, 1, " Read not successful, trying 
to recover");
-   dst_error_recovery(state);
-   goto error;
+   goto unlock;
}
+   if (read_dst(state, , GET_ACK) < 0)
+   goto report_read_failure;
+
if (read) {
if (! dst_wait_dst_ready(state, LONG_DELAY)) {
dprintk(verbose, DST_CA_NOTICE, 1, " 8820 not ready");
-   goto error;
-   }
-   if (read_dst(state, ca_string, 128) < 0) {  /*  Try to 
make this dynamic*/
-   dprintk(verbose, DST_CA_INFO, 1, " Read not successful, 
trying to recover");
-   dst_error_recovery(state);
-   goto error;
+   goto unlock;
}
+   /* Try to make this dynamic */
+   if (read_dst(state, ca_string, 128) < 0)
+   goto report_read_failure;
}
mutex_unlock(>dst_mutex);
return 0;
 
-error:
+report_read_failure:
+   dprintk(verbose, DST_CA_INFO, 1,
+   " Read not successful, trying to recover");
+error_recovery:
+   dst_error_recovery(state);
+unlock:
mutex_unlock(>dst_mutex);
return -EIO;
 }
-- 
2.14.1

[PATCH] [media] cx88: Use common error handling code in get_key_pvr2000()

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 25 Sep 2017 21:03:57 +0200

Add a jump target 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 
---
 drivers/media/pci/cx88/cx88-input.c | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/media/pci/cx88/cx88-input.c 
b/drivers/media/pci/cx88/cx88-input.c
index e02449bf2041..001fbb97bcdf 100644
--- a/drivers/media/pci/cx88/cx88-input.c
+++ b/drivers/media/pci/cx88/cx88-input.c
@@ -566,20 +566,17 @@ static int get_key_pvr2000(struct IR_i2c *ir, enum 
rc_proto *protocol,
 
/* poll IR chip */
flags = i2c_smbus_read_byte_data(ir->c, 0x10);
-   if (flags < 0) {
-   dprintk("read error\n");
-   return 0;
-   }
+   if (flags < 0)
+   goto report_read_failure;
+
/* key pressed ? */
if (0 == (flags & 0x80))
return 0;
 
/* read actual key code */
code = i2c_smbus_read_byte_data(ir->c, 0x00);
-   if (code < 0) {
-   dprintk("read error\n");
-   return 0;
-   }
+   if (code < 0)
+   goto report_read_failure;
 
dprintk("IR Key/Flags: (0x%02x/0x%02x)\n",
code & 0xff, flags & 0xff);
@@ -588,6 +585,10 @@ static int get_key_pvr2000(struct IR_i2c *ir, enum 
rc_proto *protocol,
*scancode = code & 0xff;
*toggle = 0;
return 1;
+
+report_read_failure:
+   dprintk("read error\n");
+   return 0;
 }
 
 void cx88_i2c_init_ir(struct cx88_core *core)
-- 
2.14.1

[PATCH 4/4] [media] omap3isp: Delete an unnecessary variable initialisation in isp_video_open()

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 24 Sep 2017 19:43:06 +0200

The variable "ret" will eventually be set to an appropriate value
a bit later. Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring 
---
 drivers/media/platform/omap3isp/ispvideo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/omap3isp/ispvideo.c 
b/drivers/media/platform/omap3isp/ispvideo.c
index d4118466fc8a..a9c0b2d3624d 100644
--- a/drivers/media/platform/omap3isp/ispvideo.c
+++ b/drivers/media/platform/omap3isp/ispvideo.c
@@ -1303,7 +1303,7 @@ static int isp_video_open(struct file *file)
struct isp_video *video = video_drvdata(file);
struct isp_video_fh *handle;
struct vb2_queue *queue;
-   int ret = 0;
+   int ret;
 
handle = kzalloc(sizeof(*handle), GFP_KERNEL);
if (!handle)
-- 
2.14.1

[PATCH 3/4] [media] omap3isp: Use common error handling code in isp_video_open()

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 24 Sep 2017 19:30:52 +0200

* Adjust 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.

* Delete a repeated check (for the variable "ret") which became unnecessary
  with this refactoring.

Signed-off-by: Markus Elfring 
---
 drivers/media/platform/omap3isp/ispvideo.c | 29 +
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/drivers/media/platform/omap3isp/ispvideo.c 
b/drivers/media/platform/omap3isp/ispvideo.c
index 7b9bd684337a..d4118466fc8a 100644
--- a/drivers/media/platform/omap3isp/ispvideo.c
+++ b/drivers/media/platform/omap3isp/ispvideo.c
@@ -1315,14 +1315,12 @@ static int isp_video_open(struct file *file)
/* If this is the first user, initialise the pipeline. */
if (omap3isp_get(video->isp) == NULL) {
ret = -EBUSY;
-   goto done;
+   goto delete_fh;
}
 
ret = v4l2_pipeline_pm_use(>video.entity, 1);
-   if (ret < 0) {
-   omap3isp_put(video->isp);
-   goto done;
-   }
+   if (ret < 0)
+   goto put_isp;
 
queue = >queue;
queue->type = video->type;
@@ -1335,10 +1333,8 @@ static int isp_video_open(struct file *file)
queue->dev = video->isp->dev;
 
ret = vb2_queue_init(>queue);
-   if (ret < 0) {
-   omap3isp_put(video->isp);
-   goto done;
-   }
+   if (ret < 0)
+   goto put_isp;
 
memset(>format, 0, sizeof(handle->format));
handle->format.type = video->type;
@@ -1346,14 +1342,15 @@ static int isp_video_open(struct file *file)
 
handle->video = video;
file->private_data = >vfh;
+   goto exit;
 
-done:
-   if (ret < 0) {
-   v4l2_fh_del(>vfh);
-   v4l2_fh_exit(>vfh);
-   kfree(handle);
-   }
-
+put_isp:
+   omap3isp_put(video->isp);
+delete_fh:
+   v4l2_fh_del(>vfh);
+   v4l2_fh_exit(>vfh);
+   kfree(handle);
+exit:
return ret;
 }
 
-- 
2.14.1

[PATCH 2/4] [media] omap3isp: Adjust 53 checks for null pointers

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 24 Sep 2017 18:56:33 +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 
---
 drivers/media/platform/omap3isp/isp.c| 16 
 drivers/media/platform/omap3isp/ispccdc.c| 28 ++--
 drivers/media/platform/omap3isp/ispccp2.c|  6 +++---
 drivers/media/platform/omap3isp/ispcsi2.c|  6 +++---
 drivers/media/platform/omap3isp/ispcsiphy.c  |  2 +-
 drivers/media/platform/omap3isp/isph3a_af.c  |  2 +-
 drivers/media/platform/omap3isp/isphist.c|  4 ++--
 drivers/media/platform/omap3isp/isppreview.c |  8 
 drivers/media/platform/omap3isp/ispresizer.c |  8 
 drivers/media/platform/omap3isp/ispstat.c|  4 ++--
 drivers/media/platform/omap3isp/ispvideo.c   | 22 +++---
 11 files changed, 53 insertions(+), 53 deletions(-)

diff --git a/drivers/media/platform/omap3isp/isp.c 
b/drivers/media/platform/omap3isp/isp.c
index 874b883ac83a..2ebff06bb523 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -552,25 +552,25 @@ static void isp_isr_sbl(struct isp_device *isp)
 
if (sbl_pcr & ISPSBL_PCR_CSIB_WBL_OVF) {
pipe = to_isp_pipeline(>isp_ccp2.subdev.entity);
-   if (pipe != NULL)
+   if (pipe)
pipe->error = true;
}
 
if (sbl_pcr & ISPSBL_PCR_CSIA_WBL_OVF) {
pipe = to_isp_pipeline(>isp_csi2a.subdev.entity);
-   if (pipe != NULL)
+   if (pipe)
pipe->error = true;
}
 
if (sbl_pcr & ISPSBL_PCR_CCDC_WBL_OVF) {
pipe = to_isp_pipeline(>isp_ccdc.subdev.entity);
-   if (pipe != NULL)
+   if (pipe)
pipe->error = true;
}
 
if (sbl_pcr & ISPSBL_PCR_PRV_WBL_OVF) {
pipe = to_isp_pipeline(>isp_prev.subdev.entity);
-   if (pipe != NULL)
+   if (pipe)
pipe->error = true;
}
 
@@ -579,7 +579,7 @@ static void isp_isr_sbl(struct isp_device *isp)
   | ISPSBL_PCR_RSZ3_WBL_OVF
   | ISPSBL_PCR_RSZ4_WBL_OVF)) {
pipe = to_isp_pipeline(>isp_res.subdev.entity);
-   if (pipe != NULL)
+   if (pipe)
pipe->error = true;
}
 
@@ -1401,7 +1401,7 @@ static struct isp_device *__omap3isp_get(struct 
isp_device *isp, bool irq)
 {
struct isp_device *__isp = isp;
 
-   if (isp == NULL)
+   if (!isp)
return NULL;
 
mutex_lock(>isp_mutex);
@@ -1421,7 +1421,7 @@ static struct isp_device *__omap3isp_get(struct 
isp_device *isp, bool irq)
isp_enable_interrupts(isp);
 
 out:
-   if (__isp != NULL)
+   if (__isp)
isp->ref_count++;
mutex_unlock(>isp_mutex);
 
@@ -1441,7 +1441,7 @@ struct isp_device *omap3isp_get(struct isp_device *isp)
  */
 static void __omap3isp_put(struct isp_device *isp, bool save_ctx)
 {
-   if (isp == NULL)
+   if (!isp)
return;
 
mutex_lock(>isp_mutex);
diff --git a/drivers/media/platform/omap3isp/ispccdc.c 
b/drivers/media/platform/omap3isp/ispccdc.c
index b66276ab5765..579d3b406344 100644
--- a/drivers/media/platform/omap3isp/ispccdc.c
+++ b/drivers/media/platform/omap3isp/ispccdc.c
@@ -355,7 +355,7 @@ static void ccdc_lsc_free_request(struct isp_ccdc_device 
*ccdc,
 {
struct isp_device *isp = to_isp_device(ccdc);
 
-   if (req == NULL)
+   if (!req)
return;
 
if (req->table.addr) {
@@ -423,7 +423,7 @@ static int ccdc_lsc_config(struct isp_ccdc_device *ccdc,
}
 
req = kzalloc(sizeof(*req), GFP_KERNEL);
-   if (req == NULL)
+   if (!req)
return -ENOMEM;
 
if (config->flag & OMAP3ISP_CCDC_CONFIG_LSC) {
@@ -438,7 +438,7 @@ static int ccdc_lsc_config(struct isp_ccdc_device *ccdc,
req->table.addr = dma_alloc_coherent(isp->dev, req->config.size,
 >table.dma,
 GFP_KERNEL);
-   if (req->table.addr == NULL) {
+   if (!req->table.addr) {
ret = -ENOMEM;
goto done;
}
@@ -731,7 +731,7 @@ static int ccdc_config(struct isp_ccdc_device *ccdc,
fpc_new.addr = dma_alloc_coherent(isp->dev, size,
  _new.dma,
  GFP_KERNEL);
-   if 

[PATCH 1/4] [media] omap3isp: Delete an error message for a failed memory allocation in three functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 24 Sep 2017 18:25:44 +0200

Omit an extra message for a memory allocation failure in these functions.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/media/platform/omap3isp/isp.c | 4 +---
 drivers/media/platform/omap3isp/isph3a_aewb.c | 5 +
 drivers/media/platform/omap3isp/isph3a_af.c   | 5 +
 3 files changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/media/platform/omap3isp/isp.c 
b/drivers/media/platform/omap3isp/isp.c
index 1a428fe9f070..874b883ac83a 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2236,10 +2236,8 @@ static int isp_probe(struct platform_device *pdev)
int i, m;
 
isp = devm_kzalloc(>dev, sizeof(*isp), GFP_KERNEL);
-   if (!isp) {
-   dev_err(>dev, "could not allocate memory\n");
+   if (!isp)
return -ENOMEM;
-   }
 
ret = fwnode_property_read_u32(of_fwnode_handle(pdev->dev.of_node),
   "ti,phy-type", >phy_type);
diff --git a/drivers/media/platform/omap3isp/isph3a_aewb.c 
b/drivers/media/platform/omap3isp/isph3a_aewb.c
index d44626f20ac6..9844b9d06634 100644
--- a/drivers/media/platform/omap3isp/isph3a_aewb.c
+++ b/drivers/media/platform/omap3isp/isph3a_aewb.c
@@ -303,11 +303,8 @@ int omap3isp_h3a_aewb_init(struct isp_device *isp)
/* Set recover state configuration */
aewb_recover_cfg = devm_kzalloc(isp->dev, sizeof(*aewb_recover_cfg),
GFP_KERNEL);
-   if (!aewb_recover_cfg) {
-   dev_err(aewb->isp->dev,
-   "AEWB: cannot allocate memory for recover 
configuration.\n");
+   if (!aewb_recover_cfg)
return -ENOMEM;
-   }
 
aewb_recover_cfg->saturation_limit = OMAP3ISP_AEWB_MAX_SATURATION_LIM;
aewb_recover_cfg->win_height = OMAP3ISP_AEWB_MIN_WIN_H;
diff --git a/drivers/media/platform/omap3isp/isph3a_af.c 
b/drivers/media/platform/omap3isp/isph3a_af.c
index 99bd6cc21d86..b81e869ade8c 100644
--- a/drivers/media/platform/omap3isp/isph3a_af.c
+++ b/drivers/media/platform/omap3isp/isph3a_af.c
@@ -366,11 +366,8 @@ int omap3isp_h3a_af_init(struct isp_device *isp)
/* Set recover state configuration */
af_recover_cfg = devm_kzalloc(isp->dev, sizeof(*af_recover_cfg),
  GFP_KERNEL);
-   if (!af_recover_cfg) {
-   dev_err(af->isp->dev,
-   "AF: cannot allocate memory for recover 
configuration.\n");
+   if (!af_recover_cfg)
return -ENOMEM;
-   }
 
af_recover_cfg->paxel.h_start = OMAP3ISP_AF_PAXEL_HZSTART_MIN;
af_recover_cfg->paxel.width = OMAP3ISP_AF_PAXEL_WIDTH_MIN;
-- 
2.14.1

[PATCH 0/4] [media] OMAP3 ISP: Adjustments for some function implementations

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 24 Sep 2017 19:57:34 +0200

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

Markus Elfring (4):
  Delete an error message for a failed memory allocation in three functions
  Adjust 53 checks for null pointers
  Use common error handling code in isp_video_open()
  Delete an unnecessary variable initialisation in isp_video_open()

 drivers/media/platform/omap3isp/isp.c | 20 +-
 drivers/media/platform/omap3isp/ispccdc.c | 28 +++---
 drivers/media/platform/omap3isp/ispccp2.c |  6 +--
 drivers/media/platform/omap3isp/ispcsi2.c |  6 +--
 drivers/media/platform/omap3isp/ispcsiphy.c   |  2 +-
 drivers/media/platform/omap3isp/isph3a_aewb.c |  5 +--
 drivers/media/platform/omap3isp/isph3a_af.c   |  7 +---
 drivers/media/platform/omap3isp/isphist.c |  4 +-
 drivers/media/platform/omap3isp/isppreview.c  |  8 ++--
 drivers/media/platform/omap3isp/ispresizer.c  |  8 ++--
 drivers/media/platform/omap3isp/ispstat.c |  4 +-
 drivers/media/platform/omap3isp/ispvideo.c| 53 +--
 12 files changed, 70 insertions(+), 81 deletions(-)

-- 
2.14.1

[PATCH 6/6] [media] omap_vout: Delete two unnecessary variable initialisations in omap_vout_probe()

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 24 Sep 2017 11:33:39 +0200

The variables "dssdev" and "vid_dev" will eventually be set
to appropriate pointers a bit later.
Thus omit the explicit initialisations at the beginning.

Signed-off-by: Markus Elfring 
---
 drivers/media/platform/omap/omap_vout.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/omap/omap_vout.c 
b/drivers/media/platform/omap/omap_vout.c
index f446a37064f4..0efcea820007 100644
--- a/drivers/media/platform/omap/omap_vout.c
+++ b/drivers/media/platform/omap/omap_vout.c
@@ -2075,9 +2075,9 @@ static int __init omap_vout_probe(struct platform_device 
*pdev)
 {
int ret = 0, i;
struct omap_overlay *ovl;
-   struct omap_dss_device *dssdev = NULL;
+   struct omap_dss_device *dssdev;
struct omap_dss_device *def_display;
-   struct omap2video_device *vid_dev = NULL;
+   struct omap2video_device *vid_dev;
 
if (omapdss_is_initialized() == false)
return -EPROBE_DEFER;
-- 
2.14.1

[PATCH 5/6] [media] omap_vout: Delete an unnecessary variable initialisation in omap_vout_open()

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 24 Sep 2017 11:20:11 +0200

The local variable "vout" is reassigned by a statement at the beginning.
Thus omit the explicit initialisation.

Signed-off-by: Markus Elfring 
---
 drivers/media/platform/omap/omap_vout.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/omap/omap_vout.c 
b/drivers/media/platform/omap/omap_vout.c
index 71b77426271e..f446a37064f4 100644
--- a/drivers/media/platform/omap/omap_vout.c
+++ b/drivers/media/platform/omap/omap_vout.c
@@ -1001,7 +1001,7 @@ static int omap_vout_release(struct file *file)
 static int omap_vout_open(struct file *file)
 {
struct videobuf_queue *q;
-   struct omap_vout_device *vout = NULL;
+   struct omap_vout_device *vout;
 
vout = video_drvdata(file);
if (!vout)
-- 
2.14.1

[PATCH 4/6] [media] omap_vout: Fix a possible null pointer dereference in omap_vout_open()

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 24 Sep 2017 11:00:57 +0200

Move a debug message so that a null pointer access can not happen
for the variable "vout" in this function.

Fixes: 5c7ab6348e7b3fcca2b8ee548306c774472971e2 ("V4L/DVB: V4L2: Add support 
for OMAP2/3 V4L2 display driver on top of DSS2")

Signed-off-by: Markus Elfring 
---
 drivers/media/platform/omap/omap_vout.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/omap/omap_vout.c 
b/drivers/media/platform/omap/omap_vout.c
index 2b55a8ebd1ad..71b77426271e 100644
--- a/drivers/media/platform/omap/omap_vout.c
+++ b/drivers/media/platform/omap/omap_vout.c
@@ -1004,11 +1004,11 @@ static int omap_vout_open(struct file *file)
struct omap_vout_device *vout = NULL;
 
vout = video_drvdata(file);
-   v4l2_dbg(1, debug, >vid_dev->v4l2_dev, "Entering %s\n", __func__);
-
if (!vout)
return -ENODEV;
 
+   v4l2_dbg(1, debug, >vid_dev->v4l2_dev, "Entering %s\n", __func__);
+
/* for now, we only support single open */
if (vout->opened)
return -EBUSY;
-- 
2.14.1

[PATCH 3/6] [media] omap_vout: Adjust a null pointer check in two functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 24 Sep 2017 10:30:29 +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 
---
 drivers/media/platform/omap/omap_vout.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/omap/omap_vout.c 
b/drivers/media/platform/omap/omap_vout.c
index 4a4d171ca573..2b55a8ebd1ad 100644
--- a/drivers/media/platform/omap/omap_vout.c
+++ b/drivers/media/platform/omap/omap_vout.c
@@ -1006,7 +1006,7 @@ static int omap_vout_open(struct file *file)
vout = video_drvdata(file);
v4l2_dbg(1, debug, >vid_dev->v4l2_dev, "Entering %s\n", __func__);
 
-   if (vout == NULL)
+   if (!vout)
return -ENODEV;
 
/* for now, we only support single open */
@@ -2095,7 +2095,7 @@ static int __init omap_vout_probe(struct platform_device 
*pdev)
}
 
vid_dev = kzalloc(sizeof(*vid_dev), GFP_KERNEL);
-   if (vid_dev == NULL) {
+   if (!vid_dev) {
ret = -ENOMEM;
goto err_dss_init;
}
-- 
2.14.1

[PATCH 2/6] [media] omap_vout: Improve a size determination in two functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 24 Sep 2017 10:18:26 +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.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/media/platform/omap/omap_vout.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/omap/omap_vout.c 
b/drivers/media/platform/omap/omap_vout.c
index aebc1e628ac5..4a4d171ca573 100644
--- a/drivers/media/platform/omap/omap_vout.c
+++ b/drivers/media/platform/omap/omap_vout.c
@@ -1943,8 +1943,7 @@ static int __init omap_vout_create_video_devices(struct 
platform_device *pdev)
struct omap2video_device, v4l2_dev);
 
for (k = 0; k < pdev->num_resources; k++) {
-
-   vout = kzalloc(sizeof(struct omap_vout_device), GFP_KERNEL);
+   vout = kzalloc(sizeof(*vout), GFP_KERNEL);
if (!vout)
return -ENOMEM;
 
@@ -2095,7 +2094,7 @@ static int __init omap_vout_probe(struct platform_device 
*pdev)
goto err_dss_init;
}
 
-   vid_dev = kzalloc(sizeof(struct omap2video_device), GFP_KERNEL);
+   vid_dev = kzalloc(sizeof(*vid_dev), GFP_KERNEL);
if (vid_dev == NULL) {
ret = -ENOMEM;
goto err_dss_init;
-- 
2.14.1

[PATCH 1/6] [media] omap_vout: Delete an error message for a failed memory allocation in omap_vout_create_video_devices()

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 24 Sep 2017 10:08:22 +0200

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

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/media/platform/omap/omap_vout.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/media/platform/omap/omap_vout.c 
b/drivers/media/platform/omap/omap_vout.c
index 4d29860d27b4..aebc1e628ac5 100644
--- a/drivers/media/platform/omap/omap_vout.c
+++ b/drivers/media/platform/omap/omap_vout.c
@@ -1948,7 +1948,5 @@ static int __init omap_vout_create_video_devices(struct 
platform_device *pdev)
-   if (!vout) {
-   dev_err(>dev, ": could not allocate memory\n");
+   if (!vout)
return -ENOMEM;
-   }
 
vout->vid = k;
vid_dev->vouts[k] = vout;
-- 
2.14.1

[PATCH 0/6] [media] omap_vout: Adjustments for three function implementations

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 24 Sep 2017 12:06:54 +0200

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

Markus Elfring (6):
  Delete an error message for a failed memory allocation in 
omap_vout_create_video_devices()
  Improve a size determination in two functions
  Adjust a null pointer check in two functions
  Fix a possible null pointer dereference in omap_vout_open()
  Delete an unnecessary variable initialisation in omap_vout_open()
  Delete two unnecessary variable initialisations in omap_vout_probe()

 drivers/media/platform/omap/omap_vout.c | 23 ++-
 1 file changed, 10 insertions(+), 13 deletions(-)

-- 
2.14.1

Re: [GIT PULL FOR v4.15] Cleanup fixes

[SF Markus Elfring]

> Then change patch granularity: one patch per subsystem or per
> directory (e. g. pci, usb, platform, others).

I imagine that the risks for disagreements can grow with such
a bigger scope.


> That's the usual criteria most maintainers use for cleanups.

I picked some update candidates up where the adjustments could be interpreted
as controversial (despite of your acceptance so far.)


>> Do you want a “development pause” from my queue of change possibilities?
> 
> Those patches are mainly source code "polishing".

I prefer to improve existing source files for a while instead of finding
the next shiny development “toy”.


> I really don't want to take much time handling such kind of patches,
> as they usually doesn't fix any real bug, nor add functionality.

Can any of them influence the run time behaviour in desired ways?

Regards,
Markus

[PATCH 3/3] [media] camss-csid: Adjust a null pointer check in two functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Sat, 23 Sep 2017 21:10:02 +0200

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

Comparison to NULL could be written "!format"

Thus fix the affected source code places.

Signed-off-by: Markus Elfring 
---
 drivers/media/platform/qcom/camss-8x16/camss-csid.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/qcom/camss-8x16/camss-csid.c 
b/drivers/media/platform/qcom/camss-8x16/camss-csid.c
index ffda0fbfe4d8..e546f97fa68c 100644
--- a/drivers/media/platform/qcom/camss-8x16/camss-csid.c
+++ b/drivers/media/platform/qcom/camss-8x16/camss-csid.c
@@ -653,4 +653,4 @@ static int csid_get_format(struct v4l2_subdev *sd,
-   if (format == NULL)
+   if (!format)
return -EINVAL;
 
fmt->format = *format;
@@ -677,4 +677,4 @@ static int csid_set_format(struct v4l2_subdev *sd,
-   if (format == NULL)
+   if (!format)
return -EINVAL;
 
csid_try_format(csid, cfg, fmt->pad, >format, fmt->which);
-- 
2.14.1

[PATCH 2/3] [media] camss-csid: Reduce the scope for a variable in csid_set_power()

[SF Markus Elfring]

From: Markus Elfring 
Date: Sat, 23 Sep 2017 21:00:30 +0200

Move the definition for the local variable "dev" into an if branch
so that the corresponding setting will only be performed if it was
selected by the parameter "on" of this function.

Signed-off-by: Markus Elfring 
---
 drivers/media/platform/qcom/camss-8x16/camss-csid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/camss-8x16/camss-csid.c 
b/drivers/media/platform/qcom/camss-8x16/camss-csid.c
index 92d4dc6b4a66..ffda0fbfe4d8 100644
--- a/drivers/media/platform/qcom/camss-8x16/camss-csid.c
+++ b/drivers/media/platform/qcom/camss-8x16/camss-csid.c
@@ -317,10 +317,10 @@ static int csid_reset(struct csid_device *csid)
 static int csid_set_power(struct v4l2_subdev *sd, int on)
 {
struct csid_device *csid = v4l2_get_subdevdata(sd);
-   struct device *dev = to_device_index(csid, csid->id);
int ret;
 
if (on) {
+   struct device *dev = to_device_index(csid, csid->id);
u32 hw_version;
 
ret = regulator_enable(csid->vdda);
-- 
2.14.1

[PATCH 1/3] [media] camss-csid: Use common error handling code in csid_set_power()

[SF Markus Elfring]

From: Markus Elfring 
Date: Sat, 23 Sep 2017 20:48:33 +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 
---
 drivers/media/platform/qcom/camss-8x16/camss-csid.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/media/platform/qcom/camss-8x16/camss-csid.c 
b/drivers/media/platform/qcom/camss-8x16/camss-csid.c
index 64df82817de3..92d4dc6b4a66 100644
--- a/drivers/media/platform/qcom/camss-8x16/camss-csid.c
+++ b/drivers/media/platform/qcom/camss-8x16/camss-csid.c
@@ -330,13 +330,9 @@ static int csid_set_power(struct v4l2_subdev *sd, int on)
ret = csid_set_clock_rates(csid);
-   if (ret < 0) {
-   regulator_disable(csid->vdda);
-   return ret;
-   }
+   if (ret < 0)
+   goto disable_regulator;
 
ret = camss_enable_clocks(csid->nclocks, csid->clock, dev);
-   if (ret < 0) {
-   regulator_disable(csid->vdda);
-   return ret;
-   }
+   if (ret < 0)
+   goto disable_regulator;
 
enable_irq(csid->irq);
@@ -345,8 +341,7 @@ static int csid_set_power(struct v4l2_subdev *sd, int on)
if (ret < 0) {
disable_irq(csid->irq);
camss_disable_clocks(csid->nclocks, csid->clock);
-   regulator_disable(csid->vdda);
-   return ret;
+   goto disable_regulator;
}
 
hw_version = readl_relaxed(csid->base + CAMSS_CSID_HW_VERSION);
@@ -357,6 +352,11 @@ static int csid_set_power(struct v4l2_subdev *sd, int on)
ret = regulator_disable(csid->vdda);
}
 
+   goto exit;
+
+disable_regulator:
+   regulator_disable(csid->vdda);
+exit:
return ret;
 }
 
-- 
2.14.1

[PATCH 0/3] [media] camss-csid: Fine-tuning for three function implementations

[SF Markus Elfring]

From: Markus Elfring 
Date: Sat, 23 Sep 2017 21:24:56 +0200

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

Markus Elfring (3):
  Use common error handling code in csid_set_power()
  Reduce the scope for a variable in csid_set_power()
  Adjust a null pointer check in two functions

 .../media/platform/qcom/camss-8x16/camss-csid.c| 26 +++---
 1 file changed, 13 insertions(+), 13 deletions(-)

-- 
2.14.1

Re: [media] v4l2-core: Fine-tuning for some function implementations

[SF Markus Elfring]

>> Will the software evolution be continued for related source files?
>> Are there any update candidates left over in the directory “v4l2-core”?
> 
> Sorry, I don't understand the question.

I try to explain my view again.


> We don't want to touch the videobuf-* files unless there is a very good 
> reason.

I hoped that my update suggestions could be good enough once more for this area.


> That old videobuf framework is deprecated and the code is quite fragile
> (i.e. easy to break things).

How do you think about to move this stuff into a separate subdirectory
so that it might become a bit easier to distinguish these software components?


> Everything else in that directory is under continuous development.

I am curious if there are still update candidates left over
(also from my selection of change possibilities).

Regards,
Markus

Re: [GIT PULL FOR v4.15] Cleanup fixes

[SF Markus Elfring]

>> coccinelle, checkpatch, coverity, etc.
…
> It **really** doesn't makes any sense to send patch bombs like that!

I got an other impression for this software development aspect.


> That pisses me off, as it requires a considerable amount of time from
> my side that could be used handling important stuff...

I can partly understand this view.


> You're even doing the same logical change on the same driver several times,
> like this one:
>   atmel-isc: Delete an error message for a failed memory allocation in 
> isc_formats_init()
>   atmel-isi: Delete an error message for a failed memory allocation in 
> two functions

Such a change approach can occasionally occur because of my selection
for a specific patch granularity.


> Please, never do this again.

I guess that it will happen more because there are so many results
to consider from source code analysis.


> Instead, group patches that do the same thing per subsystem.

I was also uncertain about the acceptance for the suggested
change patterns.


> This time, I was nice and I took some time doing:
> 
>   $ quilt fold < `quilt next` && quilt delete `quilt next`
> 
> In order to merge the same logic change altogether, applied to all
> drivers at the subsystem.

Thanks for your constructive information.


> Next time, I'll just ignore the hole crap.

Do you want a “development pause” from my queue of change possibilities?

Regards,
Markus

Re: [git:media_tree/master] media: dvb-frontends: delete jump targets

[SF Markus Elfring]

> * Return directly after a call of the function "kzalloc" failed

Is this wording still appropriate in the commit description for such a 
combination
of changes for 4 source files?

Regards,
Markus


…
>   buf = kmalloc(len + 1, GFP_KERNEL);
> - if (buf == NULL) {
…

Re: [git:media_tree/master] media: drivers: improve size determinations

[SF Markus Elfring]

> This is an automatic generated email to let you know that the following patch 
> were queued:

Thanks for your information about another software evolution
which you would like to integrate also.


> Subject: media: drivers: improve a size determination
…
> Replace the specification of a data structure by a pointer dereference
…
> [mche...@s-opensoure.com: merge similar patches into one]

I find the patch granularity that you chose here interesting once more.

I would find the usage of the plural more appropriate in the commit message
for such a combination of changes for 17 source files.

Regards,
Markus

Re: [git:media_tree/master] media: drivers: delete error messages for failed memory allocation

[SF Markus Elfring]

> This is an automatic generated email to let you know that the following patch 
> were queued:

Thanks for your information about another software evolution
which you would like to integrate also.


> Omit an extra message for a memory allocation failure in this function.
> 
> This issue was detected by using the Coccinelle software.
> 
> [mche...@s-opensource.com: fold several similar patches into one]

I find the patch granularity interesting that you chose here.


Can it be that the sentence “Omit extra messages for a memory allocation failure
in these functions.” would be more appropriate in the commit description
for such a combination of changes for 23 source files?

Regards,
Markus

Re: [media] spca500: Use common error handling code in spca500_synch310()

[SF Markus Elfring]

> No one needs to argue about keeping it the way it is.

I got an other impression in this case after a bit of information
was presented which seems to be contradictory.


> I don't see any improvement brought by the proposed change,

Do you care if the source code for an error message is present only once
in this function?


> other than making the code harder to read.

I suggest to reconsider this concern.


> I find goto statements hard to read, because they inherently make some
> information non local.  They are justified in error path handling,
> if the error path only unwinds what the function did early on.
> That's not the case here.

I am looking also for change possibilities without such a restriction.

 
> The same applies to dozens of patches you proposed recently.

I proposed some software updates to reduce a bit of code duplication.

Do you find any corresponding approaches useful?

Regards,
Markus

Re: [PATCH 0/8] [media] v4l2-core: Fine-tuning for some function implementations

[SF Markus Elfring]

> Sorry Markus, just stay away from the videobuf-* sources.

Will the software evolution be continued for related source files?
Are there any update candidates left over in the directory “v4l2-core”?

Regards,
Markus

Re: [media] spca500: Use common error handling code in spca500_synch310()

[SF Markus Elfring]

>>> They are both equally uninformative.
>>
>> Which identifier would you find appropriate there?
> 
> error was fine.

How do the different views fit together?

Regards,
Markus

Re: [media] spca500: Use common error handling code in spca500_synch310()

[SF Markus Elfring]

>>  return 0;
>> -error:
>> +
>> +report_failure:
>> +PERR("Set packet size: set interface error");
>>  return -EBUSY;
>>  }
> 
> Why change the label name?

I find the suggested variant a bi better.


> They are both equally uninformative.

Which identifier would you find appropriate there?

Regards,
Markus

[PATCH] [media] spca500: Use common error handling code in spca500_synch310()

[SF Markus Elfring]

From: Markus Elfring 
Date: Fri, 22 Sep 2017 18:45:07 +0200

Adjust a jump target 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 
---
 drivers/media/usb/gspca/spca500.c | 21 ++---
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/media/usb/gspca/spca500.c 
b/drivers/media/usb/gspca/spca500.c
index da2d9027914c..1f224f5e5b19 100644
--- a/drivers/media/usb/gspca/spca500.c
+++ b/drivers/media/usb/gspca/spca500.c
@@ -501,7 +501,6 @@ static int spca500_full_reset(struct gspca_dev *gspca_dev)
 static int spca500_synch310(struct gspca_dev *gspca_dev)
 {
-   if (usb_set_interface(gspca_dev->dev, gspca_dev->iface, 0) < 0) {
-   PERR("Set packet size: set interface error");
-   goto error;
-   }
+   if (usb_set_interface(gspca_dev->dev, gspca_dev->iface, 0) < 0)
+   goto report_failure;
+
spca500_ping310(gspca_dev);
@@ -514,12 +513,12 @@ static int spca500_synch310(struct gspca_dev *gspca_dev)
/* Windoze use pipe with altsetting 6 why 7 here */
-   if (usb_set_interface(gspca_dev->dev,
-   gspca_dev->iface,
-   gspca_dev->alt) < 0) {
-   PERR("Set packet size: set interface error");
-   goto error;
-   }
+   if (usb_set_interface(gspca_dev->dev, gspca_dev->iface, gspca_dev->alt)
+   < 0)
+   goto report_failure;
+
return 0;
-error:
+
+report_failure:
+   PERR("Set packet size: set interface error");
return -EBUSY;
 }
 
-- 
2.14.1

[PATCH] [media] sn9c20x: Use common error handling code in sd_init()

[SF Markus Elfring]

From: Markus Elfring 
Date: Fri, 22 Sep 2017 17:45:33 +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 
---
 drivers/media/usb/gspca/sn9c20x.c | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/media/usb/gspca/sn9c20x.c 
b/drivers/media/usb/gspca/sn9c20x.c
index c605f78d6186..282bbd77d815 100644
--- a/drivers/media/usb/gspca/sn9c20x.c
+++ b/drivers/media/usb/gspca/sn9c20x.c
@@ -1788,10 +1788,8 @@ static int sd_init(struct gspca_dev *gspca_dev)
for (i = 0; i < ARRAY_SIZE(bridge_init); i++) {
value = bridge_init[i][1];
reg_w(gspca_dev, bridge_init[i][0], , 1);
-   if (gspca_dev->usb_err < 0) {
-   pr_err("Device initialization failed\n");
-   return gspca_dev->usb_err;
-   }
+   if (gspca_dev->usb_err < 0)
+   goto report_failure;
}
 
if (sd->flags & LED_REVERSE)
@@ -1800,10 +1798,8 @@ static int sd_init(struct gspca_dev *gspca_dev)
reg_w1(gspca_dev, 0x1006, 0x20);
 
reg_w(gspca_dev, 0x10c0, i2c_init, 9);
-   if (gspca_dev->usb_err < 0) {
-   pr_err("Device initialization failed\n");
-   return gspca_dev->usb_err;
-   }
+   if (gspca_dev->usb_err < 0)
+   goto report_failure;
 
switch (sd->sensor) {
case SENSOR_OV9650:
@@ -1869,6 +1865,11 @@ static int sd_init(struct gspca_dev *gspca_dev)
pr_err("Unsupported sensor\n");
gspca_dev->usb_err = -ENODEV;
}
+   goto exit;
+
+report_failure:
+   pr_err("Device initialization failed\n");
+exit:
return gspca_dev->usb_err;
 }
 
-- 
2.14.1

Re: [media] usbvision-core: Delete unnecessary braces in 11 functions

[SF Markus Elfring]

> No.  Multi-line indents get curly braces for readability.

Which of the proposed change possibilities do you not like especially at the 
moment?

Regards,
Markus

Re: [PATCH 2/4] [media] usbvision-core: Use common error handling code in usbvision_set_compress_params()

[SF Markus Elfring]

>> @@ -1913,11 +1908,12 @@ static int usbvision_set_compress_params(struct 
>> usb_usbvision *usbvision)
>>   USB_DIR_OUT | USB_TYPE_VENDOR |
>>   USB_RECIP_ENDPOINT, 0,
>>   (__u16) USBVISION_PCM_THR1, value, 6, HZ);
>> +if (rc < 0)
>> +report_failure:
>> +dev_err(>dev->dev,
>> +"%s: ERROR=%d. USBVISION stopped - reconnect or reload 
>> driver.\n",
>> +__func__, rc);
> 
> You've been asked several times not to write code like this.

This suggestion occurred a few times.

Do you prefer to move this place to the end together with a duplicated 
statement “return rc;”?


> You do it later in the patch series as well.

To which update step do you refer here?

Regards,
Markus

[PATCH 3/3] [media] uvcvideo: Add some spaces for better code readability

[SF Markus Elfring]

From: Markus Elfring 
Date: Thu, 21 Sep 2017 21:12:29 +0200

Use space characters at some source code places according to
the Linux coding style convention.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/uvc/uvc_v4l2.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index 184edf8a0885..cebaba5c4e86 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -123,13 +123,13 @@ static __u32 uvc_try_frame_interval(struct uvc_frame 
*frame, __u32 interval)
best = dist;
}
 
-   interval = frame->dwFrameInterval[i-1];
+   interval = frame->dwFrameInterval[i - 1];
} else {
const __u32 min = frame->dwFrameInterval[0];
const __u32 max = frame->dwFrameInterval[1];
const __u32 step = frame->dwFrameInterval[2];
 
-   interval = min + (interval - min + step/2) / step * step;
+   interval = min + (interval - min + step / 2) / step * step;
if (interval > max)
interval = max;
}
@@ -201,7 +201,7 @@ static int uvc_v4l2_try_format(struct uvc_streaming *stream,
__u16 h = format->frame[i].wHeight;
 
d = min(w, rw) * min(h, rh);
-   d = w*h + rw*rh - 2*d;
+   d = w * h + rw * rh - 2 * d;
if (d < maxd) {
maxd = d;
frame = >frame[i];
@@ -219,9 +219,10 @@ static int uvc_v4l2_try_format(struct uvc_streaming 
*stream,
 
/* Use the default frame interval. */
interval = frame->dwDefaultFrameInterval;
-   uvc_trace(UVC_TRACE_FORMAT, "Using default frame interval %u.%u us "
-   "(%u.%u fps).\n", interval/10, interval%10, 1000/interval,
-   (1/interval)%10);
+   uvc_trace(UVC_TRACE_FORMAT,
+ "Using default frame interval %u.%u us (%u.%u fps).\n",
+ interval / 10, interval % 10,
+ 1000 / interval, (1 / interval) % 10);
 
/* Set the format index, frame index and frame interval. */
memset(probe, 0, sizeof *probe);
-- 
2.14.1

[PATCH 2/3] [media] uvcvideo: Adjust 14 checks for null pointers

[SF Markus Elfring]

From: Markus Elfring 
Date: Thu, 21 Sep 2017 21:00:21 +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 
---
 drivers/media/usb/uvc/uvc_v4l2.c | 27 +--
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index 6ec2b255c44a..184edf8a0885 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -41,7 +41,7 @@ static int uvc_ioctl_ctrl_map(struct uvc_video_chain *chain,
int ret;
 
map = kzalloc(sizeof *map, GFP_KERNEL);
-   if (map == NULL)
+   if (!map)
return -ENOMEM;
 
map->id = xmap->id;
@@ -211,7 +211,7 @@ static int uvc_v4l2_try_format(struct uvc_streaming *stream,
break;
}
 
-   if (frame == NULL) {
+   if (!frame) {
uvc_trace(UVC_TRACE_FORMAT, "Unsupported size %ux%u.\n",
fmt->fmt.pix.width, fmt->fmt.pix.height);
return -EINVAL;
@@ -260,9 +260,9 @@ static int uvc_v4l2_try_format(struct uvc_streaming *stream,
fmt->fmt.pix.colorspace = format->colorspace;
fmt->fmt.pix.priv = 0;
 
-   if (uvc_format != NULL)
+   if (uvc_format)
*uvc_format = format;
-   if (uvc_frame != NULL)
+   if (uvc_frame)
*uvc_frame = frame;
 
 done:
@@ -282,8 +282,7 @@ static int uvc_v4l2_get_format(struct uvc_streaming *stream,
mutex_lock(>mutex);
format = stream->cur_format;
frame = stream->cur_frame;
-
-   if (format == NULL || frame == NULL) {
+   if (!format || !frame) {
ret = -EINVAL;
goto done;
}
@@ -499,7 +498,7 @@ static int uvc_v4l2_open(struct file *file)
 
/* Create the device handle. */
handle = kzalloc(sizeof *handle, GFP_KERNEL);
-   if (handle == NULL) {
+   if (!handle) {
usb_autopm_put_interface(stream->dev->intf);
return -ENOMEM;
}
@@ -811,7 +810,7 @@ static int uvc_ioctl_enum_input(struct file *file, void *fh,
u32 index = input->index;
int pin = 0;
 
-   if (selector == NULL ||
+   if (!selector ||
(chain->dev->quirks & UVC_QUIRK_IGNORE_SELECTOR_UNIT)) {
if (index != 0)
return -EINVAL;
@@ -830,7 +829,7 @@ static int uvc_ioctl_enum_input(struct file *file, void *fh,
}
}
 
-   if (iterm == NULL || iterm->id != pin)
+   if (!iterm || iterm->id != pin)
return -EINVAL;
 
memset(input, 0, sizeof(*input));
@@ -849,7 +848,7 @@ static int uvc_ioctl_g_input(struct file *file, void *fh, 
unsigned int *input)
int ret;
u8 i;
 
-   if (chain->selector == NULL ||
+   if (!chain->selector ||
(chain->dev->quirks & UVC_QUIRK_IGNORE_SELECTOR_UNIT)) {
*input = 0;
return 0;
@@ -876,7 +875,7 @@ static int uvc_ioctl_s_input(struct file *file, void *fh, 
unsigned int input)
if (ret < 0)
return ret;
 
-   if (chain->selector == NULL ||
+   if (!chain->selector ||
(chain->dev->quirks & UVC_QUIRK_IGNORE_SELECTOR_UNIT)) {
if (input)
return -EINVAL;
@@ -1160,7 +1159,7 @@ static int uvc_ioctl_enum_framesizes(struct file *file, 
void *fh,
break;
}
}
-   if (format == NULL)
+   if (!format)
return -EINVAL;
 
if (fsize->index >= format->nframes)
@@ -1189,7 +1188,7 @@ static int uvc_ioctl_enum_frameintervals(struct file 
*file, void *fh,
break;
}
}
-   if (format == NULL)
+   if (!format)
return -EINVAL;
 
for (i = 0; i < format->nframes; i++) {
@@ -1199,7 +1198,7 @@ static int uvc_ioctl_enum_frameintervals(struct file 
*file, void *fh,
break;
}
}
-   if (frame == NULL)
+   if (!frame)
return -EINVAL;
 
if (frame->bFrameIntervalType) {
-- 
2.14.1

[PATCH 1/3] [media] uvcvideo: Use common error handling code in uvc_ioctl_g_ext_ctrls()

[SF Markus Elfring]

From: Markus Elfring 
Date: Thu, 21 Sep 2017 20:47:02 +0200

Add a jump target 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 
---
 drivers/media/usb/uvc/uvc_v4l2.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index 3e7e283a44a8..6ec2b255c44a 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -998,10 +998,8 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file, void 
*fh,
struct v4l2_queryctrl qc = { .id = ctrl->id };
 
ret = uvc_query_v4l2_ctrl(chain, );
-   if (ret < 0) {
-   ctrls->error_idx = i;
-   return ret;
-   }
+   if (ret < 0)
+   goto set_index;
 
ctrl->value = qc.default_value;
}
@@ -1017,14 +1015,17 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file, 
void *fh,
ret = uvc_ctrl_get(chain, ctrl);
if (ret < 0) {
uvc_ctrl_rollback(handle);
-   ctrls->error_idx = i;
-   return ret;
+   goto set_index;
}
}
 
ctrls->error_idx = 0;
 
return uvc_ctrl_rollback(handle);
+
+set_index:
+   ctrls->error_idx = i;
+   return ret;
 }
 
 static int uvc_ioctl_s_try_ext_ctrls(struct uvc_fh *handle,
-- 
2.14.1

[PATCH 0/3] [media] uvcvideo: Fine-tuning for some function implementations

[SF Markus Elfring]

From: Markus Elfring 
Date: Thu, 21 Sep 2017 21:20:12 +0200

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

Markus Elfring (3):
  Use common error handling code in uvc_ioctl_g_ext_ctrls()
  Adjust 14 checks for null pointers
  Add some spaces for better code readability

 drivers/media/usb/uvc/uvc_v4l2.c | 53 
 1 file changed, 27 insertions(+), 26 deletions(-)

-- 
2.14.1

[PATCH 4/4] [media] usbvision-core: Replace four printk() calls by dev_err()

[SF Markus Elfring]

From: Markus Elfring 
Date: Thu, 21 Sep 2017 16:47:28 +0200

* Replace the local variable "proc" by the identifier "__func__".

* Use the interface "dev_err" instead of "printk" in these functions.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/usbvision/usbvision-core.c | 18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/media/usb/usbvision/usbvision-core.c 
b/drivers/media/usb/usbvision/usbvision-core.c
index 54db35b03106..2c98805244df 100644
--- a/drivers/media/usb/usbvision/usbvision-core.c
+++ b/drivers/media/usb/usbvision/usbvision-core.c
@@ -1619,7 +1619,6 @@ static int usbvision_init_webcam(struct usb_usbvision 
*usbvision)
  */
 static int usbvision_set_video_format(struct usb_usbvision *usbvision, int 
format)
 {
-   static const char proc[] = "usbvision_set_video_format";
unsigned char *value = usbvision->ctrl_urb_buffer;
int rc;
 
@@ -1631,8 +1630,9 @@ static int usbvision_set_video_format(struct 
usb_usbvision *usbvision, int forma
if ((format != ISOC_MODE_YUV422)
&& (format != ISOC_MODE_YUV420)
&& (format != ISOC_MODE_COMPRESS)) {
-   printk(KERN_ERR "usbvision: unknown video format %02x, using 
default YUV420",
-  format);
+   dev_err(>dev->dev,
+   "%s: unknown video format %02x, using default YUV420\n",
+   __func__, format);
format = ISOC_MODE_YUV420;
}
value[0] = 0x0A;  /* TODO: See the effect of the filter */
@@ -1643,8 +1643,9 @@ static int usbvision_set_video_format(struct 
usb_usbvision *usbvision, int forma
 USB_RECIP_ENDPOINT, 0,
 (__u16) USBVISION_FILT_CONT, value, 2, HZ);
if (rc < 0)
-   printk(KERN_ERR "%s: ERROR=%d. USBVISION stopped - reconnect or 
reload driver.\n",
-  proc, rc);
+   dev_err(>dev->dev,
+   "%s: ERROR=%d. USBVISION stopped - reconnect or reload 
driver.\n",
+   __func__, rc);
 
usbvision->isoc_mode = format;
return rc;
@@ -2180,7 +2181,8 @@ int usbvision_restart_isoc(struct usb_usbvision 
*usbvision)
 int usbvision_audio_off(struct usb_usbvision *usbvision)
 {
if (usbvision_write_reg(usbvision, USBVISION_IOPIN_REG, 
USBVISION_AUDIO_MUTE) < 0) {
-   printk(KERN_ERR "usbvision_audio_off: can't write reg\n");
+   dev_err(>dev->dev,
+   "%s: can't write reg\n", __func__);
return -1;
}
usbvision->audio_mute = 0;
@@ -2192,7 +2194,9 @@ int usbvision_set_audio(struct usb_usbvision *usbvision, 
int audio_channel)
 {
if (!usbvision->audio_mute) {
if (usbvision_write_reg(usbvision, USBVISION_IOPIN_REG, 
audio_channel) < 0) {
-   printk(KERN_ERR "usbvision_set_audio: can't write iopin 
register for audio switching\n");
+   dev_err(>dev->dev,
+   "%s: can't write iopin register for audio 
switching\n",
+   __func__);
return -1;
}
}
-- 
2.14.1

[PATCH 3/4] [media] usbvision-core: Delete unnecessary braces in 11 functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Thu, 21 Sep 2017 16:24:20 +0200

Do not use curly brackets at some source code places
where a single statement should be sufficient.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/usbvision/usbvision-core.c | 71 
 1 file changed, 31 insertions(+), 40 deletions(-)

diff --git a/drivers/media/usb/usbvision/usbvision-core.c 
b/drivers/media/usb/usbvision/usbvision-core.c
index bb6f4f69165f..54db35b03106 100644
--- a/drivers/media/usb/usbvision/usbvision-core.c
+++ b/drivers/media/usb/usbvision/usbvision-core.c
@@ -188,10 +188,9 @@ static int scratch_free(struct usb_usbvision *usbvision)
int free = usbvision->scratch_read_ptr - usbvision->scratch_write_ptr;
if (free <= 0)
free += scratch_buf_size;
-   if (free) {
+   if (free)
free -= 1;  
/* at least one byte in the buffer must */

/* left blank, otherwise there is no chance to differ between full and empty */
-   }
PDEBUG(DBG_SCRATCH, "return %d\n", free);
 
return free;
@@ -699,11 +698,12 @@ static enum parse_state usbvision_parse_compress(struct 
usb_usbvision *usbvision
 
frame = usbvision->cur_frame;
image_size = frame->frmwidth * frame->frmheight;
-   if ((frame->v4l2_format.format == V4L2_PIX_FMT_YUV422P) ||
-   (frame->v4l2_format.format == V4L2_PIX_FMT_YVU420)) {   /* this 
is a planar format */
+   if (frame->v4l2_format.format == V4L2_PIX_FMT_YUV422P ||
+   frame->v4l2_format.format == V4L2_PIX_FMT_YVU420)
+   /* this is a planar format */
/* ... v4l2_linesize not used here. */
f = frame->data + (frame->width * frame->curline);
-   } else
+   else
f = frame->data + (frame->v4l2_linesize * frame->curline);
 
if (frame->v4l2_format.format == V4L2_PIX_FMT_YUYV) { /* initialise u 
and v pointers */
@@ -734,22 +734,19 @@ static enum parse_state usbvision_parse_compress(struct 
usb_usbvision *usbvision
return parse_state_next_frame;
}
 
-   if (frame->curline != (int)strip_header[2]) {
+   if (frame->curline != (int)strip_header[2])
/* line number mismatch error */
usbvision->strip_line_number_errors++;
-   }
 
strip_len = 2 * (unsigned int)strip_header[1];
-   if (strip_len > USBVISION_STRIP_LEN_MAX) {
+   if (strip_len > USBVISION_STRIP_LEN_MAX)
/* strip overrun */
/* I think this never happens */
usbvision_request_intra(usbvision);
-   }
 
-   if (scratch_len(usbvision) < strip_len) {
+   if (scratch_len(usbvision) < strip_len)
/* there is not enough data for the strip */
return parse_state_out;
-   }
 
if (usbvision->intra_frame_buffer) {
Y = usbvision->intra_frame_buffer + frame->frmwidth * 
frame->curline;
@@ -1306,11 +1303,11 @@ static void usbvision_isoc_irq(struct urb *urb)
/* If we collected enough data let's parse! */
if (scratch_len(usbvision) > USBVISION_HEADER_LENGTH &&
!list_empty(&(usbvision->inqueue))) {
-   if (!(*f)) {
+   if (!(*f))
(*f) = list_entry(usbvision->inqueue.next,
  struct usbvision_frame,
  frame);
-   }
+
usbvision_parse_data(usbvision);
} else {
/* If we don't have a frame
@@ -1334,12 +1331,10 @@ static void usbvision_isoc_irq(struct urb *urb)
urb->status = 0;
urb->dev = usbvision->dev;
err_code = usb_submit_urb(urb, GFP_ATOMIC);
-
-   if (err_code) {
+   if (err_code)
dev_err(>dev->dev,
"%s: usb_submit_urb failed: error %d\n",
__func__, err_code);
-   }
 
return;
 }
@@ -1398,11 +1393,10 @@ int usbvision_write_reg(struct usb_usbvision 
*usbvision, unsigned char reg,
USB_DIR_OUT | USB_TYPE_VENDOR |
USB_RECIP_ENDPOINT, 0, (__u16) reg,
usbvision->ctrl_urb_buffer, 1, HZ);
-
-   if (err_code < 0) {
+   if (err_code < 0)
dev_err(>dev->dev,
"%s: failed: error %d\n", __func__, err_code);
-   }
+
return err_code;
 }
 
@@ -1443,10 +1437,10 @@ static int usbvision_write_reg_irq(struct usb_usbvision 
*usbvision, int address,
memcpy(usbvision->ctrl_urb_buffer, data, len);
 
err_code = 

[PATCH 2/4] [media] usbvision-core: Use common error handling code in usbvision_set_compress_params()

[SF Markus Elfring]

From: Markus Elfring 
Date: Thu, 21 Sep 2017 12:45:49 +0200

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

* Replace the local variable "proc" by the identifier "__func__".

* Use the interface "dev_err" instead of "printk".

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/usbvision/usbvision-core.c | 18 +++---
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/media/usb/usbvision/usbvision-core.c 
b/drivers/media/usb/usbvision/usbvision-core.c
index 16b76c85eeec..bb6f4f69165f 100644
--- a/drivers/media/usb/usbvision/usbvision-core.c
+++ b/drivers/media/usb/usbvision/usbvision-core.c
@@ -1857,7 +1857,6 @@ int usbvision_stream_interrupt(struct usb_usbvision 
*usbvision)
 
 static int usbvision_set_compress_params(struct usb_usbvision *usbvision)
 {
-   static const char proc[] = "usbvision_set_compresion_params: ";
int rc;
unsigned char *value = usbvision->ctrl_urb_buffer;
 
@@ -1882,12 +1881,8 @@ static int usbvision_set_compress_params(struct 
usb_usbvision *usbvision)
 USB_DIR_OUT | USB_TYPE_VENDOR |
 USB_RECIP_ENDPOINT, 0,
 (__u16) USBVISION_INTRA_CYC, value, 5, HZ);
-
-   if (rc < 0) {
-   printk(KERN_ERR "%sERROR=%d. USBVISION stopped - reconnect or 
reload driver.\n",
-  proc, rc);
-   return rc;
-   }
+   if (rc < 0)
+   goto report_failure;
 
if (usbvision->bridge_type == BRIDGE_NT1004) {
value[0] =  20; /* PCM Threshold 1 */
@@ -1913,11 +1908,12 @@ static int usbvision_set_compress_params(struct 
usb_usbvision *usbvision)
 USB_DIR_OUT | USB_TYPE_VENDOR |
 USB_RECIP_ENDPOINT, 0,
 (__u16) USBVISION_PCM_THR1, value, 6, HZ);
+   if (rc < 0)
+report_failure:
+   dev_err(>dev->dev,
+   "%s: ERROR=%d. USBVISION stopped - reconnect or reload 
driver.\n",
+   __func__, rc);
 
-   if (rc < 0) {
-   printk(KERN_ERR "%sERROR=%d. USBVISION stopped - reconnect or 
reload driver.\n",
-  proc, rc);
-   }
return rc;
 }
 
-- 
2.14.1

[PATCH 1/4] [media] usbvision-core: Use common error handling code in usbvision_set_input()

[SF Markus Elfring]

From: Markus Elfring 
Date: Thu, 21 Sep 2017 11:50:54 +0200

* Add a jump target 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.

* Replace the local variable "proc" by the identifier "__func__".

* Use the interface "dev_err" instead of "printk".

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/usbvision/usbvision-core.c | 23 ++-
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/media/usb/usbvision/usbvision-core.c 
b/drivers/media/usb/usbvision/usbvision-core.c
index 3f87fbc80be2..16b76c85eeec 100644
--- a/drivers/media/usb/usbvision/usbvision-core.c
+++ b/drivers/media/usb/usbvision/usbvision-core.c
@@ -1931,7 +1931,6 @@ static int usbvision_set_compress_params(struct 
usb_usbvision *usbvision)
  */
 int usbvision_set_input(struct usb_usbvision *usbvision)
 {
-   static const char proc[] = "usbvision_set_input: ";
int rc;
unsigned char *value = usbvision->ctrl_urb_buffer;
unsigned char dvi_yuv_value;
@@ -1953,12 +1952,8 @@ int usbvision_set_input(struct usb_usbvision *usbvision)
}
 
rc = usbvision_write_reg(usbvision, USBVISION_VIN_REG1, value[0]);
-   if (rc < 0) {
-   printk(KERN_ERR "%sERROR=%d. USBVISION stopped - reconnect or 
reload driver.\n",
-  proc, rc);
-   return rc;
-   }
-
+   if (rc < 0)
+   goto report_failure;
 
if (usbvision->tvnorm_id & V4L2_STD_PAL) {
value[0] = 0xC0;
@@ -2019,12 +2014,8 @@ int usbvision_set_input(struct usb_usbvision *usbvision)
 USBVISION_OP_CODE, /* USBVISION specific code */
 USB_DIR_OUT | USB_TYPE_VENDOR | 
USB_RECIP_ENDPOINT, 0,
 (__u16) USBVISION_LXSIZE_I, value, 8, HZ);
-   if (rc < 0) {
-   printk(KERN_ERR "%sERROR=%d. USBVISION stopped - reconnect or 
reload driver.\n",
-  proc, rc);
-   return rc;
-   }
-
+   if (rc < 0)
+   goto report_failure;
 
dvi_yuv_value = 0x00;   /* U comes after V, Ya comes after U/V, Yb 
comes after Yb */
 
@@ -2036,6 +2027,12 @@ int usbvision_set_input(struct usb_usbvision *usbvision)
}
 
return usbvision_write_reg(usbvision, USBVISION_DVI_YUV, dvi_yuv_value);
+
+report_failure:
+   dev_err(>dev->dev,
+   "%s: ERROR=%d. USBVISION stopped - reconnect or reload 
driver.\n",
+   __func__, rc);
+   return rc;
 }
 
 
-- 
2.14.1

[PATCH 0/4] [media] usbvision-core: Fine-tuning for some function implementations

[SF Markus Elfring]

From: Markus Elfring 
Date: Thu, 21 Sep 2017 17:00:17 +0200

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

Markus Elfring (4):
  Use common error handling code in usbvision_set_input()
  Use common error handling code in usbvision_set_compress_params()
  Delete unnecessary braces in 11 functions
  Replace four printk() calls by dev_err()

 drivers/media/usb/usbvision/usbvision-core.c | 128 ---
 1 file changed, 58 insertions(+), 70 deletions(-)

-- 
2.14.1

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

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

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

[SF Markus Elfring]

From: Markus Elfring 
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 
---
 drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c 
b/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
index fef3c8554e91..2e97b1e64249 100644
--- a/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
+++ b/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
@@ -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, _stv0299_config, 
>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, 
_novas_grundig_29504_491_config, >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, _tdbe2_config, 
>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, 
_philips_tdm1316l_config, >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, _tdmb7_config, 
>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, 
_tdm1316l_config, >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(>dvbnet);
dvb_dmxdev_release(>dmxdev);
dvb_dmx_release(>dvb_demux);
-   if (ttusb->fe != NULL) {
+   if (ttusb->fe) {
dvb_unregister_frontend(ttusb->fe);
dvb_frontend_detach(ttusb->fe);
}
-- 
2.14.1

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

[SF Markus Elfring]

From: Markus Elfring 
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 
---
 drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c 
b/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
index 38394c9ecc67..fef3c8554e91 100644
--- a/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
+++ b/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
@@ -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(>i2c_adap, 0, sizeof(struct i2c_adapter));
+   memset(>i2c_adap, 0, sizeof(ttusb->i2c_adap));
strcpy(ttusb->i2c_adap.name, "TTUSB DEC");
 
i2c_set_adapdata(>i2c_adap, ttusb);
-- 
2.14.1

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

[SF Markus Elfring]

From: Markus Elfring 
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 
---
 drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c 
b/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
index b842f367249f..38394c9ecc67 100644
--- a/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
+++ b/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
@@ -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(>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(>adapter,
  "Technotrend/Hauppauge Nova-USB",
  THIS_MODULE, >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(>i2c_adap);
 err_unregister_adapter:
dvb_unregister_adapter (>adapter);
+err_free_iso_urbs:
ttusb_free_iso_urbs(ttusb);
+err_free_usb:
kfree(ttusb);
return result;
 }
-- 
2.14.1

[PATCH 0/3] [media] TTUSB DVB Budget: Fine-tuning for three function implementations

[SF Markus Elfring]

From: Markus Elfring 
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(-)

-- 
2.14.1

Re: [media] Siano: Use common error handling code in smsusb_init_device()

[SF Markus Elfring]

> If smscore_register_device() succeeds then mdev is freed when we call
> smsusb_term_device(intf);  The call tree is:

Thanks for your constructive information.


How do you think about another implementation detail in this function then?

May the statement “kfree(mdev);” be executed before “smsusb_term_device(intf);”
in one if branch?

Regards,
Markus

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

[SF Markus Elfring]

From: Markus Elfring 
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 
---
 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, 0x, 0x, 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(>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

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

[SF Markus Elfring]

From: Markus Elfring 
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 
---
 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(>dev->dev, "no context!\n");
-   return;
-   }
+   if (!pipe_info)
+   goto report_failure;
+
dev = pipe_info->dev;
-   if (!dev) {
-   dev_err(>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(>dev->dev, "no context!\n");
 }
 
 static int s2255_start_readpipe(struct s2255_dev *dev)
-- 
2.14.1

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

[SF Markus Elfring]

From: Markus Elfring 
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 
---
 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 2/5] [media] s2255drv: Adjust 13 checks for null pointers

[SF Markus Elfring]

From: Markus Elfring 
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 
---
 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(>wait_fw);
return;
}
-   if (data->fw_urb == NULL) {
+   if (!data->fw_urb) {
s2255_dev_err(>dev, "disconnected\n");
atomic_set(>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(>dev->dev, "no context!\n");
return;
}
dev = pipe_info->dev;
-   if (dev == NULL) {
+   if (!dev) {
dev_err(>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(>dev, "null usb device\n");
retval = -ENODEV;
goto errorUDEV;
-- 
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 
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 
---
 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(>dev, "out of memory\n");
+   if (!dev)
return -ENOMEM;
-   }
 
dev->cmdbuf = kzalloc(S2255_CMDBUF_SIZE, GFP_KERNEL);
-   if (dev->cmdbuf == NULL) {
-   s2255_dev_err(>dev, "out of memory\n");
+   if (!dev->cmdbuf)
goto errorFWDATA1;
-   }
 
atomic_set(>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(>dev, "out of memory!\n");
+   if (!dev->fw_data->pfw_data)
goto errorFWDATA2;
-   }
+
/* load the first chunk */
if (request_firmware(>fw_data->fw,
 FIRMWARE_FILE_NAME, >udev->dev)) {
-- 
2.14.1

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

[SF Markus Elfring]

From: Markus Elfring 
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] [media] Siano: Use common error handling code in smsusb_init_device()

[SF Markus Elfring]

From: Markus Elfring 
Date: Wed, 20 Sep 2017 14:30:55 +0200

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

This refactoring might fix also an error situation where the
function "kfree" was not called after a software failure
was noticed in two cases.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/siano/smsusb.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/media/usb/siano/smsusb.c b/drivers/media/usb/siano/smsusb.c
index 8c1f926567ec..b8e7b05cf6d0 100644
--- a/drivers/media/usb/siano/smsusb.c
+++ b/drivers/media/usb/siano/smsusb.c
@@ -458,12 +458,10 @@ static int smsusb_init_device(struct usb_interface *intf, 
int board_id)
rc = smscore_register_device(, >coredev, mdev);
if (rc < 0) {
pr_err("smscore_register_device(...) failed, rc %d\n", rc);
-   smsusb_term_device(intf);
 #ifdef CONFIG_MEDIA_CONTROLLER_DVB
media_device_unregister(mdev);
 #endif
-   kfree(mdev);
-   return rc;
+   goto terminate_device;
}
 
smscore_set_board_id(dev->coredev, board_id);
@@ -480,8 +478,7 @@ static int smsusb_init_device(struct usb_interface *intf, 
int board_id)
rc = smsusb_start_streaming(dev);
if (rc < 0) {
pr_err("smsusb_start_streaming(...) failed\n");
-   smsusb_term_device(intf);
-   return rc;
+   goto terminate_device;
}
 
dev->state = SMSUSB_ACTIVE;
@@ -489,13 +486,17 @@ static int smsusb_init_device(struct usb_interface *intf, 
int board_id)
rc = smscore_start_device(dev->coredev);
if (rc < 0) {
pr_err("smscore_start_device(...) failed\n");
-   smsusb_term_device(intf);
-   return rc;
+   goto terminate_device;
}
 
pr_debug("device 0x%p created\n", dev);
 
return rc;
+
+terminate_device:
+   smsusb_term_device(intf);
+   kfree(mdev);
+   return rc;
 }
 
 static int smsusb_probe(struct usb_interface *intf,
-- 
2.14.1

Re: [PATCH 4/6] [media] go7007: Use common error handling code in s2250_probe()

[SF Markus Elfring]

>> @@ -555,17 +553,13 @@ static int s2250_probe(struct i2c_client *client,
>>  /* initialize the audio */
>>  if (write_regs(audio, aud_regs) < 0) {
>>  dev_err(>dev, "error initializing audio\n");
>> -goto fail;
>> +goto e_io;
> 
> Preserve the error code.

Do you suggest then to adjust the implementation of the function "write_regs"
so that a more meaningful value would be used instead of the failure indication 
"-1"?

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/media/usb/go7007/s2250-board.c?h=v4.14-rc1#n302
http://elixir.free-electrons.com/linux/v4.14-rc1/source/drivers/media/usb/go7007/s2250-board.c#L298

Regards,
Markus

[PATCH 3/3] [media] pvrusb2-ioread: Delete unnecessary braces in six functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Wed, 20 Sep 2017 08:15:51 +0200

Do not use curly brackets at some source code places
where a single statement should be sufficient.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/pvrusb2/pvrusb2-ioread.c | 38 --
 1 file changed, 15 insertions(+), 23 deletions(-)

diff --git a/drivers/media/usb/pvrusb2/pvrusb2-ioread.c 
b/drivers/media/usb/pvrusb2/pvrusb2-ioread.c
index 4349f9b5f838..9a0eb2875c9a 100644
--- a/drivers/media/usb/pvrusb2/pvrusb2-ioread.c
+++ b/drivers/media/usb/pvrusb2/pvrusb2-ioread.c
@@ -120,9 +120,8 @@ void pvr2_ioread_set_sync_key(struct pvr2_ioread *cp,
cp->sync_key_len = 0;
if (sync_key_len) {
cp->sync_key_ptr = kmalloc(sync_key_len,GFP_KERNEL);
-   if (cp->sync_key_ptr) {
+   if (cp->sync_key_ptr)
cp->sync_key_len = sync_key_len;
-   }
}
}
if (!cp->sync_key_len) return;
@@ -203,9 +202,9 @@ int pvr2_ioread_setup(struct pvr2_ioread *cp,struct 
pvr2_stream *sp)
   cp);
pvr2_ioread_stop(cp);
pvr2_stream_kill(cp->stream);
-   if (pvr2_stream_get_buffer_count(cp->stream)) {
+   if (pvr2_stream_get_buffer_count(cp->stream))
pvr2_stream_set_buffer_count(cp->stream,0);
-   }
+
cp->stream = NULL;
}
if (sp) {
@@ -238,13 +237,10 @@ int pvr2_ioread_set_enabled(struct pvr2_ioread *cp,int fl)
if ((!fl) == (!(cp->enabled))) return ret;
 
mutex_lock(>mutex);
-   do {
-   if (fl) {
-   ret = pvr2_ioread_start(cp);
-   } else {
-   pvr2_ioread_stop(cp);
-   }
-   } while (0);
+   if (fl)
+   ret = pvr2_ioread_start(cp);
+   else
+   pvr2_ioread_stop(cp);
mutex_unlock(>mutex);
return ret;
 }
@@ -318,13 +314,12 @@ static void pvr2_ioread_filter(struct pvr2_ioread *cp)
for (idx = cp->c_data_offs; idx < cp->c_data_len; idx++) {
if (cp->sync_buf_offs >= cp->sync_key_len) break;
if (cp->c_data_ptr[idx] ==
-   cp->sync_key_ptr[cp->sync_buf_offs]) {
+   cp->sync_key_ptr[cp->sync_buf_offs])
// Found the next key byte
(cp->sync_buf_offs)++;
-   } else {
+   else
// Whoops, mismatched.  Start key over...
cp->sync_buf_offs = 0;
-   }
}
 
// Consume what we've walked through
@@ -360,10 +355,10 @@ static void pvr2_ioread_filter(struct pvr2_ioread *cp)
 int pvr2_ioread_avail(struct pvr2_ioread *cp)
 {
int ret;
-   if (!(cp->enabled)) {
+
+   if (!cp->enabled)
// Stream is not enabled; so this is an I/O error
return -EIO;
-   }
 
if (cp->sync_state == 1) {
pvr2_ioread_filter(cp);
@@ -372,15 +367,13 @@ int pvr2_ioread_avail(struct pvr2_ioread *cp)
 
ret = 0;
if (cp->stream_running) {
-   if (!pvr2_stream_get_ready_count(cp->stream)) {
+   if (!pvr2_stream_get_ready_count(cp->stream))
// No data available at all right now.
ret = -EAGAIN;
-   }
} else {
-   if (pvr2_stream_get_ready_count(cp->stream) < BUFFER_COUNT/2) {
+   if (pvr2_stream_get_ready_count(cp->stream) < BUFFER_COUNT / 2)
// Haven't buffered up enough yet; try again later
ret = -EAGAIN;
-   }
}
 
if ((!(cp->spigot_open)) != (!(ret == 0))) {
@@ -476,14 +469,13 @@ cp);
mutex_unlock(>mutex);
 
if (!ret) {
-   if (copied_cnt) {
+   if (copied_cnt)
// If anything was copied, return that count
ret = copied_cnt;
-   } else {
+   else
// Nothing copied; suggest to caller that another
// attempt should be tried again later
ret = -EAGAIN;
-   }
}
 
pvr2_trace(PVR2_TRACE_DATA_FLOW,
-- 
2.14.1

[PATCH 2/3] [media] pvrusb2-ioread: Delete an unnecessary check before kfree() in two functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 19 Sep 2017 22:12:49 +0200

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

WARNING: kfree(NULL) is safe and this check is probably not required

Thus fix the affected source code places.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/pvrusb2/pvrusb2-ioread.c | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/media/usb/pvrusb2/pvrusb2-ioread.c 
b/drivers/media/usb/pvrusb2/pvrusb2-ioread.c
index 0218614ce988..4349f9b5f838 100644
--- a/drivers/media/usb/pvrusb2/pvrusb2-ioread.c
+++ b/drivers/media/usb/pvrusb2/pvrusb2-ioread.c
@@ -98,10 +98,8 @@ void pvr2_ioread_destroy(struct pvr2_ioread *cp)
if (!cp) return;
pvr2_ioread_done(cp);
pvr2_trace(PVR2_TRACE_STRUCT,"pvr2_ioread_destroy id=%p",cp);
-   if (cp->sync_key_ptr) {
-   kfree(cp->sync_key_ptr);
-   cp->sync_key_ptr = NULL;
-   }
+   kfree(cp->sync_key_ptr);
+   cp->sync_key_ptr = NULL;
kfree(cp);
 }
 
@@ -117,10 +115,8 @@ void pvr2_ioread_set_sync_key(struct pvr2_ioread *cp,
 (!memcmp(sync_key_ptr,cp->sync_key_ptr,sync_key_len return;
 
if (sync_key_len != cp->sync_key_len) {
-   if (cp->sync_key_ptr) {
-   kfree(cp->sync_key_ptr);
-   cp->sync_key_ptr = NULL;
-   }
+   kfree(cp->sync_key_ptr);
+   cp->sync_key_ptr = NULL;
cp->sync_key_len = 0;
if (sync_key_len) {
cp->sync_key_ptr = kmalloc(sync_key_len,GFP_KERNEL);
-- 
2.14.1

[PATCH 1/3] [media] pvrusb2-ioread: Use common error handling code in pvr2_ioread_get_buffer()

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 19 Sep 2017 21:50:05 +0200

Add a jump target 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 
---
 drivers/media/usb/pvrusb2/pvrusb2-ioread.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/media/usb/pvrusb2/pvrusb2-ioread.c 
b/drivers/media/usb/pvrusb2/pvrusb2-ioread.c
index 602097bdcf14..0218614ce988 100644
--- a/drivers/media/usb/pvrusb2/pvrusb2-ioread.c
+++ b/drivers/media/usb/pvrusb2/pvrusb2-ioread.c
@@ -266,8 +266,7 @@ static int pvr2_ioread_get_buffer(struct pvr2_ioread *cp)
pvr2_trace(PVR2_TRACE_DATA_FLOW,
   "/*---TRACE_READ---*/ 
pvr2_ioread_read id=%p queue_error=%d",
   cp,stat);
-   pvr2_ioread_stop(cp);
-   return 0;
+   goto stop_read;
}
cp->c_buf = NULL;
cp->c_data_ptr = NULL;
@@ -286,9 +285,8 @@ static int pvr2_ioread_get_buffer(struct pvr2_ioread *cp)
pvr2_trace(PVR2_TRACE_DATA_FLOW,
   "/*---TRACE_READ---*/ 
pvr2_ioread_read id=%p buffer_error=%d",
   cp,stat);
-   pvr2_ioread_stop(cp);
// Give up.
-   return 0;
+   goto stop_read;
}
// Start over...
continue;
@@ -298,6 +296,10 @@ static int pvr2_ioread_get_buffer(struct pvr2_ioread *cp)
pvr2_buffer_get_id(cp->c_buf)];
}
return !0;
+
+stop_read:
+   pvr2_ioread_stop(cp);
+   return 0;
 }
 
 static void pvr2_ioread_filter(struct pvr2_ioread *cp)
-- 
2.14.1

[PATCH 0/3] [media] pvrusb2-ioread: Fine-tuning for eight function implementations

[SF Markus Elfring]

From: Markus Elfring 
Date: Wed, 20 Sep 2017 08:28:48 +0200

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

Markus Elfring (3):
  Use common error handling code in pvr2_ioread_get_buffer()
  Delete an unnecessary check before kfree() in two functions
  Delete unnecessary braces in six functions

 drivers/media/usb/pvrusb2/pvrusb2-ioread.c | 60 +-
 1 file changed, 25 insertions(+), 35 deletions(-)

-- 
2.14.1

[PATCH 3/3] [media] hdpvr: Return an error code only as a constant in hdpvr_alloc_buffers()

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 19 Sep 2017 19:32:36 +0200

* Return an error code without storing it in an intermediate variable.

* Delete the local variable "retval" which became unnecessary
  with this refactoring.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/hdpvr/hdpvr-video.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/usb/hdpvr/hdpvr-video.c 
b/drivers/media/usb/hdpvr/hdpvr-video.c
index f06efcd70c14..2b39834309d2 100644
--- a/drivers/media/usb/hdpvr/hdpvr-video.c
+++ b/drivers/media/usb/hdpvr/hdpvr-video.c
@@ -136,6 +136,5 @@ int hdpvr_free_buffers(struct hdpvr_device *dev)
 int hdpvr_alloc_buffers(struct hdpvr_device *dev, uint count)
 {
uint i;
-   int retval = -ENOMEM;
u8 *mem;
struct hdpvr_buffer *buf;
@@ -181,6 +180,6 @@ int hdpvr_alloc_buffers(struct hdpvr_device *dev, uint 
count)
kfree(buf);
 exit:
hdpvr_free_buffers(dev);
-   return retval;
+   return -ENOMEM;
 }
 
-- 
2.14.1

[PATCH 2/3] [media] hdpvr: Improve a size determination in hdpvr_alloc_buffers()

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 19 Sep 2017 19:27:53 +0200

Replace the specification of a data structure by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/hdpvr/hdpvr-video.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/usb/hdpvr/hdpvr-video.c 
b/drivers/media/usb/hdpvr/hdpvr-video.c
index 657d910dfa1d..f06efcd70c14 100644
--- a/drivers/media/usb/hdpvr/hdpvr-video.c
+++ b/drivers/media/usb/hdpvr/hdpvr-video.c
@@ -145,6 +145,5 @@ int hdpvr_alloc_buffers(struct hdpvr_device *dev, uint 
count)
 "allocating %u buffers\n", count);
 
for (i = 0; i < count; i++) {
-
-   buf = kzalloc(sizeof(struct hdpvr_buffer), GFP_KERNEL);
+   buf = kzalloc(sizeof(*buf), GFP_KERNEL);
if (!buf)
-- 
2.14.1

[PATCH 1/3] [media] hdpvr: Delete three error messages for a failed memory allocation

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 19 Sep 2017 09:33:26 +0200

Omit extra messages for a memory allocation failure in these functions.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/hdpvr/hdpvr-core.c  | 8 ++--
 drivers/media/usb/hdpvr/hdpvr-video.c | 5 ++---
 2 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/media/usb/hdpvr/hdpvr-core.c 
b/drivers/media/usb/hdpvr/hdpvr-core.c
index dbe29c6c4d8b..f1c156814e10 100644
--- a/drivers/media/usb/hdpvr/hdpvr-core.c
+++ b/drivers/media/usb/hdpvr/hdpvr-core.c
@@ -282,6 +282,4 @@ static int hdpvr_probe(struct usb_interface *interface,
-   if (!dev) {
-   dev_err(>dev, "Out of memory\n");
+   if (!dev)
goto error;
-   }
 
/* init video transfer queues first of all */
@@ -302,6 +300,4 @@ static int hdpvr_probe(struct usb_interface *interface,
-   if (!dev->usbc_buf) {
-   v4l2_err(>v4l2_dev, "Out of memory\n");
+   if (!dev->usbc_buf)
goto error;
-   }
 
init_waitqueue_head(>wait_buffer);
diff --git a/drivers/media/usb/hdpvr/hdpvr-video.c 
b/drivers/media/usb/hdpvr/hdpvr-video.c
index 7fb036d6a86e..657d910dfa1d 100644
--- a/drivers/media/usb/hdpvr/hdpvr-video.c
+++ b/drivers/media/usb/hdpvr/hdpvr-video.c
@@ -150,7 +150,6 @@ int hdpvr_alloc_buffers(struct hdpvr_device *dev, uint 
count)
-   if (!buf) {
-   v4l2_err(>v4l2_dev, "cannot allocate buffer\n");
+   if (!buf)
goto exit;
-   }
+
buf->dev = dev;
 
urb = usb_alloc_urb(0, GFP_KERNEL);
-- 
2.14.1

[PATCH 0/3] [media] HD PVR USB: Adjustments for two function implementations

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 19 Sep 2017 20:21:23 +0200

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

Markus Elfring (3):
  Delete three error messages for a failed memory allocation
  Improve a size determination in hdpvr_alloc_buffers()
  Return an error code only as a constant in hdpvr_alloc_buffers()

 drivers/media/usb/hdpvr/hdpvr-core.c  |  8 ++--
 drivers/media/usb/hdpvr/hdpvr-video.c | 11 ---
 2 files changed, 6 insertions(+), 13 deletions(-)

-- 
2.14.1

[PATCH 2/2] [media] vicam: Return directly after a failed kmalloc() in vicam_dostream()

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 18 Sep 2017 21:56:55 +0200

* Return directly after a call of the function "kmalloc" failed
  at the beginning.

* Delete the jump target "exit" which became unnecessary
  with this refactoring.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/gspca/vicam.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/gspca/vicam.c b/drivers/media/usb/gspca/vicam.c
index 15b6887a8e97..11508ab283cd 100644
--- a/drivers/media/usb/gspca/vicam.c
+++ b/drivers/media/usb/gspca/vicam.c
@@ -184,7 +184,7 @@ static void vicam_dostream(struct work_struct *work)
   HEADER_SIZE;
buffer = kmalloc(frame_sz, GFP_KERNEL | GFP_DMA);
if (!buffer)
-   goto exit;
+   return;
 
while (gspca_dev->present && gspca_dev->streaming) {
 #ifdef CONFIG_PM
@@ -205,7 +205,7 @@ static void vicam_dostream(struct work_struct *work)
frame_sz - HEADER_SIZE);
gspca_frame_add(gspca_dev, LAST_PACKET, NULL, 0);
}
-exit:
+
kfree(buffer);
 }
 
-- 
2.14.1

[PATCH 1/2] [media] vicam: Delete an error message for a failed memory allocation in vicam_dostream()

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 18 Sep 2017 21:48:55 +0200

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

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/gspca/vicam.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/media/usb/gspca/vicam.c b/drivers/media/usb/gspca/vicam.c
index 554b90ef2200..15b6887a8e97 100644
--- a/drivers/media/usb/gspca/vicam.c
+++ b/drivers/media/usb/gspca/vicam.c
@@ -183,10 +183,8 @@ static void vicam_dostream(struct work_struct *work)
frame_sz = gspca_dev->cam.cam_mode[gspca_dev->curr_mode].sizeimage +
   HEADER_SIZE;
buffer = kmalloc(frame_sz, GFP_KERNEL | GFP_DMA);
-   if (!buffer) {
-   pr_err("Couldn't allocate USB buffer\n");
+   if (!buffer)
goto exit;
-   }
 
while (gspca_dev->present && gspca_dev->streaming) {
 #ifdef CONFIG_PM
-- 
2.14.1

[PATCH 0/2] [media] ViCam: Adjustments for vicam_dostream()

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 18 Sep 2017 22:05:22 +0200

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

Markus Elfring (2):
  Delete an error message for a failed memory allocation
  Return directly after a failed kmalloc()

 drivers/media/usb/gspca/vicam.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

-- 
2.14.1

[PATCH] [media] sq905: Delete an error message for a failed memory allocation in sq905_dostream()

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 18 Sep 2017 21:30:58 +0200

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

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/gspca/sq905.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/media/usb/gspca/sq905.c b/drivers/media/usb/gspca/sq905.c
index f1da34a10ce8..ca8cdd753a2b 100644
--- a/drivers/media/usb/gspca/sq905.c
+++ b/drivers/media/usb/gspca/sq905.c
@@ -218,10 +218,8 @@ static void sq905_dostream(struct work_struct *work)
u8 *buffer;
 
buffer = kmalloc(SQ905_MAX_TRANSFER, GFP_KERNEL | GFP_DMA);
-   if (!buffer) {
-   pr_err("Couldn't allocate USB buffer\n");
+   if (!buffer)
goto quit_stream;
-   }
 
frame_sz = gspca_dev->cam.cam_mode[gspca_dev->curr_mode].sizeimage
+ FRAME_HEADER_LEN;
-- 
2.14.1

[PATCH] [media] jl2005bcd: Delete an error message for a failed memory allocation in jl2005c_dostream()

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 18 Sep 2017 19:24:24 +0200

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

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/gspca/jl2005bcd.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/media/usb/gspca/jl2005bcd.c 
b/drivers/media/usb/gspca/jl2005bcd.c
index 17c7a953564c..23400ead1919 100644
--- a/drivers/media/usb/gspca/jl2005bcd.c
+++ b/drivers/media/usb/gspca/jl2005bcd.c
@@ -320,10 +320,8 @@ static void jl2005c_dostream(struct work_struct *work)
u8 *buffer;
 
buffer = kmalloc(JL2005C_MAX_TRANSFER, GFP_KERNEL | GFP_DMA);
-   if (!buffer) {
-   pr_err("Couldn't allocate USB buffer\n");
+   if (!buffer)
goto quit_stream;
-   }
 
while (gspca_dev->present && gspca_dev->streaming) {
 #ifdef CONFIG_PM
-- 
2.14.1

[PATCH] [media] gspca: Use common error handling code in gspca_init_transfer()

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 18 Sep 2017 18:40:05 +0200

Add a jump target 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 
---
 drivers/media/usb/gspca/gspca.c | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/media/usb/gspca/gspca.c b/drivers/media/usb/gspca/gspca.c
index 0f141762abf1..22cdefe38e07 100644
--- a/drivers/media/usb/gspca/gspca.c
+++ b/drivers/media/usb/gspca/gspca.c
@@ -904,10 +904,8 @@ static int gspca_init_transfer(struct gspca_dev *gspca_dev)
ret = create_urbs(gspca_dev,
alt_xfer(>altsetting[alt], xfer,
 gspca_dev->xfer_ep));
-   if (ret < 0) {
-   destroy_urbs(gspca_dev);
-   goto out;
-   }
+   if (ret < 0)
+   goto destroy_urbs;
}
 
/* clear the bulk endpoint */
@@ -917,10 +915,9 @@ static int gspca_init_transfer(struct gspca_dev *gspca_dev)
 
/* start the cam */
ret = gspca_dev->sd_desc->start(gspca_dev);
-   if (ret < 0) {
-   destroy_urbs(gspca_dev);
-   goto out;
-   }
+   if (ret < 0)
+   goto destroy_urbs;
+
gspca_dev->streaming = 1;
v4l2_ctrl_handler_setup(gspca_dev->vdev.ctrl_handler);
 
@@ -970,6 +967,10 @@ static int gspca_init_transfer(struct gspca_dev *gspca_dev)
 out:
gspca_input_create_urb(gspca_dev);
return ret;
+
+destroy_urbs:
+   destroy_urbs(gspca_dev);
+   goto out;
 }
 
 static void gspca_set_default_mode(struct gspca_dev *gspca_dev)
-- 
2.14.1