If you are convinced that dropping the null tests is a good idea, then you can submit the patch that makes the change to the relevant maintainers and mailing lists.
Would you like to integrate the following proposal into your source code repository?
Regards, Markus
From e61965bbcb143a54696fbd468989110519e41497 Mon Sep 17 00:00:00 2001
From: Markus Elfring elfring@users.sourceforge.net Date: Wed, 22 Oct 2014 18:28:12 +0200 Subject: [PATCH] GPU-DRM-GMA500: Deletion of unnecessary checks before two function calls
A semantic patch approach was proposed with the subject "[PATCH with Coccinelle?] Deletion of unnecessary checks before specific function calls" on 2014-03-05. https://lkml.org/lkml/2014/3/5/344 http://article.gmane.org/gmane.comp.version-control.coccinelle/3513/
This patch pattern application was repeated with the help of the software "Coccinelle 1.0.0-rc22" on the source files for Linux 3.17.1. An extract of the automatically generated update suggestions is shown here.
It was determined that the affected source code places call functions which perform input parameter validation already. It is therefore not needed that a similar safety check is repeated at the call site.
Signed-off-by: Markus Elfring elfring@users.sourceforge.net --- drivers/gpu/drm/gma500/cdv_intel_hdmi.c | 3 +-- drivers/gpu/drm/gma500/cdv_intel_lvds.c | 9 +++------ drivers/gpu/drm/gma500/oaktrail_lvds.c | 3 +-- drivers/gpu/drm/gma500/psb_drv.c | 3 +-- drivers/gpu/drm/gma500/psb_intel_lvds.c | 9 +++------ 5 files changed, 9 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/gma500/cdv_intel_hdmi.c b/drivers/gpu/drm/gma500/cdv_intel_hdmi.c index 4268bf2..0d69624 100644 --- a/drivers/gpu/drm/gma500/cdv_intel_hdmi.c +++ b/drivers/gpu/drm/gma500/cdv_intel_hdmi.c @@ -246,8 +246,7 @@ static void cdv_hdmi_destroy(struct drm_connector *connector) { struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
- if (gma_encoder->i2c_bus) - psb_intel_i2c_destroy(gma_encoder->i2c_bus); + psb_intel_i2c_destroy(gma_encoder->i2c_bus); drm_connector_unregister(connector); drm_connector_cleanup(connector); kfree(connector); diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c b/drivers/gpu/drm/gma500/cdv_intel_lvds.c index 0b77039..8f24013 100644 --- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c +++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c @@ -444,8 +444,7 @@ static void cdv_intel_lvds_destroy(struct drm_connector *connector) { struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
- if (gma_encoder->i2c_bus) - psb_intel_i2c_destroy(gma_encoder->i2c_bus); + psb_intel_i2c_destroy(gma_encoder->i2c_bus); drm_connector_unregister(connector); drm_connector_cleanup(connector); kfree(connector); @@ -780,12 +779,10 @@ out: failed_find: mutex_unlock(&dev->mode_config.mutex); printk(KERN_ERR "Failed find\n"); - if (gma_encoder->ddc_bus) - psb_intel_i2c_destroy(gma_encoder->ddc_bus); + psb_intel_i2c_destroy(gma_encoder->ddc_bus); failed_ddc: printk(KERN_ERR "Failed DDC\n"); - if (gma_encoder->i2c_bus) - psb_intel_i2c_destroy(gma_encoder->i2c_bus); + psb_intel_i2c_destroy(gma_encoder->i2c_bus); failed_blc_i2c: printk(KERN_ERR "Failed BLC\n"); drm_encoder_cleanup(encoder); diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds.c b/drivers/gpu/drm/gma500/oaktrail_lvds.c index 0d39da6..49c5c415 100644 --- a/drivers/gpu/drm/gma500/oaktrail_lvds.c +++ b/drivers/gpu/drm/gma500/oaktrail_lvds.c @@ -411,8 +411,7 @@ failed_find: mutex_unlock(&dev->mode_config.mutex);
dev_dbg(dev->dev, "No LVDS modes found, disabling.\n"); - if (gma_encoder->ddc_bus) - psb_intel_i2c_destroy(gma_encoder->ddc_bus); + psb_intel_i2c_destroy(gma_encoder->ddc_bus);
/* failed_ddc: */
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c index 6ec3a90..0efe165 100644 --- a/drivers/gpu/drm/gma500/psb_drv.c +++ b/drivers/gpu/drm/gma500/psb_drv.c @@ -210,8 +210,7 @@ static int psb_driver_unload(struct drm_device *dev) iounmap(dev_priv->aux_reg); dev_priv->aux_reg = NULL; } - if (dev_priv->aux_pdev) - pci_dev_put(dev_priv->aux_pdev); + pci_dev_put(dev_priv->aux_pdev);
/* Destroy VBT data */ psb_intel_destroy_bios(dev); diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c index 88aad95..e73c3f9 100644 --- a/drivers/gpu/drm/gma500/psb_intel_lvds.c +++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c @@ -561,8 +561,7 @@ void psb_intel_lvds_destroy(struct drm_connector *connector) struct gma_encoder *gma_encoder = gma_attached_encoder(connector); struct psb_intel_lvds_priv *lvds_priv = gma_encoder->dev_priv;
- if (lvds_priv->ddc_bus) - psb_intel_i2c_destroy(lvds_priv->ddc_bus); + psb_intel_i2c_destroy(lvds_priv->ddc_bus); drm_connector_unregister(connector); drm_connector_cleanup(connector); kfree(connector); @@ -834,11 +833,9 @@ out:
failed_find: mutex_unlock(&dev->mode_config.mutex); - if (lvds_priv->ddc_bus) - psb_intel_i2c_destroy(lvds_priv->ddc_bus); + psb_intel_i2c_destroy(lvds_priv->ddc_bus); failed_ddc: - if (lvds_priv->i2c_bus) - psb_intel_i2c_destroy(lvds_priv->i2c_bus); + psb_intel_i2c_destroy(lvds_priv->i2c_bus); failed_blc_i2c: drm_encoder_cleanup(encoder); drm_connector_cleanup(connector);
On Wed, 22 Oct 2014 18:48:21 +0200 SF Markus Elfring elfring@users.sourceforge.net wrote:
If you are convinced that dropping the null tests is a good idea, then you can submit the patch that makes the change to the relevant maintainers and mailing lists.
Would you like to integrate the following proposal into your source code repository?
What platforms have you tested the code on at this point ?
Alan
What platforms have you tested the code on at this point ?
None. - My "test computer" does not provide the corresponding hardware for the affected driver source files.
Regards, Markus
I still have the hardware in use and I am willing to test.
If necessary, please send me a URL to the snapshot of the kernel with the modified code. I can compile this (takes about 8 hours ;-) on the high speed atom n2600) and run a couple of random tests, such as starting the WM, browsing some webpages, changing resolution, and run a 2D benchmark.
Greetings, Arthur Borsboom On 26 Oct 2014 13:10, "SF Markus Elfring" elfring@users.sourceforge.net wrote:
What platforms have you tested the code on at this point ?
None. - My "test computer" does not provide the corresponding hardware for the affected driver source files.
Regards, Markus
From: Markus Elfring elfring@users.sourceforge.net Date: Fri, 22 Jul 2016 10:30:30 +0200
The functions pci_dev_put() and psb_intel_i2c_destroy() test whether their argument is NULL and then return immediately. Thus the tests around their calls are not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring elfring@users.sourceforge.net --- drivers/gpu/drm/gma500/cdv_intel_hdmi.c | 3 +-- drivers/gpu/drm/gma500/cdv_intel_lvds.c | 9 +++------ drivers/gpu/drm/gma500/psb_drv.c | 6 ++---- drivers/gpu/drm/gma500/psb_intel_lvds.c | 9 +++------ 4 files changed, 9 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/gma500/cdv_intel_hdmi.c b/drivers/gpu/drm/gma500/cdv_intel_hdmi.c index 28f9d90..563f193 100644 --- a/drivers/gpu/drm/gma500/cdv_intel_hdmi.c +++ b/drivers/gpu/drm/gma500/cdv_intel_hdmi.c @@ -246,8 +246,7 @@ static void cdv_hdmi_destroy(struct drm_connector *connector) { struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
- if (gma_encoder->i2c_bus) - psb_intel_i2c_destroy(gma_encoder->i2c_bus); + psb_intel_i2c_destroy(gma_encoder->i2c_bus); drm_connector_unregister(connector); drm_connector_cleanup(connector); kfree(connector); diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c b/drivers/gpu/drm/gma500/cdv_intel_lvds.c index 813ef23..38dc890 100644 --- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c +++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c @@ -444,8 +444,7 @@ static void cdv_intel_lvds_destroy(struct drm_connector *connector) { struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
- if (gma_encoder->i2c_bus) - psb_intel_i2c_destroy(gma_encoder->i2c_bus); + psb_intel_i2c_destroy(gma_encoder->i2c_bus); drm_connector_unregister(connector); drm_connector_cleanup(connector); kfree(connector); @@ -780,12 +779,10 @@ out: failed_find: mutex_unlock(&dev->mode_config.mutex); printk(KERN_ERR "Failed find\n"); - if (gma_encoder->ddc_bus) - psb_intel_i2c_destroy(gma_encoder->ddc_bus); + psb_intel_i2c_destroy(gma_encoder->ddc_bus); failed_ddc: printk(KERN_ERR "Failed DDC\n"); - if (gma_encoder->i2c_bus) - psb_intel_i2c_destroy(gma_encoder->i2c_bus); + psb_intel_i2c_destroy(gma_encoder->i2c_bus); failed_blc_i2c: printk(KERN_ERR "Failed BLC\n"); drm_encoder_cleanup(encoder); diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c index 82b8ce4..50eb944f 100644 --- a/drivers/gpu/drm/gma500/psb_drv.c +++ b/drivers/gpu/drm/gma500/psb_drv.c @@ -210,10 +210,8 @@ static int psb_driver_unload(struct drm_device *dev) iounmap(dev_priv->aux_reg); dev_priv->aux_reg = NULL; } - if (dev_priv->aux_pdev) - pci_dev_put(dev_priv->aux_pdev); - if (dev_priv->lpc_pdev) - pci_dev_put(dev_priv->lpc_pdev); + pci_dev_put(dev_priv->aux_pdev); + pci_dev_put(dev_priv->lpc_pdev);
/* Destroy VBT data */ psb_intel_destroy_bios(dev); diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c index b1b9331..e55733c 100644 --- a/drivers/gpu/drm/gma500/psb_intel_lvds.c +++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c @@ -561,8 +561,7 @@ void psb_intel_lvds_destroy(struct drm_connector *connector) struct gma_encoder *gma_encoder = gma_attached_encoder(connector); struct psb_intel_lvds_priv *lvds_priv = gma_encoder->dev_priv;
- if (lvds_priv->ddc_bus) - psb_intel_i2c_destroy(lvds_priv->ddc_bus); + psb_intel_i2c_destroy(lvds_priv->ddc_bus); drm_connector_unregister(connector); drm_connector_cleanup(connector); kfree(connector); @@ -835,11 +834,9 @@ out:
failed_find: mutex_unlock(&dev->mode_config.mutex); - if (lvds_priv->ddc_bus) - psb_intel_i2c_destroy(lvds_priv->ddc_bus); + psb_intel_i2c_destroy(lvds_priv->ddc_bus); failed_ddc: - if (lvds_priv->i2c_bus) - psb_intel_i2c_destroy(lvds_priv->i2c_bus); + psb_intel_i2c_destroy(lvds_priv->i2c_bus); failed_blc_i2c: drm_encoder_cleanup(encoder); drm_connector_cleanup(connector);
On Fri, Jul 22, 2016 at 10:45:30AM +0200, SF Markus Elfring wrote:
From: Markus Elfring elfring@users.sourceforge.net Date: Fri, 22 Jul 2016 10:30:30 +0200
The functions pci_dev_put() and psb_intel_i2c_destroy() test whether their argument is NULL and then return immediately. Thus the tests around their calls are not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring elfring@users.sourceforge.net
Two more applied. And your threading still looks funny, all the patches are in-reply to one I don't even have. -Daniel
drivers/gpu/drm/gma500/cdv_intel_hdmi.c | 3 +-- drivers/gpu/drm/gma500/cdv_intel_lvds.c | 9 +++------ drivers/gpu/drm/gma500/psb_drv.c | 6 ++---- drivers/gpu/drm/gma500/psb_intel_lvds.c | 9 +++------ 4 files changed, 9 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/gma500/cdv_intel_hdmi.c b/drivers/gpu/drm/gma500/cdv_intel_hdmi.c index 28f9d90..563f193 100644 --- a/drivers/gpu/drm/gma500/cdv_intel_hdmi.c +++ b/drivers/gpu/drm/gma500/cdv_intel_hdmi.c @@ -246,8 +246,7 @@ static void cdv_hdmi_destroy(struct drm_connector *connector) { struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
- if (gma_encoder->i2c_bus)
psb_intel_i2c_destroy(gma_encoder->i2c_bus);
- psb_intel_i2c_destroy(gma_encoder->i2c_bus); drm_connector_unregister(connector); drm_connector_cleanup(connector); kfree(connector);
diff --git a/drivers/gpu/drm/gma500/cdv_intel_lvds.c b/drivers/gpu/drm/gma500/cdv_intel_lvds.c index 813ef23..38dc890 100644 --- a/drivers/gpu/drm/gma500/cdv_intel_lvds.c +++ b/drivers/gpu/drm/gma500/cdv_intel_lvds.c @@ -444,8 +444,7 @@ static void cdv_intel_lvds_destroy(struct drm_connector *connector) { struct gma_encoder *gma_encoder = gma_attached_encoder(connector);
- if (gma_encoder->i2c_bus)
psb_intel_i2c_destroy(gma_encoder->i2c_bus);
- psb_intel_i2c_destroy(gma_encoder->i2c_bus); drm_connector_unregister(connector); drm_connector_cleanup(connector); kfree(connector);
@@ -780,12 +779,10 @@ out: failed_find: mutex_unlock(&dev->mode_config.mutex); printk(KERN_ERR "Failed find\n");
- if (gma_encoder->ddc_bus)
psb_intel_i2c_destroy(gma_encoder->ddc_bus);
- psb_intel_i2c_destroy(gma_encoder->ddc_bus);
failed_ddc: printk(KERN_ERR "Failed DDC\n");
- if (gma_encoder->i2c_bus)
psb_intel_i2c_destroy(gma_encoder->i2c_bus);
- psb_intel_i2c_destroy(gma_encoder->i2c_bus);
failed_blc_i2c: printk(KERN_ERR "Failed BLC\n"); drm_encoder_cleanup(encoder); diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c index 82b8ce4..50eb944f 100644 --- a/drivers/gpu/drm/gma500/psb_drv.c +++ b/drivers/gpu/drm/gma500/psb_drv.c @@ -210,10 +210,8 @@ static int psb_driver_unload(struct drm_device *dev) iounmap(dev_priv->aux_reg); dev_priv->aux_reg = NULL; }
if (dev_priv->aux_pdev)
pci_dev_put(dev_priv->aux_pdev);
if (dev_priv->lpc_pdev)
pci_dev_put(dev_priv->lpc_pdev);
pci_dev_put(dev_priv->aux_pdev);
pci_dev_put(dev_priv->lpc_pdev);
/* Destroy VBT data */ psb_intel_destroy_bios(dev);
diff --git a/drivers/gpu/drm/gma500/psb_intel_lvds.c b/drivers/gpu/drm/gma500/psb_intel_lvds.c index b1b9331..e55733c 100644 --- a/drivers/gpu/drm/gma500/psb_intel_lvds.c +++ b/drivers/gpu/drm/gma500/psb_intel_lvds.c @@ -561,8 +561,7 @@ void psb_intel_lvds_destroy(struct drm_connector *connector) struct gma_encoder *gma_encoder = gma_attached_encoder(connector); struct psb_intel_lvds_priv *lvds_priv = gma_encoder->dev_priv;
- if (lvds_priv->ddc_bus)
psb_intel_i2c_destroy(lvds_priv->ddc_bus);
- psb_intel_i2c_destroy(lvds_priv->ddc_bus); drm_connector_unregister(connector); drm_connector_cleanup(connector); kfree(connector);
@@ -835,11 +834,9 @@ out:
failed_find: mutex_unlock(&dev->mode_config.mutex);
- if (lvds_priv->ddc_bus)
psb_intel_i2c_destroy(lvds_priv->ddc_bus);
- psb_intel_i2c_destroy(lvds_priv->ddc_bus);
failed_ddc:
- if (lvds_priv->i2c_bus)
psb_intel_i2c_destroy(lvds_priv->i2c_bus);
- psb_intel_i2c_destroy(lvds_priv->i2c_bus);
failed_blc_i2c: drm_encoder_cleanup(encoder); drm_connector_cleanup(connector); -- 2.9.2
dri-devel@lists.freedesktop.org