Re: [PATCH with SmPL?] staging: rtl8188eu: Adjustments around jump labels
@@ -212,8 +212,7 @@ efuse_phymap_to_logical(u8 *phymap, u16 _offset, u16
_size_byte, u8 *pbuf)
exit:
kfree(efuseTbl);
- if (eFuseWord)
- kfree(eFuseWord);
+ kfree(eFuseWord);
>>>
>>> I think that this code has been updated already. It would be better to
>>> add labels so that kfree is only executed when needed.
>>
>> Are there any chances to achieve the suggested fine-tuning for jump labels
>> also with another semantic patch approach?
>
> No, I don't think so. The pattern is not regular enough.
Now I have got a different impression for corresponding improvement
possibilities.
elfring@Sonne:~/Projekte/Linux/stable-patched> spatch.opt -debug -sp-file
~/Projekte/Coccinelle/janitor/move_function_call_before_jump_label1.cocci
drivers/staging/rtl8188eu/core/rtw_efuse.c
init_defs_builtins: /usr/local/share/coccinelle/standard.h
---
processing semantic patch file:
/home/elfring/Projekte/Coccinelle/janitor/move_function_call_before_jump_label1.cocci
with isos from: /usr/local/share/coccinelle/standard.iso
---
@move_function_call_before_jump_label@
expression x;
identifier fu, label;
type t;
@@
t fu(...)
{
... when any
x = kzalloc(...);
if (x == NULL) {
...
goto label;
}
... when any
+kfree(x);
label:
-kfree(x);
...
}
HANDLING: drivers/staging/rtl8188eu/core/rtw_efuse.c
---
let's go
---
---
---
move_function_call_before_jump_label =
---
dependencies for rule move_function_call_before_jump_label satisfied:
binding in = []
binding relevant in = []
(ONCE) USING optional_storage builtin isomorphism
transformation info returned:
transform state: 5
with rule_elem:
<<<
kfree(move_function_call_before_jump_label:x);
move_function_call_before_jump_label:label:
with binding: [move_function_call_before_jump_label.x -->
efuseTbl]
transform state: 204
with rule_elem:
-kfree-(-move_function_call_before_jump_label:x-)-;
with binding: [move_function_call_before_jump_label.x -->
efuseTbl]
binding out = []
transform one node: 204
transform one node: 5
---
Finished
---
diff =
--- drivers/staging/rtl8188eu/core/rtw_efuse.c
+++ /tmp/cocci-output-4498-349827-rtw_efuse.c
@@ -209,8 +209,8 @@ efuse_phymap_to_logical(u8 *phymap, u16
/* 5. Calculate Efuse utilization. */
/* */
+kfree(efuseTbl);
exit:
- kfree(efuseTbl);
kfree(eFuseWord);
}
Check duplication for 1 files
Can my update suggestion be generalised a bit more for the movement of specific
jump labels
towards the end of a function implementation like in the use case
"efuse_phymap_to_logical()"?
Regards,
Markus
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 0/2] staging: rtl8188eu: Deletion of a few unnecessary checks
From: Markus Elfring Date: Wed, 12 Nov 2014 20:42:18 +0100 Another update suggestion was taken into account after patches were applied from static source code analysis. Markus Elfring (2): staging: rtl8188eu: Deletion of unnecessary checks before three function calls staging: rtl8188eu: Better memory clean-up in efuse_phymap_to_logical() drivers/staging/rtl8188eu/core/rtw_efuse.c | 13 - drivers/staging/rtl8188eu/core/rtw_mlme.c| 3 +-- drivers/staging/rtl8188eu/core/rtw_sta_mgt.c | 3 +-- drivers/staging/rtl8188eu/core/rtw_xmit.c| 6 ++ drivers/staging/rtl8188eu/os_dep/usb_intf.c | 6 +++--- 5 files changed, 15 insertions(+), 16 deletions(-) -- 2.1.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 1/2] staging: rtl8188eu: Deletion of unnecessary checks before three function calls
From: Markus Elfring
Date: Wed, 12 Nov 2014 20:25:49 +0100
The functions kfree(), rtw_free_netdev() and vfree() test whether their
argument is NULL and then return immediately. Thus the test around the call
is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/staging/rtl8188eu/core/rtw_efuse.c | 3 +--
drivers/staging/rtl8188eu/core/rtw_mlme.c| 3 +--
drivers/staging/rtl8188eu/core/rtw_sta_mgt.c | 3 +--
drivers/staging/rtl8188eu/core/rtw_xmit.c| 6 ++
drivers/staging/rtl8188eu/os_dep/usb_intf.c | 6 +++---
5 files changed, 8 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_efuse.c
b/drivers/staging/rtl8188eu/core/rtw_efuse.c
index 5b997b2..697876b 100644
--- a/drivers/staging/rtl8188eu/core/rtw_efuse.c
+++ b/drivers/staging/rtl8188eu/core/rtw_efuse.c
@@ -212,8 +212,7 @@ efuse_phymap_to_logical(u8 *phymap, u16 _offset, u16
_size_byte, u8 *pbuf)
exit:
kfree(efuseTbl);
- if (eFuseWord)
- kfree(eFuseWord);
+ kfree(eFuseWord);
}
static void efuse_read_phymap_from_txpktbuf(
diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme.c
b/drivers/staging/rtl8188eu/core/rtw_mlme.c
index 149c271..df54350 100644
--- a/drivers/staging/rtl8188eu/core/rtw_mlme.c
+++ b/drivers/staging/rtl8188eu/core/rtw_mlme.c
@@ -122,8 +122,7 @@ void rtw_free_mlme_priv(struct mlme_priv *pmlmepriv)
rtw_free_mlme_priv_ie_data(pmlmepriv);
if (pmlmepriv) {
- if (pmlmepriv->free_bss_buf)
- vfree(pmlmepriv->free_bss_buf);
+ vfree(pmlmepriv->free_bss_buf);
}
}
diff --git a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
index e1dc8fa..af1de9c 100644
--- a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
+++ b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
@@ -201,8 +201,7 @@ u32 _rtw_free_sta_priv(struct sta_priv *pstapriv)
rtw_mfree_sta_priv_lock(pstapriv);
- if (pstapriv->pallocated_stainfo_buf)
- vfree(pstapriv->pallocated_stainfo_buf);
+ vfree(pstapriv->pallocated_stainfo_buf);
}
return _SUCCESS;
diff --git a/drivers/staging/rtl8188eu/core/rtw_xmit.c
b/drivers/staging/rtl8188eu/core/rtw_xmit.c
index 639ace0..011c9cf 100644
--- a/drivers/staging/rtl8188eu/core/rtw_xmit.c
+++ b/drivers/staging/rtl8188eu/core/rtw_xmit.c
@@ -246,11 +246,9 @@ void _rtw_free_xmit_priv (struct xmit_priv *pxmitpriv)
pxmitbuf++;
}
- if (pxmitpriv->pallocated_frame_buf)
- vfree(pxmitpriv->pallocated_frame_buf);
+ vfree(pxmitpriv->pallocated_frame_buf);
- if (pxmitpriv->pallocated_xmitbuf)
- vfree(pxmitpriv->pallocated_xmitbuf);
+ vfree(pxmitpriv->pallocated_xmitbuf);
/* free xmit extension buff */
pxmitbuf = (struct xmit_buf *)pxmitpriv->pxmit_extbuf;
diff --git a/drivers/staging/rtl8188eu/os_dep/usb_intf.c
b/drivers/staging/rtl8188eu/os_dep/usb_intf.c
index 407a318..4e2c34b 100644
--- a/drivers/staging/rtl8188eu/os_dep/usb_intf.c
+++ b/drivers/staging/rtl8188eu/os_dep/usb_intf.c
@@ -456,8 +456,9 @@ free_adapter:
if (status != _SUCCESS) {
if (pnetdev)
rtw_free_netdev(pnetdev);
- else if (padapter)
+ else
vfree(padapter);
+
padapter = NULL;
}
exit:
@@ -487,8 +488,7 @@ static void rtw_usb_if1_deinit(struct adapter *if1)
DBG_88E("+r871xu_dev_remove, hw_init_completed=%d\n",
if1->hw_init_completed);
rtw_free_drv_sw(if1);
- if (pnetdev)
- rtw_free_netdev(pnetdev);
+ rtw_free_netdev(pnetdev);
}
static int rtw_drv_init(struct usb_interface *pusb_intf, const struct
usb_device_id *pdid)
--
2.1.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 2/2] staging: rtl8188eu: Better memory clean-up in efuse_phymap_to_logical()
From: Markus Elfring
Date: Wed, 12 Nov 2014 20:40:12 +0100
Memory releases were handled in an inefficient way by the implementation of
the efuse_phymap_to_logical() function in case of an allocation failure.
The corresponding clean-up was improved by reordering of kfree() calls
and a few adjustments for jump labels.
Reported-by: Julia Lawall
Signed-off-by: Markus Elfring
---
drivers/staging/rtl8188eu/core/rtw_efuse.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_efuse.c
b/drivers/staging/rtl8188eu/core/rtw_efuse.c
index 697876b..359f169 100644
--- a/drivers/staging/rtl8188eu/core/rtw_efuse.c
+++ b/drivers/staging/rtl8188eu/core/rtw_efuse.c
@@ -112,7 +112,7 @@ efuse_phymap_to_logical(u8 *phymap, u16 _offset, u16
_size_byte, u8 *pbuf)
eFuseWord = (u16 **)rtw_malloc2d(EFUSE_MAX_SECTION_88E,
EFUSE_MAX_WORD_UNIT, sizeof(u16));
if (eFuseWord == NULL) {
DBG_88E("%s: alloc eFuseWord fail!\n", __func__);
- goto exit;
+ goto cleanup1;
}
/* 0. Refresh efuse init map as all oxFF. */
@@ -130,7 +130,7 @@ efuse_phymap_to_logical(u8 *phymap, u16 _offset, u16
_size_byte, u8 *pbuf)
eFuse_Addr++;
} else {
DBG_88E("EFUSE is empty efuse_Addr-%d efuse_data =%x\n",
eFuse_Addr, rtemp8);
- goto exit;
+ goto cleanup2;
}
/* */
@@ -209,10 +209,14 @@ efuse_phymap_to_logical(u8 *phymap, u16 _offset, u16
_size_byte, u8 *pbuf)
/* 5. Calculate Efuse utilization. */
/* */
-exit:
+cleanup2:
+ kfree(eFuseWord);
+
+cleanup1:
kfree(efuseTbl);
- kfree(eFuseWord);
+exit:
+ ;
}
static void efuse_read_phymap_from_txpktbuf(
--
2.1.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/2] staging: rtl8188eu: Deletion of unnecessary checks before three function calls
>> @@ -487,8 +488,7 @@ static void rtw_usb_if1_deinit(struct adapter *if1)
>> DBG_88E("+r871xu_dev_remove, hw_init_completed=%d\n",
>> if1->hw_init_completed);
>> rtw_free_drv_sw(if1);
>> -if (pnetdev)
>> -rtw_free_netdev(pnetdev);
>> +rtw_free_netdev(pnetdev);
>
> I still feel that hiding the if statement inside the function call makes
> the code more subtle and it is a bad harmful thing to do.
I find your feedback interesting.
> This is especially true if you have trained yourself to know that
> free_netdev() can't accept NULL pointers.
Do you need to adjust your concerns a bit over time when function variants
provide a corresponding safety check in their implementations?
Regards,
Markus
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 2/2] staging: rtl8188eu: Better memory clean-up in efuse_phymap_to_logical()
>> +goto cleanup1; > > 1) Don't use GW-BASIC label names. Label names should reflect what the > label does such as free_fuse_word or free_fuse_tabel. > > 2) Don't use do-nothing labels. Just return directly. Does the document "CodingStyle" need any extensions for special cases? Are there any update candidates in the chapter "7: Centralized exiting of functions"? Regards, Markus ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 2/2] staging: rtl8188eu: Better memory clean-up in efuse_phymap_to_logical()
+ goto cleanup1; >>> >>> 1) Don't use GW-BASIC label names. Label names should reflect what the >>> label does such as free_fuse_word or free_fuse_tabel. >>> >>> 2) Don't use do-nothing labels. Just return directly. >> >> Does the document "CodingStyle" need any extensions for special cases? > > I don't understand. Should the naming convention become more explicit for jump labels? > CodingStyle says: > > "If there is no cleanup needed then just return directly." Do you want that I send another update suggestion with other corrections for jump labels in the affected function implementation? Regards, Markus ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 2/2] staging: rtl8188eu: Better memory clean-up in efuse_phymap_to_logical()
> You are not using the most recent version of the code. The issue has > already been fixed. Thanks for your reminder. https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/drivers/staging/rtl8188eu/core/rtw_efuse.c?id=3cfab18ce55282a85e2c7e5db15c5daf065efdb4 Regards, Markus ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/1] net: Hyper-V: Deletion of an unnecessary check before the function call "vfree"
From: Markus Elfring Date: Thu, 20 Nov 2014 15:15:21 +0100 The vfree() function performs also input parameter validation. Thus the test around the call is not needed. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- drivers/net/hyperv/netvsc.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c index da2d346..ffe7481 100644 --- a/drivers/net/hyperv/netvsc.c +++ b/drivers/net/hyperv/netvsc.c @@ -548,8 +548,7 @@ int netvsc_device_remove(struct hv_device *device) vmbus_close(device->channel); /* Release all resources */ - if (net_device->sub_cb_buf) - vfree(net_device->sub_cb_buf); + vfree(net_device->sub_cb_buf); kfree(net_device); return 0; -- 2.1.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: net: Hyper-V: Deletion of an unnecessary check before the function call "vfree"
> This does not apply to the net-next tree, please respin. Thanks for your reply. How do you think about to try out the scripts which I published in March to get more constructive feedback? Will they run faster for another analysis on current Linux source files with your test systems (than my computer)? Regards, Markus ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: net: Hyper-V: Deletion of an unnecessary check before the function call "vfree"
> This has nothing to do with me asking you to frame your patches > against the correct tree. I imagine than someone other can also pick up this update suggestion (a simple change of two lines) quicker before I might try another software build again from a different commit as a base. Regards, Markus ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: net: Hyper-V: Deletion of an unnecessary check before the function call "vfree"
>> I imagine than someone other can also pick up this update suggestion >> (a simple change of two lines) quicker before I might try another >> software build again from a different commit as a base. > > I have no idea why someone would do that. I imagine that other software users (besides me) like developers and testers might also become curious to try the proposed changes out. How much will they eventually help to run Linux components a bit faster? > If you don't bother resubmit, nobody will. I hope that there are more possibilities for anticipation and acceptance of source code improvement potentials. Would you also like to contribute a bit more fine-tuning for the affected software versions? Regards, Markus ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: net: Hyper-V: Deletion of an unnecessary check before the function call "vfree"
> Whereas if you learn how to base your changes cleanly on the correct > base now, all of your future submissions will go quickly and smoothly > into my tree. My reluctance to work with more Linux repositories will evolve over time. The faster affected software versions can be rebuilt the more it will become interesting to try even more source code improvements out, won't it? I find it nice that you could accept update suggestions for a few other Linux components already. Regards, Markus ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/2] staging: android: ion: Deletion of a few unnecessary checks
From: Markus Elfring Date: Sun, 23 Nov 2014 19:13:56 +0100 Another update suggestion was taken into account after a patch was applied from static source code analysis. Markus Elfring (2): Deletion of unnecessary checks before two function calls One function call less in ion_buffer_create() after error detection drivers/staging/android/ion/ion.c | 9 +++-- drivers/staging/android/ion/ion_dummy_driver.c | 6 ++ drivers/staging/android/ion/tegra/tegra_ion.c | 6 ++ 3 files changed, 7 insertions(+), 14 deletions(-) -- 2.1.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/2] staging: android: ion: Deletion of unnecessary checks before two function calls
From: Markus Elfring
Date: Sun, 23 Nov 2014 18:48:15 +0100
The functions ion_heap_destroy() and vfree() perform also input
parameter validation. Thus the test around the call is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/staging/android/ion/ion.c | 6 ++
drivers/staging/android/ion/ion_dummy_driver.c | 6 ++
drivers/staging/android/ion/tegra/tegra_ion.c | 6 ++
3 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 2703609..df12cc3 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -263,8 +263,7 @@ err:
heap->ops->unmap_dma(heap, buffer);
heap->ops->free(buffer);
err1:
- if (buffer->pages)
- vfree(buffer->pages);
+ vfree(buffer->pages);
err2:
kfree(buffer);
return ERR_PTR(ret);
@@ -276,8 +275,7 @@ void ion_buffer_destroy(struct ion_buffer *buffer)
buffer->heap->ops->unmap_kernel(buffer->heap, buffer);
buffer->heap->ops->unmap_dma(buffer->heap, buffer);
buffer->heap->ops->free(buffer);
- if (buffer->pages)
- vfree(buffer->pages);
+ vfree(buffer->pages);
kfree(buffer);
}
diff --git a/drivers/staging/android/ion/ion_dummy_driver.c
b/drivers/staging/android/ion/ion_dummy_driver.c
index 3a45e79..d06bf52 100644
--- a/drivers/staging/android/ion/ion_dummy_driver.c
+++ b/drivers/staging/android/ion/ion_dummy_driver.c
@@ -112,10 +112,8 @@ static int __init ion_dummy_init(void)
}
return 0;
err:
- for (i = 0; i < dummy_ion_pdata.nr; i++) {
- if (heaps[i])
- ion_heap_destroy(heaps[i]);
- }
+ for (i = 0; i < dummy_ion_pdata.nr; ++i)
+ ion_heap_destroy(heaps[i]);
kfree(heaps);
if (carveout_ptr) {
diff --git a/drivers/staging/android/ion/tegra/tegra_ion.c
b/drivers/staging/android/ion/tegra/tegra_ion.c
index 11c7cce..5b8ef0e 100644
--- a/drivers/staging/android/ion/tegra/tegra_ion.c
+++ b/drivers/staging/android/ion/tegra/tegra_ion.c
@@ -54,10 +54,8 @@ static int tegra_ion_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, idev);
return 0;
err:
- for (i = 0; i < num_heaps; i++) {
- if (heaps[i])
- ion_heap_destroy(heaps[i]);
- }
+ for (i = 0; i < num_heaps; ++i)
+ ion_heap_destroy(heaps[i]);
return err;
}
--
2.1.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/2] staging: android: ion: One function call less in ion_buffer_create() after error detection
From: Markus Elfring
Date: Sun, 23 Nov 2014 19:12:29 +0100
The jump label "err1" was used by the ion_buffer_create() function in case of
a memory allocation failure just to pass a null pointer to a vfree() function
call by a data structure element.
This implementation detail could be improved by the deletion of this
inappropriate jump label.
Signed-off-by: Markus Elfring
---
drivers/staging/android/ion/ion.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index df12cc3..7a26b85 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -226,7 +226,7 @@ static struct ion_buffer *ion_buffer_create(struct ion_heap
*heap,
buffer->pages = vmalloc(sizeof(struct page *) * num_pages);
if (!buffer->pages) {
ret = -ENOMEM;
- goto err1;
+ goto err2;
}
for_each_sg(table->sgl, sg, table->nents, i) {
@@ -262,7 +262,6 @@ static struct ion_buffer *ion_buffer_create(struct ion_heap
*heap,
err:
heap->ops->unmap_dma(heap, buffer);
heap->ops->free(buffer);
-err1:
vfree(buffer->pages);
err2:
kfree(buffer);
--
2.1.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/1] staging: olpc_dcon: Deletion of a check before backlight_device_unregister()
From: Markus Elfring Date: Mon, 24 Nov 2014 22:58:48 +0100 The backlight_device_unregister() function tests whether its argument is NULL and then returns immediately. Thus the test around the call is not needed. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- drivers/staging/olpc_dcon/olpc_dcon.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/staging/olpc_dcon/olpc_dcon.c b/drivers/staging/olpc_dcon/olpc_dcon.c index eb83b28..6a9a881 100644 --- a/drivers/staging/olpc_dcon/olpc_dcon.c +++ b/drivers/staging/olpc_dcon/olpc_dcon.c @@ -682,8 +682,7 @@ static int dcon_remove(struct i2c_client *client) free_irq(DCON_IRQ, dcon); - if (dcon->bl_dev) - backlight_device_unregister(dcon->bl_dev); + backlight_device_unregister(dcon->bl_dev); if (dcon_device != NULL) platform_device_unregister(dcon_device); -- 2.1.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/1] IMX-DRM-core: Deletion of a check before drm_fbdev_cma_restore_mode()
From: Markus Elfring Date: Tue, 25 Nov 2014 16:06:19 +0100 The drm_fbdev_cma_restore_mode() function tests whether its argument is NULL and then returns immediately. Thus the test around the call is not needed. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- drivers/staging/imx-drm/imx-drm-core.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/staging/imx-drm/imx-drm-core.c b/drivers/staging/imx-drm/imx-drm-core.c index 6b22106..0a1ecef 100644 --- a/drivers/staging/imx-drm/imx-drm-core.c +++ b/drivers/staging/imx-drm/imx-drm-core.c @@ -64,8 +64,7 @@ static void imx_drm_driver_lastclose(struct drm_device *drm) #if IS_ENABLED(CONFIG_DRM_IMX_FB_HELPER) struct imx_drm_device *imxdrm = drm->dev_private; - if (imxdrm->fbhelper) - drm_fbdev_cma_restore_mode(imxdrm->fbhelper); + drm_fbdev_cma_restore_mode(imxdrm->fbhelper); #endif } -- 2.1.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/1] staging: ozwpan: Deletion of unnecessary checks before the function call "oz_free_urb_link"
From: Markus Elfring Date: Tue, 25 Nov 2014 16:51:08 +0100 The oz_free_urb_link() function tests whether its argument is NULL and then returns immediately. Thus the test around the call is not needed. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- drivers/staging/ozwpan/ozhcd.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/staging/ozwpan/ozhcd.c b/drivers/staging/ozwpan/ozhcd.c index 30bd928..c9b6d98 100644 --- a/drivers/staging/ozwpan/ozhcd.c +++ b/drivers/staging/ozwpan/ozhcd.c @@ -411,8 +411,7 @@ static void oz_complete_urb(struct usb_hcd *hcd, struct urb *urb, } spin_lock(&g_tasklet_lock); spin_unlock_irqrestore(&g_tasklet_lock, irq_state); - if (cancel_urbl) - oz_free_urb_link(cancel_urbl); + oz_free_urb_link(cancel_urbl); } /* @@ -581,8 +580,7 @@ static int oz_dequeue_ep_urb(struct oz_port *port, u8 ep_addr, int in_dir, } } spin_unlock_bh(&port->ozhcd->hcd_lock); - if (urbl) - oz_free_urb_link(urbl); + oz_free_urb_link(urbl); return urbl ? 0 : -EIDRM; } -- 2.1.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] net: Hyper-V: Deletion of an unnecessary check before the function call "vfree"
From: Markus Elfring Date: Tue, 25 Nov 2014 22:33:45 +0100 The vfree() function performs also input parameter validation. Thus the test around the call is not needed. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring Signed-off-by: Haiyang Zhang --- drivers/net/hyperv/netvsc.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c index 6b46311..6fc834e 100644 --- a/drivers/net/hyperv/netvsc.c +++ b/drivers/net/hyperv/netvsc.c @@ -561,9 +561,7 @@ int netvsc_device_remove(struct hv_device *device) vmbus_close(device->channel); /* Release all resources */ - if (net_device->sub_cb_buf) - vfree(net_device->sub_cb_buf); - + vfree(net_device->sub_cb_buf); free_netvsc_device(net_device); return 0; } -- 2.1.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] staging: android: ion: One function call less in ion_buffer_create() after error detection
>> diff --git a/drivers/staging/android/ion/ion.c
>> b/drivers/staging/android/ion/ion.c
>> index df12cc3..7a26b85 100644
>> --- a/drivers/staging/android/ion/ion.c
>> +++ b/drivers/staging/android/ion/ion.c
>> @@ -226,7 +226,7 @@ static struct ion_buffer *ion_buffer_create(struct
>> ion_heap *heap,
>> buffer->pages = vmalloc(sizeof(struct page *) * num_pages);
>> if (!buffer->pages) {
>> ret = -ENOMEM;
>> -goto err1;
>> +goto err2;
>> }
>>
>> for_each_sg(table->sgl, sg, table->nents, i) {
>> @@ -262,7 +262,6 @@ static struct ion_buffer *ion_buffer_create(struct
>> ion_heap *heap,
>> err:
>> heap->ops->unmap_dma(heap, buffer);
>> heap->ops->free(buffer);
>> -err1:
>> vfree(buffer->pages);
>> err2:
>
> Now we have "err" and "err2", which doesn't make much sense,
> please fix up.
How do you want this source code to be improved?
Should I introduce longer names for the affected jump labels?
Regards,
Markus
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/1] [media] lirc_zilog: Deletion of unnecessary checks before the function call "vfree"
From: Markus Elfring
Date: Mon, 1 Dec 2014 19:49:39 +0100
The vfree() function performs also input parameter validation. Thus the test
around the call is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/staging/media/lirc/lirc_zilog.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/media/lirc/lirc_zilog.c
b/drivers/staging/media/lirc/lirc_zilog.c
index 3259aac..50b255a 100644
--- a/drivers/staging/media/lirc/lirc_zilog.c
+++ b/drivers/staging/media/lirc/lirc_zilog.c
@@ -729,11 +729,9 @@ static int send_boot_data(struct IR_tx *tx)
static void fw_unload_locked(void)
{
if (tx_data) {
- if (tx_data->code_sets)
- vfree(tx_data->code_sets);
+ vfree(tx_data->code_sets);
- if (tx_data->datap)
- vfree(tx_data->datap);
+ vfree(tx_data->datap);
vfree(tx_data);
tx_data = NULL;
--
2.1.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/2] [media] mn88473: Delete an unnecessary check
From: Markus Elfring Date: Mon, 1 Dec 2014 23:16:34 +0100 Another update suggestion was taken into account after a patch was applied from static source code analysis. Markus Elfring (2): Deletion of an unnecessary check before the function call "release_firmware" One function call less in mn88473_init() after error detection drivers/staging/media/mn88473/mn88473.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) -- 2.1.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/2] [media] mn88473: Deletion of an unnecessary check before the function call "release_firmware"
From: Markus Elfring Date: Mon, 1 Dec 2014 22:55:29 +0100 The release_firmware() function tests whether its argument is NULL and then returns immediately. Thus the test around the call is not needed. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- drivers/staging/media/mn88473/mn88473.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/staging/media/mn88473/mn88473.c b/drivers/staging/media/mn88473/mn88473.c index 1659335..52180bb 100644 --- a/drivers/staging/media/mn88473/mn88473.c +++ b/drivers/staging/media/mn88473/mn88473.c @@ -262,8 +262,7 @@ static int mn88473_init(struct dvb_frontend *fe) return 0; err: - if (fw) - release_firmware(fw); + release_firmware(fw); dev_dbg(&client->dev, "failed=%d\n", ret); return ret; -- 2.1.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/2] [media] mn88473: One function call less in mn88473_init() after error detection
From: Markus Elfring
Date: Mon, 1 Dec 2014 23:15:20 +0100
The release_firmware() function was called by the mn88473_init() function even
if a previous function call "request_firmware" failed.
This implementation detail could be improved by the introduction of another
jump label.
Signed-off-by: Markus Elfring
---
drivers/staging/media/mn88473/mn88473.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/media/mn88473/mn88473.c
b/drivers/staging/media/mn88473/mn88473.c
index 52180bb..a333744 100644
--- a/drivers/staging/media/mn88473/mn88473.c
+++ b/drivers/staging/media/mn88473/mn88473.c
@@ -225,7 +225,7 @@ static int mn88473_init(struct dvb_frontend *fe)
ret = request_firmware(&fw, fw_file, &client->dev);
if (ret) {
dev_err(&client->dev, "firmare file '%s' not found\n", fw_file);
- goto err;
+ goto err_request_firmware;
}
dev_info(&client->dev, "downloading firmware from file '%s'\n",
@@ -261,9 +261,10 @@ static int mn88473_init(struct dvb_frontend *fe)
dev->warm = true;
return 0;
+
err:
release_firmware(fw);
-
+err_request_firmware:
dev_dbg(&client->dev, "failed=%d\n", ret);
return ret;
}
--
2.1.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/1] lustre: Deletion of unnecessary checks before three function calls
From: Markus Elfring Date: Tue, 2 Dec 2014 11:40:33 +0100 The functions free_ll_remote_perm(), free_rmtperm_hash() and iput() test whether their argument is NULL and then return immediately. Thus the test around their calls is not needed. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- drivers/staging/lustre/lustre/llite/remote_perm.c | 5 ++--- drivers/staging/lustre/lustre/llite/statahead.c | 3 +-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/staging/lustre/lustre/llite/remote_perm.c b/drivers/staging/lustre/lustre/llite/remote_perm.c index c05a912..a581826 100644 --- a/drivers/staging/lustre/lustre/llite/remote_perm.c +++ b/drivers/staging/lustre/lustre/llite/remote_perm.c @@ -194,7 +194,7 @@ int ll_update_remote_perm(struct inode *inode, struct mdt_remote_perm *perm) if (!lli->lli_remote_perms) lli->lli_remote_perms = perm_hash; - else if (perm_hash) + else free_rmtperm_hash(perm_hash); head = lli->lli_remote_perms + remote_perm_hashfunc(perm->rp_uid); @@ -209,8 +209,7 @@ again: continue; if (tmp->lrp_fsgid != perm->rp_fsgid) continue; - if (lrp) - free_ll_remote_perm(lrp); + free_ll_remote_perm(lrp); lrp = tmp; break; } diff --git a/drivers/staging/lustre/lustre/llite/statahead.c b/drivers/staging/lustre/lustre/llite/statahead.c index 227854b..6ad9dd0 100644 --- a/drivers/staging/lustre/lustre/llite/statahead.c +++ b/drivers/staging/lustre/lustre/llite/statahead.c @@ -334,8 +334,7 @@ static void ll_sa_entry_put(struct ll_statahead_info *sai, LASSERT(ll_sa_entry_unhashed(entry)); ll_sa_entry_cleanup(sai, entry); - if (entry->se_inode) - iput(entry->se_inode); + iput(entry->se_inode); OBD_FREE(entry, entry->se_size); atomic_dec(&sai->sai_cache_count); -- 2.1.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 00/10] staging/irda/net: Adjustments for several function implementations
From: Markus Elfring Date: Thu, 12 Oct 2017 11:25:43 +0200 Several update suggestions were taken into account from static source code analysis. Markus Elfring (10): Improve a size determination in 20 functions Delete ten error messages for a failed memory allocation Adjust 385 checks for null pointers Delete an unnecessary variable initialisation in irlap_recv_discovery_xid_cmd() Delete an unnecessary variable initialisation in irlap_recv_discovery_xid_rsp() Delete an unnecessary variable initialisation in two functions Delete an unnecessary variable initialisation in four functions Use common error handling code in irias_new_object() Combine some seq_printf() calls in two functions Use seq_puts() in four functions drivers/staging/irda/net/af_irda.c| 56 + drivers/staging/irda/net/discovery.c | 31 +++-- drivers/staging/irda/net/ircomm/ircomm_core.c | 10 +- drivers/staging/irda/net/ircomm/ircomm_tty.c | 4 +- drivers/staging/irda/net/irda_device.c| 2 +- drivers/staging/irda/net/iriap.c | 61 +- drivers/staging/irda/net/iriap_event.c| 34 +++--- drivers/staging/irda/net/irias_object.c | 116 --- drivers/staging/irda/net/irlan/irlan_common.c | 3 +- drivers/staging/irda/net/irlap.c | 84 +++--- drivers/staging/irda/net/irlap_event.c| 77 ++-- drivers/staging/irda/net/irlap_frame.c| 74 ++-- drivers/staging/irda/net/irlmp.c | 161 -- drivers/staging/irda/net/irlmp_event.c| 50 drivers/staging/irda/net/irlmp_frame.c| 24 ++-- drivers/staging/irda/net/irnetlink.c | 2 +- drivers/staging/irda/net/irproc.c | 2 +- drivers/staging/irda/net/irqueue.c| 32 ++--- drivers/staging/irda/net/irsysctl.c | 4 +- drivers/staging/irda/net/irttp.c | 75 ++-- drivers/staging/irda/net/parameters.c | 12 +- drivers/staging/irda/net/qos.c| 20 ++-- drivers/staging/irda/net/timer.c | 12 +- drivers/staging/irda/net/wrapper.c| 3 +- 24 files changed, 450 insertions(+), 499 deletions(-) -- 2.14.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 01/10] staging: irda: Improve a size determination in 20 functions
From: Markus Elfring
Date: Tue, 10 Oct 2017 19:35:56 +0200
* Replace the specification of data types by pointer dereferences
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.
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 the affected source code place.
Signed-off-by: Markus Elfring
---
drivers/staging/irda/net/ircomm/ircomm_core.c | 2 +-
drivers/staging/irda/net/ircomm/ircomm_tty.c | 2 +-
drivers/staging/irda/net/irias_object.c | 16
drivers/staging/irda/net/irlap.c | 2 +-
drivers/staging/irda/net/irlap_frame.c| 7 ---
drivers/staging/irda/net/irlmp.c | 8
drivers/staging/irda/net/irttp.c | 4 ++--
7 files changed, 21 insertions(+), 20 deletions(-)
diff --git a/drivers/staging/irda/net/ircomm/ircomm_core.c
b/drivers/staging/irda/net/ircomm/ircomm_core.c
index 3af219545f6d..6c02fbf380bd 100644
--- a/drivers/staging/irda/net/ircomm/ircomm_core.c
+++ b/drivers/staging/irda/net/ircomm/ircomm_core.c
@@ -114,7 +114,7 @@ struct ircomm_cb *ircomm_open(notify_t *notify, __u8
service_type, int line)
IRDA_ASSERT(ircomm != NULL, return NULL;);
- self = kzalloc(sizeof(struct ircomm_cb), GFP_KERNEL);
+ self = kzalloc(sizeof(*self), GFP_KERNEL);
if (self == NULL)
return NULL;
diff --git a/drivers/staging/irda/net/ircomm/ircomm_tty.c
b/drivers/staging/irda/net/ircomm/ircomm_tty.c
index ec157c3419b5..d1beec413fa3 100644
--- a/drivers/staging/irda/net/ircomm/ircomm_tty.c
+++ b/drivers/staging/irda/net/ircomm/ircomm_tty.c
@@ -380,7 +380,7 @@ static int ircomm_tty_install(struct tty_driver *driver,
struct tty_struct *tty)
self = hashbin_lock_find(ircomm_tty, line, NULL);
if (!self) {
/* No, so make new instance */
- self = kzalloc(sizeof(struct ircomm_tty_cb), GFP_KERNEL);
+ self = kzalloc(sizeof(*self), GFP_KERNEL);
if (self == NULL)
return -ENOMEM;
diff --git a/drivers/staging/irda/net/irias_object.c
b/drivers/staging/irda/net/irias_object.c
index 53b86d0e1630..1064fac2fd36 100644
--- a/drivers/staging/irda/net/irias_object.c
+++ b/drivers/staging/irda/net/irias_object.c
@@ -48,7 +48,7 @@ struct ias_object *irias_new_object( char *name, int id)
{
struct ias_object *obj;
- obj = kzalloc(sizeof(struct ias_object), GFP_ATOMIC);
+ obj = kzalloc(sizeof(*obj), GFP_ATOMIC);
if (obj == NULL) {
net_warn_ratelimited("%s(), Unable to allocate object!\n",
__func__);
@@ -318,7 +318,7 @@ void irias_add_integer_attrib(struct ias_object *obj, char
*name, int value,
IRDA_ASSERT(obj->magic == IAS_OBJECT_MAGIC, return;);
IRDA_ASSERT(name != NULL, return;);
- attrib = kzalloc(sizeof(struct ias_attrib), GFP_ATOMIC);
+ attrib = kzalloc(sizeof(*attrib), GFP_ATOMIC);
if (attrib == NULL) {
net_warn_ratelimited("%s: Unable to allocate attribute!\n",
__func__);
@@ -362,7 +362,7 @@ void irias_add_octseq_attrib(struct ias_object *obj, char
*name, __u8 *octets,
IRDA_ASSERT(name != NULL, return;);
IRDA_ASSERT(octets != NULL, return;);
- attrib = kzalloc(sizeof(struct ias_attrib), GFP_ATOMIC);
+ attrib = kzalloc(sizeof(*attrib), GFP_ATOMIC);
if (attrib == NULL) {
net_warn_ratelimited("%s: Unable to allocate attribute!\n",
__func__);
@@ -404,7 +404,7 @@ void irias_add_string_attrib(struct ias_object *obj, char
*name, char *value,
IRDA_ASSERT(name != NULL, return;);
IRDA_ASSERT(value != NULL, return;);
- attrib = kzalloc(sizeof( struct ias_attrib), GFP_ATOMIC);
+ attrib = kzalloc(sizeof(*attrib), GFP_ATOMIC);
if (attrib == NULL) {
net_warn_ratelimited("%s: Unable to allocate attribute!\n",
__func__);
@@ -439,7 +439,7 @@ struct ias_value *irias_new_integer_value(int integer)
{
struct ias_value *value;
- value = kzalloc(sizeof(struct ias_value), GFP_ATOMIC);
+ value = kzalloc(sizeof(*value), GFP_ATOMIC);
if (value == NULL)
return NULL;
@@ -462,7 +462,7 @@ struct ias_value *irias_new_string_value(char *string)
{
struct ias_value *value;
- value = kzalloc(sizeof(struct ias_value), GFP_ATOMIC);
+ value = kzalloc(sizeof(*value), GFP_ATOMIC);
if (value == NULL)
return NULL;
@@ -491,7 +491,7 @@ struct ias_value *irias_new_octseq_value(__u8 *octseq , int
len)
{
struct ias_value *value;
- value = kzallo
[PATCH 02/10] staging: irda: Delete ten error messages for a failed memory allocation
From: Markus Elfring
Date: Tue, 10 Oct 2017 21:10:43 +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/staging/irda/net/irias_object.c | 24
drivers/staging/irda/net/irlap_frame.c | 4 +---
drivers/staging/irda/net/irlmp.c| 1 -
drivers/staging/irda/net/irttp.c| 1 -
4 files changed, 5 insertions(+), 25 deletions(-)
diff --git a/drivers/staging/irda/net/irias_object.c
b/drivers/staging/irda/net/irias_object.c
index 1064fac2fd36..4db986b9d756 100644
--- a/drivers/staging/irda/net/irias_object.c
+++ b/drivers/staging/irda/net/irias_object.c
@@ -49,17 +49,12 @@ struct ias_object *irias_new_object( char *name, int id)
struct ias_object *obj;
obj = kzalloc(sizeof(*obj), GFP_ATOMIC);
- if (obj == NULL) {
- net_warn_ratelimited("%s(), Unable to allocate object!\n",
-__func__);
+ if (!obj)
return NULL;
- }
obj->magic = IAS_OBJECT_MAGIC;
obj->name = kstrndup(name, IAS_MAX_CLASSNAME, GFP_ATOMIC);
if (!obj->name) {
- net_warn_ratelimited("%s(), Unable to allocate name!\n",
-__func__);
kfree(obj);
return NULL;
}
@@ -319,11 +314,8 @@ void irias_add_integer_attrib(struct ias_object *obj, char
*name, int value,
IRDA_ASSERT(name != NULL, return;);
attrib = kzalloc(sizeof(*attrib), GFP_ATOMIC);
- if (attrib == NULL) {
- net_warn_ratelimited("%s: Unable to allocate attribute!\n",
-__func__);
+ if (!attrib)
return;
- }
attrib->magic = IAS_ATTRIB_MAGIC;
attrib->name = kstrndup(name, IAS_MAX_ATTRIBNAME, GFP_ATOMIC);
@@ -363,11 +355,8 @@ void irias_add_octseq_attrib(struct ias_object *obj, char
*name, __u8 *octets,
IRDA_ASSERT(octets != NULL, return;);
attrib = kzalloc(sizeof(*attrib), GFP_ATOMIC);
- if (attrib == NULL) {
- net_warn_ratelimited("%s: Unable to allocate attribute!\n",
-__func__);
+ if (!attrib)
return;
- }
attrib->magic = IAS_ATTRIB_MAGIC;
attrib->name = kstrndup(name, IAS_MAX_ATTRIBNAME, GFP_ATOMIC);
@@ -405,11 +394,8 @@ void irias_add_string_attrib(struct ias_object *obj, char
*name, char *value,
IRDA_ASSERT(value != NULL, return;);
attrib = kzalloc(sizeof(*attrib), GFP_ATOMIC);
- if (attrib == NULL) {
- net_warn_ratelimited("%s: Unable to allocate attribute!\n",
-__func__);
+ if (!attrib)
return;
- }
attrib->magic = IAS_ATTRIB_MAGIC;
attrib->name = kstrndup(name, IAS_MAX_ATTRIBNAME, GFP_ATOMIC);
@@ -470,7 +456,6 @@ struct ias_value *irias_new_string_value(char *string)
value->charset = CS_ASCII;
value->t.string = kstrndup(string, IAS_MAX_STRING, GFP_ATOMIC);
if (!value->t.string) {
- net_warn_ratelimited("%s: Unable to kmalloc!\n", __func__);
kfree(value);
return NULL;
}
@@ -503,7 +488,6 @@ struct ias_value *irias_new_octseq_value(__u8 *octseq , int
len)
value->t.oct_seq = kmemdup(octseq, len, GFP_ATOMIC);
if (value->t.oct_seq == NULL){
- net_warn_ratelimited("%s: Unable to kmalloc!\n", __func__);
kfree(value);
return NULL;
}
diff --git a/drivers/staging/irda/net/irlap_frame.c
b/drivers/staging/irda/net/irlap_frame.c
index 21891ef7ee33..d4d88a5d2976 100644
--- a/drivers/staging/irda/net/irlap_frame.c
+++ b/drivers/staging/irda/net/irlap_frame.c
@@ -433,10 +433,8 @@ static void irlap_recv_discovery_xid_rsp(struct irlap_cb
*self,
}
discovery = kzalloc(sizeof(*discovery), GFP_ATOMIC);
- if (!discovery) {
- net_warn_ratelimited("%s: kmalloc failed!\n", __func__);
+ if (!discovery)
return;
- }
discovery->data.daddr = info->daddr;
discovery->data.saddr = self->saddr;
diff --git a/drivers/staging/irda/net/irlmp.c b/drivers/staging/irda/net/irlmp.c
index 38772a3b9df8..f075735e4b9b 100644
--- a/drivers/staging/irda/net/irlmp.c
+++ b/drivers/staging/irda/net/irlmp.c
@@ -641,7 +641,6 @@ struct lsap_cb *irlmp_dup(struct lsap_cb *orig, void
*instance)
/* Allocate a new instance */
new = kmemdup(orig, sizeof(*new), GFP_ATOMIC);
if (!new) {
- pr_debug("%s(), unable to kmalloc\n", __func__);
spin_unlock_irqrestore(&irlmp->unconnected_lsaps->hb_spinlock,
flags);
return NULL;
diff --git a/drivers/staging/irda/net/irttp.c b/drivers/staging/
[PATCH 03/10] staging/irda/net: Adjust 385 checks for null pointers
From: Markus Elfring
Date: Wed, 11 Oct 2017 22:10:26 +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.
Use space characters at some places according to the Linux coding
style convention.
Signed-off-by: Markus Elfring
---
drivers/staging/irda/net/af_irda.c | 56 ++---
drivers/staging/irda/net/discovery.c| 31 ---
drivers/staging/irda/net/irda_device.c | 2 +-
drivers/staging/irda/net/iriap.c| 61 +++---
drivers/staging/irda/net/iriap_event.c | 34
drivers/staging/irda/net/irias_object.c | 62 +++---
drivers/staging/irda/net/irlap.c| 73
drivers/staging/irda/net/irlap_event.c | 77 +
drivers/staging/irda/net/irlap_frame.c | 51 +--
drivers/staging/irda/net/irlmp.c| 144 +++-
drivers/staging/irda/net/irlmp_event.c | 50 ++-
drivers/staging/irda/net/irlmp_frame.c | 24 +++---
drivers/staging/irda/net/irnetlink.c| 2 +-
drivers/staging/irda/net/irproc.c | 2 +-
drivers/staging/irda/net/irqueue.c | 32 +++
drivers/staging/irda/net/irsysctl.c | 4 +-
drivers/staging/irda/net/irttp.c| 68 +++
drivers/staging/irda/net/parameters.c | 12 +--
drivers/staging/irda/net/qos.c | 20 ++---
drivers/staging/irda/net/timer.c| 12 +--
drivers/staging/irda/net/wrapper.c | 3 +-
21 files changed, 396 insertions(+), 424 deletions(-)
diff --git a/drivers/staging/irda/net/af_irda.c
b/drivers/staging/irda/net/af_irda.c
index 23fa7c8b09a5..cb60b89053dd 100644
--- a/drivers/staging/irda/net/af_irda.c
+++ b/drivers/staging/irda/net/af_irda.c
@@ -121,7 +121,7 @@ static void irda_disconnect_indication(void *instance, void
*sap,
dev_kfree_skb(skb);
sk = instance;
- if (sk == NULL) {
+ if (!sk) {
pr_debug("%s(%p) : BUG : sk is NULL\n",
__func__, self);
return;
@@ -182,7 +182,7 @@ static void irda_connect_confirm(void *instance, void *sap,
pr_debug("%s(%p)\n", __func__, self);
sk = instance;
- if (sk == NULL) {
+ if (!sk) {
dev_kfree_skb(skb);
return;
}
@@ -246,7 +246,7 @@ static void irda_connect_indication(void *instance, void
*sap,
pr_debug("%s(%p)\n", __func__, self);
sk = instance;
- if (sk == NULL) {
+ if (!sk) {
dev_kfree_skb(skb);
return;
}
@@ -301,7 +301,7 @@ static void irda_connect_response(struct irda_sock *self)
struct sk_buff *skb;
skb = alloc_skb(TTP_MAX_HEADER + TTP_SAR_HEADER, GFP_KERNEL);
- if (skb == NULL) {
+ if (!skb) {
pr_debug("%s() Unable to allocate sk_buff!\n",
__func__);
return;
@@ -326,7 +326,7 @@ static void irda_flow_indication(void *instance, void *sap,
LOCAL_FLOW flow)
self = instance;
sk = instance;
- BUG_ON(sk == NULL);
+ BUG_ON(!sk);
switch (flow) {
case FLOW_STOP:
@@ -434,7 +434,7 @@ static void irda_discovery_timeout(u_long priv)
struct irda_sock *self;
self = (struct irda_sock *) priv;
- BUG_ON(self == NULL);
+ BUG_ON(!self);
/* Nothing for the caller */
self->cachelog = NULL;
@@ -473,7 +473,7 @@ static int irda_open_tsap(struct irda_sock *self, __u8
tsap_sel, char *name)
self->tsap = irttp_open_tsap(tsap_sel, DEFAULT_INITIAL_CREDIT,
¬ify);
- if (self->tsap == NULL) {
+ if (!self->tsap) {
pr_debug("%s(), Unable to allocate TSAP!\n",
__func__);
return -ENOMEM;
@@ -507,7 +507,7 @@ static int irda_open_lsap(struct irda_sock *self, int pid)
strncpy(notify.name, "Ultra", NOTIFY_MAX_NAME);
self->lsap = irlmp_open_lsap(LSAP_CONNLESS, ¬ify, pid);
- if (self->lsap == NULL) {
+ if (!self->lsap) {
pr_debug("%s(), Unable to allocate LSAP!\n", __func__);
return -ENOMEM;
}
@@ -539,7 +539,7 @@ static int irda_find_lsap_sel(struct irda_sock *self, char
*name)
self->iriap = iriap_open(LSAP_ANY, IAS_CLIENT, self,
irda_getvalue_confirm);
- if(self->iriap == NULL)
+ if (!self->iriap)
return -ENOMEM;
/* Treat unexpected wakeup as disconnect */
@@ -625,7 +625,7 @@ static int irda_discover_daddr_and_lsap_sel(struct
irda_sock *self, char *name)
discoveries = irlmp_get_discoveries(&number, self->mask.word,
self->nslots);
/* Check if the we got so
[PATCH 04/10] staging/irda/net: Delete an unnecessary variable initialisation in irlap_recv_discovery_xid_cmd()
From: Markus Elfring
Date: Wed, 11 Oct 2017 22:18:34 +0200
The local variable "discovery" will only be used in a single if branch
of this function. Thus omit the explicit initialisation at the beginning.
Signed-off-by: Markus Elfring
---
drivers/staging/irda/net/irlap_frame.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/irda/net/irlap_frame.c
b/drivers/staging/irda/net/irlap_frame.c
index 94972db87951..33259ed5f3cd 100644
--- a/drivers/staging/irda/net/irlap_frame.c
+++ b/drivers/staging/irda/net/irlap_frame.c
@@ -481,7 +481,7 @@ static void irlap_recv_discovery_xid_cmd(struct irlap_cb
*self,
struct irlap_info *info)
{
struct xid_frame *xid;
- discovery_t *discovery = NULL;
+ discovery_t *discovery;
__u8 *discovery_info;
char *text;
--
2.14.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 05/10] staging/irda/net: Delete an unnecessary variable initialisation in irlap_recv_discovery_xid_rsp()
From: Markus Elfring
Date: Wed, 11 Oct 2017 22:20:22 +0200
The variable "discovery" will eventually be set to an appropriate pointer
a bit later. Thus omit the explicit initialisation at the beginning.
Signed-off-by: Markus Elfring
---
drivers/staging/irda/net/irlap_frame.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/irda/net/irlap_frame.c
b/drivers/staging/irda/net/irlap_frame.c
index 33259ed5f3cd..5b8be5b9812e 100644
--- a/drivers/staging/irda/net/irlap_frame.c
+++ b/drivers/staging/irda/net/irlap_frame.c
@@ -408,7 +408,7 @@ static void irlap_recv_discovery_xid_rsp(struct irlap_cb
*self,
struct irlap_info *info)
{
struct xid_frame *xid;
- discovery_t *discovery = NULL;
+ discovery_t *discovery;
__u8 *discovery_info;
char *text;
--
2.14.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 06/10] staging/irda/net: Delete an unnecessary variable initialisation in two functions
From: Markus Elfring
Date: Wed, 11 Oct 2017 22:22:13 +0200
The local variable "tx_skb" will only be used in a single if branch
of these functions. Thus omit the explicit initialisation at the beginning.
Signed-off-by: Markus Elfring
---
drivers/staging/irda/net/irlap_frame.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/irda/net/irlap_frame.c
b/drivers/staging/irda/net/irlap_frame.c
index 5b8be5b9812e..25ceb06efd58 100644
--- a/drivers/staging/irda/net/irlap_frame.c
+++ b/drivers/staging/irda/net/irlap_frame.c
@@ -862,7 +862,7 @@ void irlap_send_data_primary_poll(struct irlap_cb *self,
struct sk_buff *skb)
void irlap_send_data_secondary_final(struct irlap_cb *self,
struct sk_buff *skb)
{
- struct sk_buff *tx_skb = NULL;
+ struct sk_buff *tx_skb;
IRDA_ASSERT(self, return;);
IRDA_ASSERT(self->magic == LAP_MAGIC, return;);
@@ -922,7 +922,7 @@ void irlap_send_data_secondary_final(struct irlap_cb *self,
*/
void irlap_send_data_secondary(struct irlap_cb *self, struct sk_buff *skb)
{
- struct sk_buff *tx_skb = NULL;
+ struct sk_buff *tx_skb;
/* Is this reliable or unreliable data? */
if (skb->data[1] == I_FRAME) {
--
2.14.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 07/10] staging/irda/net: Delete an unnecessary variable initialisation in four functions
From: Markus Elfring
Date: Wed, 11 Oct 2017 22:26:00 +0200
The variable "tx_skb" will eventually be set to an appropriate pointer
a bit later. Thus omit the explicit initialisation at the beginning.
Signed-off-by: Markus Elfring
---
drivers/staging/irda/net/irlap_frame.c | 6 +++---
drivers/staging/irda/net/irttp.c | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/irda/net/irlap_frame.c
b/drivers/staging/irda/net/irlap_frame.c
index 25ceb06efd58..16fe7f53fdb7 100644
--- a/drivers/staging/irda/net/irlap_frame.c
+++ b/drivers/staging/irda/net/irlap_frame.c
@@ -258,7 +258,7 @@ void irlap_send_ua_response_frame(struct irlap_cb *self,
struct qos_info *qos)
*/
void irlap_send_dm_frame( struct irlap_cb *self)
{
- struct sk_buff *tx_skb = NULL;
+ struct sk_buff *tx_skb;
struct dm_frame *frame;
IRDA_ASSERT(self, return;);
@@ -288,7 +288,7 @@ void irlap_send_dm_frame( struct irlap_cb *self)
*/
void irlap_send_disc_frame(struct irlap_cb *self)
{
- struct sk_buff *tx_skb = NULL;
+ struct sk_buff *tx_skb;
struct disc_frame *frame;
IRDA_ASSERT(self, return;);
@@ -315,7 +315,7 @@ void irlap_send_disc_frame(struct irlap_cb *self)
void irlap_send_discovery_xid_frame(struct irlap_cb *self, int S, __u8 s,
__u8 command, discovery_t *discovery)
{
- struct sk_buff *tx_skb = NULL;
+ struct sk_buff *tx_skb;
struct xid_frame *frame;
__u32 bcast = BROADCAST;
__u8 *info;
diff --git a/drivers/staging/irda/net/irttp.c b/drivers/staging/irda/net/irttp.c
index 2adba87aeb68..bc612227cdc3 100644
--- a/drivers/staging/irda/net/irttp.c
+++ b/drivers/staging/irda/net/irttp.c
@@ -811,7 +811,7 @@ static void irttp_run_tx_queue(struct tsap_cb *self)
*/
static inline void irttp_give_credit(struct tsap_cb *self)
{
- struct sk_buff *tx_skb = NULL;
+ struct sk_buff *tx_skb;
unsigned long flags;
int n;
--
2.14.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 08/10] staging/irda/net: Use common error handling code in irias_new_object()
From: Markus Elfring
Date: Thu, 12 Oct 2017 08:52:53 +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/staging/irda/net/irias_object.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/irda/net/irias_object.c
b/drivers/staging/irda/net/irias_object.c
index 4c2c65e28836..0c89800d597d 100644
--- a/drivers/staging/irda/net/irias_object.c
+++ b/drivers/staging/irda/net/irias_object.c
@@ -54,10 +54,9 @@ struct ias_object *irias_new_object( char *name, int id)
obj->magic = IAS_OBJECT_MAGIC;
obj->name = kstrndup(name, IAS_MAX_CLASSNAME, GFP_ATOMIC);
- if (!obj->name) {
- kfree(obj);
- return NULL;
- }
+ if (!obj->name)
+ goto free_object;
+
obj->id = id;
/* Locking notes : the attrib spinlock has lower precendence
@@ -68,11 +67,14 @@ struct ias_object *irias_new_object( char *name, int id)
net_warn_ratelimited("%s(), Unable to allocate attribs!\n",
__func__);
kfree(obj->name);
- kfree(obj);
- return NULL;
+ goto free_object;
}
return obj;
+
+free_object:
+ kfree(obj);
+ return NULL;
}
EXPORT_SYMBOL(irias_new_object);
--
2.14.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 09/10] staging/irda/net: Combine some seq_printf() calls in two functions
From: Markus Elfring
Date: Thu, 12 Oct 2017 08:58:38 +0200
Some data were printed into a sequence by separate function calls.
Print the same data by a single function call at each place instead.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/staging/irda/net/irlap.c | 9 -
drivers/staging/irda/net/irlmp.c | 6 ++
2 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/irda/net/irlap.c b/drivers/staging/irda/net/irlap.c
index 715cedab2f41..345c4eb55a59 100644
--- a/drivers/staging/irda/net/irlap.c
+++ b/drivers/staging/irda/net/irlap.c
@@ -1141,9 +1141,9 @@ static int irlap_seq_show(struct seq_file *seq, void *v)
seq_printf(seq, "vr: %d ", self->vr);
seq_printf(seq, "va: %d\n", self->va);
- seq_printf(seq, "
qos\tbps\tmaxtt\tdsize\twinsize\taddbofs\tmintt\tldisc\tcomp\n");
-
- seq_printf(seq, " tx\t%d\t",
+ seq_printf(seq,
+ "
qos\tbps\tmaxtt\tdsize\twinsize\taddbofs\tmintt\tldisc\tcomp\n"
+ " tx\t%d\t",
self->qos_tx.baud_rate.value);
seq_printf(seq, "%d\t",
self->qos_tx.max_turn_time.value);
@@ -1157,9 +1157,8 @@ static int irlap_seq_show(struct seq_file *seq, void *v)
self->qos_tx.min_turn_time.value);
seq_printf(seq, "%d\t",
self->qos_tx.link_disc_time.value);
- seq_printf(seq, "\n");
- seq_printf(seq, " rx\t%d\t",
+ seq_printf(seq, "\n rx\t%d\t",
self->qos_rx.baud_rate.value);
seq_printf(seq, "%d\t",
self->qos_rx.max_turn_time.value);
diff --git a/drivers/staging/irda/net/irlmp.c b/drivers/staging/irda/net/irlmp.c
index 318660fbc094..6a09cf621bd4 100644
--- a/drivers/staging/irda/net/irlmp.c
+++ b/drivers/staging/irda/net/irlmp.c
@@ -1920,8 +1920,7 @@ static int irlmp_seq_show(struct seq_file *seq, void *v)
seq_printf(seq,
"slsap_sel: %#02x, dlsap_sel: %#02x, ",
self->slsap_sel, self->dlsap_sel);
- seq_printf(seq, "(%s)", self->notify.name);
- seq_printf(seq, "\n");
+ seq_printf(seq, "(%s)\n", self->notify.name);
} else if (iter->hashbin == irlmp->links) {
struct lap_cb *lap = v;
@@ -1930,9 +1929,8 @@ static int irlmp_seq_show(struct seq_file *seq, void *v)
seq_printf(seq, "saddr: %#08x, daddr: %#08x, ",
lap->saddr, lap->daddr);
- seq_printf(seq, "num lsaps: %d",
+ seq_printf(seq, "num lsaps: %d\n",
HASHBIN_GET_SIZE(lap->lsaps));
- seq_printf(seq, "\n");
/* Careful for priority inversions here !
* All other uses of attrib spinlock are independent of
--
2.14.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 10/10] staging/irda/net: Use seq_puts() in four functions
From: Markus Elfring
Date: Thu, 12 Oct 2017 11:08:36 +0200
Strings which did not contain a data format specification should be put
into a sequence. Thus use the corresponding function "seq_puts".
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/staging/irda/net/ircomm/ircomm_core.c | 8
drivers/staging/irda/net/ircomm/ircomm_tty.c | 2 +-
drivers/staging/irda/net/irlan/irlan_common.c | 3 +--
drivers/staging/irda/net/irlmp.c | 2 +-
4 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/irda/net/ircomm/ircomm_core.c
b/drivers/staging/irda/net/ircomm/ircomm_core.c
index 6c02fbf380bd..9bb2ff306470 100644
--- a/drivers/staging/irda/net/ircomm/ircomm_core.c
+++ b/drivers/staging/irda/net/ircomm/ircomm_core.c
@@ -530,13 +530,13 @@ static int ircomm_seq_show(struct seq_file *seq, void *v)
self->slsap_sel, self->dlsap_sel);
if(self->service_type & IRCOMM_3_WIRE_RAW)
- seq_printf(seq, " 3-wire-raw");
+ seq_puts(seq, " 3-wire-raw");
if(self->service_type & IRCOMM_3_WIRE)
- seq_printf(seq, " 3-wire");
+ seq_puts(seq, " 3-wire");
if(self->service_type & IRCOMM_9_WIRE)
- seq_printf(seq, " 9-wire");
+ seq_puts(seq, " 9-wire");
if(self->service_type & IRCOMM_CENTRONICS)
- seq_printf(seq, " Centronics");
+ seq_puts(seq, " Centronics");
seq_putc(seq, '\n');
return 0;
diff --git a/drivers/staging/irda/net/ircomm/ircomm_tty.c
b/drivers/staging/irda/net/ircomm/ircomm_tty.c
index d1beec413fa3..dc7097576917 100644
--- a/drivers/staging/irda/net/ircomm/ircomm_tty.c
+++ b/drivers/staging/irda/net/ircomm/ircomm_tty.c
@@ -1174,7 +1174,7 @@ static void ircomm_tty_line_info(struct ircomm_tty_cb
*self, struct seq_file *m)
seq_printf(m, "Port name: %s\n", self->settings.port_name);
- seq_printf(m, "DTE status:");
+ seq_puts(m, "DTE status:");
sep = ' ';
if (self->settings.dte & IRCOMM_RTS) {
seq_printf(m, "%cRTS", sep);
diff --git a/drivers/staging/irda/net/irlan/irlan_common.c
b/drivers/staging/irda/net/irlan/irlan_common.c
index 481bbc2a4349..f4633d4dc390 100644
--- a/drivers/staging/irda/net/irlan/irlan_common.c
+++ b/drivers/staging/irda/net/irlan/irlan_common.c
@@ -1138,8 +1138,7 @@ static int irlan_seq_show(struct seq_file *seq, void *v)
seq_printf(seq,"media: %s\n",
irlan_media[self->media]);
- seq_printf(seq,"local filter:\n");
- seq_printf(seq,"remote filter: ");
+ seq_puts(seq, "local filter:\nremote filter: ");
irlan_print_filter(seq, self->client.filter_type);
seq_printf(seq,"tx busy: %s\n",
netif_queue_stopped(self->dev) ? "TRUE" :
"FALSE");
diff --git a/drivers/staging/irda/net/irlmp.c b/drivers/staging/irda/net/irlmp.c
index 6a09cf621bd4..60a3dd7d6f49 100644
--- a/drivers/staging/irda/net/irlmp.c
+++ b/drivers/staging/irda/net/irlmp.c
@@ -1937,7 +1937,7 @@ static int irlmp_seq_show(struct seq_file *seq, void *v)
* the object spinlock, so we are safe. Jean II */
spin_lock(&lap->lsaps->hb_spinlock);
- seq_printf(seq, "\n Connected LSAPs:\n");
+ seq_puts(seq, "\n Connected LSAPs:\n");
for (self = (struct lsap_cb *) hashbin_get_first(lap->lsaps);
self;
self = (struct lsap_cb *)hashbin_get_next(lap->lsaps)) {
--
2.14.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: staging/irda/net: Adjustments for several function implementations
> Did you read drivers/staging/irda/TODO ? Yes. How do recent contributions (by other software developers) fit to information that is provided in this file? Regards, Markus ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: comedi: usbduxfast: Improve unlocking of a mutex in usbduxfast_ai_insn_read()
From: Markus Elfring
Date: Thu, 2 Nov 2017 20:30:31 +0100
* Add a jump target so that a call of the function "mutex_unlock" is stored
only twice in this function implementation.
* Replace five calls by goto statements.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/staging/comedi/drivers/usbduxfast.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/comedi/drivers/usbduxfast.c
b/drivers/staging/comedi/drivers/usbduxfast.c
index 608403c7586b..e5884faf7275 100644
--- a/drivers/staging/comedi/drivers/usbduxfast.c
+++ b/drivers/staging/comedi/drivers/usbduxfast.c
@@ -777,8 +777,8 @@ static int usbduxfast_ai_insn_read(struct comedi_device
*dev,
if (devpriv->ai_cmd_running) {
dev_err(dev->class_dev,
"ai_insn_read not possible, async cmd is running\n");
- mutex_unlock(&devpriv->mut);
- return -EBUSY;
+ ret = -EBUSY;
+ goto unlock;
}
/* set command for the first channel */
@@ -798,10 +798,8 @@ static int usbduxfast_ai_insn_read(struct comedi_device
*dev,
usbduxfast_cmd_data(dev, 6, 0x01, 0x00, rngmask, 0x00);
ret = usbduxfast_send_cmd(dev, SENDADCOMMANDS);
- if (ret < 0) {
- mutex_unlock(&devpriv->mut);
- return ret;
- }
+ if (ret < 0)
+ goto unlock;
for (i = 0; i < PACKETS_TO_IGNORE; i++) {
ret = usb_bulk_msg(usb, usb_rcvbulkpipe(usb, BULKINEP),
@@ -809,8 +807,7 @@ static int usbduxfast_ai_insn_read(struct comedi_device
*dev,
&actual_length, 1);
if (ret < 0) {
dev_err(dev->class_dev, "insn timeout, no data\n");
- mutex_unlock(&devpriv->mut);
- return ret;
+ goto unlock;
}
}
@@ -820,14 +817,13 @@ static int usbduxfast_ai_insn_read(struct comedi_device
*dev,
&actual_length, 1);
if (ret < 0) {
dev_err(dev->class_dev, "insn data error: %d\n", ret);
- mutex_unlock(&devpriv->mut);
- return ret;
+ goto unlock;
}
n = actual_length / sizeof(u16);
if ((n % 16) != 0) {
dev_err(dev->class_dev, "insn data packet corrupted\n");
- mutex_unlock(&devpriv->mut);
- return -EINVAL;
+ ret = -EINVAL;
+ goto unlock;
}
for (j = chan; (j < n) && (i < insn->n); j = j + 16) {
data[i] = ((u16 *)(devpriv->inbuf))[j];
@@ -838,6 +834,10 @@ static int usbduxfast_ai_insn_read(struct comedi_device
*dev,
mutex_unlock(&devpriv->mut);
return insn->n;
+
+unlock:
+ mutex_unlock(&devpriv->mut);
+ return ret;
}
static int usbduxfast_upload_firmware(struct comedi_device *dev,
--
2.15.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: comedi: usbduxsigma: Improve unlocking of a mutex in three functions
From: Markus Elfring
Date: Thu, 2 Nov 2017 21:16:50 +0100
* Add a jump target so that a call of the function "mutex_unlock" is stored
only twice in these function implementations.
* Replace seven calls by goto statements.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/staging/comedi/drivers/usbduxsigma.c | 48 +++-
1 file changed, 26 insertions(+), 22 deletions(-)
diff --git a/drivers/staging/comedi/drivers/usbduxsigma.c
b/drivers/staging/comedi/drivers/usbduxsigma.c
index 456e9f13becb..7e8284ed265a 100644
--- a/drivers/staging/comedi/drivers/usbduxsigma.c
+++ b/drivers/staging/comedi/drivers/usbduxsigma.c
@@ -672,10 +672,8 @@ static int usbduxsigma_ai_cmd(struct comedi_device *dev,
devpriv->dux_commands[8] = sysred;
ret = usbbuxsigma_send_cmd(dev, USBBUXSIGMA_AD_CMD);
- if (ret < 0) {
- mutex_unlock(&devpriv->mut);
- return ret;
- }
+ if (ret < 0)
+ goto unlock;
devpriv->ai_counter = devpriv->ai_timer;
@@ -686,8 +684,7 @@ static int usbduxsigma_ai_cmd(struct comedi_device *dev,
devpriv->n_ai_urbs, 1);
if (ret < 0) {
devpriv->ai_cmd_running = 0;
- mutex_unlock(&devpriv->mut);
- return ret;
+ goto unlock;
}
s->async->inttrig = NULL;
} else {/* TRIG_INT */
@@ -697,6 +694,10 @@ static int usbduxsigma_ai_cmd(struct comedi_device *dev,
mutex_unlock(&devpriv->mut);
return 0;
+
+unlock:
+ mutex_unlock(&devpriv->mut);
+ return ret;
}
static int usbduxsigma_ai_insn_read(struct comedi_device *dev,
@@ -714,8 +715,8 @@ static int usbduxsigma_ai_insn_read(struct comedi_device
*dev,
mutex_lock(&devpriv->mut);
if (devpriv->ai_cmd_running) {
- mutex_unlock(&devpriv->mut);
- return -EBUSY;
+ ret = -EBUSY;
+ goto unlock;
}
create_adc_command(chan, &muxsg0, &muxsg1);
@@ -730,19 +731,15 @@ static int usbduxsigma_ai_insn_read(struct comedi_device
*dev,
/* adc commands */
ret = usbbuxsigma_send_cmd(dev, USBDUXSIGMA_SINGLE_AD_CMD);
- if (ret < 0) {
- mutex_unlock(&devpriv->mut);
- return ret;
- }
+ if (ret < 0)
+ goto unlock;
for (i = 0; i < insn->n; i++) {
u32 val;
ret = usbduxsigma_receive_cmd(dev, USBDUXSIGMA_SINGLE_AD_CMD);
- if (ret < 0) {
- mutex_unlock(&devpriv->mut);
- return ret;
- }
+ if (ret < 0)
+ goto unlock;
/* 32 bits big endian from the A/D converter */
val = be32_to_cpu(get_unaligned((__be32
@@ -753,6 +750,10 @@ static int usbduxsigma_ai_insn_read(struct comedi_device
*dev,
mutex_unlock(&devpriv->mut);
return insn->n;
+
+unlock:
+ mutex_unlock(&devpriv->mut);
+ return ret;
}
static int usbduxsigma_ao_insn_read(struct comedi_device *dev,
@@ -782,8 +783,8 @@ static int usbduxsigma_ao_insn_write(struct comedi_device
*dev,
mutex_lock(&devpriv->mut);
if (devpriv->ao_cmd_running) {
- mutex_unlock(&devpriv->mut);
- return -EBUSY;
+ ret = -EBUSY;
+ goto unlock;
}
for (i = 0; i < insn->n; i++) {
@@ -791,15 +792,18 @@ static int usbduxsigma_ao_insn_write(struct comedi_device
*dev,
devpriv->dux_commands[2] = data[i]; /* value */
devpriv->dux_commands[3] = chan;/* channel number */
ret = usbbuxsigma_send_cmd(dev, USBDUXSIGMA_DA_CMD);
- if (ret < 0) {
- mutex_unlock(&devpriv->mut);
- return ret;
- }
+ if (ret < 0)
+ goto unlock;
+
s->readback[chan] = data[i];
}
mutex_unlock(&devpriv->mut);
return insn->n;
+
+unlock:
+ mutex_unlock(&devpriv->mut);
+ return ret;
}
static int usbduxsigma_ao_inttrig(struct comedi_device *dev,
--
2.15.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: fbtft: fb_ra8875: Use common error handling code in write_reg8_bus8()
From: Markus Elfring
Date: Thu, 2 Nov 2017 22:12:58 +0100
* Add a jump target so that a specific error message is stored only once
at the end of this function implementation.
* Replace two calls of the function "dev_err" by goto statements.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/staging/fbtft/fb_ra8875.c | 22 ++
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/fbtft/fb_ra8875.c
b/drivers/staging/fbtft/fb_ra8875.c
index 6d1cad85957b..6a94b5810a31 100644
--- a/drivers/staging/fbtft/fb_ra8875.c
+++ b/drivers/staging/fbtft/fb_ra8875.c
@@ -218,12 +218,9 @@ static void write_reg8_bus8(struct fbtft_par *par, int
len, ...)
*buf++ = 0x80;
*buf = (u8)va_arg(args, unsigned int);
ret = par->fbtftops.write(par, par->buf, 2);
- if (ret < 0) {
- va_end(args);
- dev_err(par->info->device, "write() failed and returned %dn",
- ret);
- return;
- }
+ if (ret < 0)
+ goto end_va;
+
len--;
udelay(100);
@@ -236,18 +233,19 @@ static void write_reg8_bus8(struct fbtft_par *par, int
len, ...)
*buf++ = (u8)va_arg(args, unsigned int);
ret = par->fbtftops.write(par, par->buf, len + 1);
- if (ret < 0) {
- va_end(args);
- dev_err(par->info->device,
- "write() failed and returned %dn", ret);
- return;
- }
+ if (ret < 0)
+ goto end_va;
}
va_end(args);
/* restore user spi-speed */
par->fbtftops.write = fbtft_write_spi;
udelay(100);
+ return;
+
+end_va:
+ va_end(args);
+ dev_err(par->info->device, "write() failed and returned %dn", ret);
}
static int write_vmem16_bus8(struct fbtft_par *par, size_t offset, size_t len)
--
2.15.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: fbtft: fb_ssd1331: Use common error handling code in write_reg8_bus8()
From: Markus Elfring
Date: Thu, 2 Nov 2017 22:27:13 +0100
* Add a jump target so that a specific error message is stored only once
at the end of this function implementation.
* Replace two calls of the function "dev_err" by goto statements.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/staging/fbtft/fb_ssd1331.c | 22 ++
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/fbtft/fb_ssd1331.c
b/drivers/staging/fbtft/fb_ssd1331.c
index 9aa9864fcf30..b3a8f2668951 100644
--- a/drivers/staging/fbtft/fb_ssd1331.c
+++ b/drivers/staging/fbtft/fb_ssd1331.c
@@ -76,12 +76,9 @@ static void write_reg8_bus8(struct fbtft_par *par, int len,
...)
if (par->gpio.dc != -1)
gpio_set_value(par->gpio.dc, 0);
ret = par->fbtftops.write(par, par->buf, sizeof(u8));
- if (ret < 0) {
- va_end(args);
- dev_err(par->info->device,
- "write() failed and returned %d\n", ret);
- return;
- }
+ if (ret < 0)
+ goto end_va;
+
len--;
if (len) {
@@ -89,16 +86,17 @@ static void write_reg8_bus8(struct fbtft_par *par, int len,
...)
while (i--)
*buf++ = (u8)va_arg(args, unsigned int);
ret = par->fbtftops.write(par, par->buf, len * (sizeof(u8)));
- if (ret < 0) {
- va_end(args);
- dev_err(par->info->device,
- "write() failed and returned %d\n", ret);
- return;
- }
+ if (ret < 0)
+ goto end_va;
}
if (par->gpio.dc != -1)
gpio_set_value(par->gpio.dc, 1);
va_end(args);
+ return;
+
+end_va:
+ va_end(args);
+ dev_err(par->info->device, "write() failed and returned %d\n", ret);
}
/*
--
2.15.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: iio: ad7152: Improve unlocking of a mutex in ad7152_start_calib()
From: Markus Elfring
Date: Fri, 3 Nov 2017 09:00:25 +0100
* Add a jump target so that a call of the function "mutex_unlock" is stored
only twice in this function implementation.
* Replace two calls by goto statements.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/staging/iio/cdc/ad7152.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/iio/cdc/ad7152.c b/drivers/staging/iio/cdc/ad7152.c
index 61377ca444de..59d1b35f6a4e 100644
--- a/drivers/staging/iio/cdc/ad7152.c
+++ b/drivers/staging/iio/cdc/ad7152.c
@@ -118,22 +118,23 @@ static inline ssize_t ad7152_start_calib(struct device
*dev,
mutex_lock(&chip->state_lock);
ret = i2c_smbus_write_byte_data(chip->client, AD7152_REG_CFG, regval);
- if (ret < 0) {
- mutex_unlock(&chip->state_lock);
- return ret;
- }
+ if (ret < 0)
+ goto unlock;
do {
mdelay(20);
ret = i2c_smbus_read_byte_data(chip->client, AD7152_REG_CFG);
- if (ret < 0) {
- mutex_unlock(&chip->state_lock);
- return ret;
- }
+ if (ret < 0)
+ goto unlock;
+
} while ((ret == regval) && timeout--);
mutex_unlock(&chip->state_lock);
return len;
+
+unlock:
+ mutex_unlock(&chip->state_lock);
+ return ret;
}
static ssize_t ad7152_start_offset_calib(struct device *dev,
--
2.15.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: iio: ad7746: Improve unlocking of a mutex in ad7746_start_calib()
From: Markus Elfring
Date: Fri, 3 Nov 2017 09:26:28 +0100
* Add a jump target so that a call of the function "mutex_unlock" is stored
only twice in this function implementation.
* Replace two calls by goto statements.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/staging/iio/cdc/ad7746.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
index a124853a05f0..c4a864725376 100644
--- a/drivers/staging/iio/cdc/ad7746.c
+++ b/drivers/staging/iio/cdc/ad7746.c
@@ -302,23 +302,24 @@ static inline ssize_t ad7746_start_calib(struct device
*dev,
mutex_lock(&chip->lock);
regval |= chip->config;
ret = i2c_smbus_write_byte_data(chip->client, AD7746_REG_CFG, regval);
- if (ret < 0) {
- mutex_unlock(&chip->lock);
- return ret;
- }
+ if (ret < 0)
+ goto unlock;
do {
msleep(20);
ret = i2c_smbus_read_byte_data(chip->client, AD7746_REG_CFG);
- if (ret < 0) {
- mutex_unlock(&chip->lock);
- return ret;
- }
+ if (ret < 0)
+ goto unlock;
+
} while ((ret == regval) && timeout--);
mutex_unlock(&chip->lock);
return len;
+
+unlock:
+ mutex_unlock(&chip->lock);
+ return ret;
}
static ssize_t ad7746_start_offset_calib(struct device *dev,
--
2.15.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging/media/davinci_vpfe: Use common error handling code in vpfe_attach_irq()
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(&vpfe_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(&vpfe_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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: usbduxfast: Improve unlocking of a mutex in usbduxfast_ai_insn_read()
>> @@ -838,6 +834,10 @@ static int usbduxfast_ai_insn_read(struct comedi_device >> *dev, >> mutex_unlock(&devpriv->mut); >> return insn->n; > > Minor niggle: You could also remove that call to mutex_unlock() by replacing > the above three lines with: > > ret = insn->n; > > which will fall through to the 'unlock:' label below. Thanks for your suggestion. Such a software refactoring is also possible if a corresponding consensus could be achieved. * Can such a change mean that the lock scope will be extended for both use cases (successful and failed function execution)? * How much does this implementation matter for you? * Would you like to achieve a small reduction of the object code there? * How do you think about consequences from special communication settings by a well-known maintainer for my update suggestions? Regards, Markus ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: staging: comedi: usbduxfast: Improve unlocking of a mutex in usbduxfast_ai_insn_read()
> FYI, you are responding to someone who is on my blacklist I am curious if this communication setting will ever be adjusted. > and I never accept patches from. The history shows that our collaboration style changed over time. I got a few update suggestions integrated (also by you) because other contributors found them good enough to repeat them. Regards, Markus ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging/rts5208/rtsx: Improve unlocking of a mutex in rtsx_resume()
From: Markus Elfring
Date: Fri, 3 Nov 2017 20:02:22 +0100
* Add a jump target so that a call of the function "mutex_unlock" is stored
only twice in this function implementation.
* Replace two calls by goto statements.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/staging/rts5208/rtsx.c | 15 +++
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/rts5208/rtsx.c b/drivers/staging/rts5208/rtsx.c
index 89e2cfe7d1cc..14022a76ecfb 100644
--- a/drivers/staging/rts5208/rtsx.c
+++ b/drivers/staging/rts5208/rtsx.c
@@ -349,9 +349,7 @@ static int rtsx_resume(struct pci_dev *pci)
dev_err(&dev->pci->dev,
"%s: pci_enable_device failed, disabling device\n",
CR_DRIVER_NAME);
- /* unlock the device pointers */
- mutex_unlock(&dev->dev_mutex);
- return -EIO;
+ goto unlock;
}
pci_set_master(pci);
@@ -360,11 +358,8 @@ static int rtsx_resume(struct pci_dev *pci)
chip->msi_en = 0;
}
- if (rtsx_acquire_irq(dev) < 0) {
- /* unlock the device pointers */
- mutex_unlock(&dev->dev_mutex);
- return -EIO;
- }
+ if (rtsx_acquire_irq(dev) < 0)
+ goto unlock;
rtsx_write_register(chip, HOST_SLEEP_STATE, 0x03, 0x00);
rtsx_init_chip(chip);
@@ -373,6 +368,10 @@ static int rtsx_resume(struct pci_dev *pci)
mutex_unlock(&dev->dev_mutex);
return 0;
+
+unlock:
+ mutex_unlock(&dev->dev_mutex);
+ return -EIO;
}
#endif /* CONFIG_PM */
--
2.15.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: unisys: visorchipset: Use common error handling code in setup_crash_devices_work_queue()
From: Markus Elfring
Date: Fri, 3 Nov 2017 20:37:03 +0100
* Add a jump target so that a specific error message is stored only once
at the end of this function implementation.
* Replace four calls of the function "dev_err" by goto statements.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/staging/unisys/visorbus/visorchipset.c | 36 --
1 file changed, 16 insertions(+), 20 deletions(-)
diff --git a/drivers/staging/unisys/visorbus/visorchipset.c
b/drivers/staging/unisys/visorbus/visorchipset.c
index fed554a43151..1d54821dd7b6 100644
--- a/drivers/staging/unisys/visorbus/visorchipset.c
+++ b/drivers/staging/unisys/visorbus/visorchipset.c
@@ -1231,11 +1231,9 @@ static void setup_crash_devices_work_queue(struct
work_struct *work)
if (visorchannel_read(chipset_dev->controlvm_channel,
offsetof(struct visor_controlvm_channel,
saved_crash_message_count),
- &local_crash_msg_count, sizeof(u16)) < 0) {
- dev_err(&chipset_dev->acpi_device->dev,
- "failed to read channel\n");
- return;
- }
+ &local_crash_msg_count, sizeof(u16)) < 0)
+ goto report_read_failure;
+
if (local_crash_msg_count != CONTROLVM_CRASHMSG_MAX) {
dev_err(&chipset_dev->acpi_device->dev, "invalid count\n");
return;
@@ -1244,30 +1242,24 @@ static void setup_crash_devices_work_queue(struct
work_struct *work)
if (visorchannel_read(chipset_dev->controlvm_channel,
offsetof(struct visor_controlvm_channel,
saved_crash_message_offset),
- &local_crash_msg_offset, sizeof(u32)) < 0) {
- dev_err(&chipset_dev->acpi_device->dev,
- "failed to read channel\n");
- return;
- }
+ &local_crash_msg_offset, sizeof(u32)) < 0)
+ goto report_read_failure;
+
/* read create device message for storage bus offset */
if (visorchannel_read(chipset_dev->controlvm_channel,
local_crash_msg_offset,
&local_crash_bus_msg,
- sizeof(struct controlvm_message)) < 0) {
- dev_err(&chipset_dev->acpi_device->dev,
- "failed to read channel\n");
- return;
- }
+ sizeof(struct controlvm_message)) < 0)
+ goto report_read_failure;
+
/* read create device message for storage device */
if (visorchannel_read(chipset_dev->controlvm_channel,
local_crash_msg_offset +
sizeof(struct controlvm_message),
&local_crash_dev_msg,
- sizeof(struct controlvm_message)) < 0) {
- dev_err(&chipset_dev->acpi_device->dev,
- "failed to read channel\n");
- return;
- }
+ sizeof(struct controlvm_message)) < 0)
+ goto report_read_failure;
+
/* reuse IOVM create bus message */
if (!local_crash_bus_msg.cmd.create_bus.channel_addr) {
dev_err(&chipset_dev->acpi_device->dev,
@@ -1282,6 +1274,10 @@ static void setup_crash_devices_work_queue(struct
work_struct *work)
return;
}
visorbus_device_create(&local_crash_dev_msg);
+ return;
+
+report_read_failure:
+ dev_err(&chipset_dev->acpi_device->dev, "failed to read channel\n");
}
void visorbus_response(struct visor_device *bus_info, int response,
--
2.15.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/4] staging/wlan-ng/prism2fw: Adjustments for two function implementations
From: Markus Elfring Date: Wed, 13 Dec 2017 14:03:02 +0100 A few update suggestions were taken into account from static source code analysis. Markus Elfring (4): Delete an error message for a failed memory allocation in mkimage() Use a known error code after a failed kzalloc() in mkimage() Delete an error message for a failed memory allocation in writeimage() Use common error handling code in writeimage() drivers/staging/wlan-ng/prism2fw.c | 15 +-- 1 file changed, 5 insertions(+), 10 deletions(-) -- 2.15.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/4] staging/wlan-ng/prism2fw: Delete an error message for a failed memory allocation in mkimage()
From: Markus Elfring
Date: Wed, 13 Dec 2017 12:57:13 +0100
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/staging/wlan-ng/prism2fw.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/wlan-ng/prism2fw.c
b/drivers/staging/wlan-ng/prism2fw.c
index 344bec8cc31b..6888d1b093fa 100644
--- a/drivers/staging/wlan-ng/prism2fw.c
+++ b/drivers/staging/wlan-ng/prism2fw.c
@@ -557,10 +557,9 @@ static int mkimage(struct imgchunk *clist, unsigned int
*ccnt)
/* Allocate buffer space for chunks */
for (i = 0; i < *ccnt; i++) {
clist[i].data = kzalloc(clist[i].len, GFP_KERNEL);
- if (!clist[i].data) {
- pr_err("failed to allocate image space, exitting.\n");
+ if (!clist[i].data)
return 1;
- }
+
pr_debug("chunk[%d]: addr=0x%06x len=%d\n",
i, clist[i].addr, clist[i].len);
}
--
2.15.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/4] staging/wlan-ng/prism2fw: Use a known error code after a failed kzalloc() in mkimage()
From: Markus Elfring
Date: Wed, 13 Dec 2017 13:20:10 +0100
Make a memory allocation failure clearer by using the error code "-ENOMEM"
(instead of the constant "1") in this function.
Signed-off-by: Markus Elfring
---
drivers/staging/wlan-ng/prism2fw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/wlan-ng/prism2fw.c
b/drivers/staging/wlan-ng/prism2fw.c
index 6888d1b093fa..efca9108dad8 100644
--- a/drivers/staging/wlan-ng/prism2fw.c
+++ b/drivers/staging/wlan-ng/prism2fw.c
@@ -558,7 +558,7 @@ static int mkimage(struct imgchunk *clist, unsigned int
*ccnt)
for (i = 0; i < *ccnt; i++) {
clist[i].data = kzalloc(clist[i].len, GFP_KERNEL);
if (!clist[i].data)
- return 1;
+ return -ENOMEM;
pr_debug("chunk[%d]: addr=0x%06x len=%d\n",
i, clist[i].addr, clist[i].len);
--
2.15.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/4] staging/wlan-ng/prism2fw: Delete an error message for a failed memory allocation in writeimage()
From: Markus Elfring
Date: Wed, 13 Dec 2017 13:28:15 +0100
Omit an extra message for a memory allocation failure in this function.
Signed-off-by: Markus Elfring
---
drivers/staging/wlan-ng/prism2fw.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/staging/wlan-ng/prism2fw.c
b/drivers/staging/wlan-ng/prism2fw.c
index efca9108dad8..e6f7ed17106a 100644
--- a/drivers/staging/wlan-ng/prism2fw.c
+++ b/drivers/staging/wlan-ng/prism2fw.c
@@ -1014,9 +1014,6 @@ static int writeimage(struct wlandevice *wlandev, struct
imgchunk *fchunk,
if (!rstmsg || !rwrmsg) {
kfree(rstmsg);
kfree(rwrmsg);
- netdev_err(wlandev->netdev,
- "%s: no memory for firmware download, aborting
download\n",
- __func__);
return -ENOMEM;
}
--
2.15.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/4] staging/wlan-ng/prism2fw: Use common error handling code in writeimage()
From: Markus Elfring
Date: Wed, 13 Dec 2017 13:41:41 +0100
Replace two calls of the function "kfree" by a jump to the same statements
at the end of this function so that the generated object code could become
a bit smaller.
Signed-off-by: Markus Elfring
---
drivers/staging/wlan-ng/prism2fw.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/wlan-ng/prism2fw.c
b/drivers/staging/wlan-ng/prism2fw.c
index e6f7ed17106a..42ad5ff7772e 100644
--- a/drivers/staging/wlan-ng/prism2fw.c
+++ b/drivers/staging/wlan-ng/prism2fw.c
@@ -1012,9 +1012,8 @@ static int writeimage(struct wlandevice *wlandev, struct
imgchunk *fchunk,
rstmsg = kzalloc(sizeof(*rstmsg), GFP_KERNEL);
rwrmsg = kzalloc(sizeof(*rwrmsg), GFP_KERNEL);
if (!rstmsg || !rwrmsg) {
- kfree(rstmsg);
- kfree(rwrmsg);
- return -ENOMEM;
+ result = -ENOMEM;
+ goto free_result;
}
/* Initialize the messages */
--
2.15.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: vt6656: Delete an error message for a failed memory allocation in vnt_alloc_bufs()
From: Markus Elfring
Date: Wed, 13 Dec 2017 15:15:45 +0100
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/staging/vt6656/main_usb.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/staging/vt6656/main_usb.c
b/drivers/staging/vt6656/main_usb.c
index cc6d8778fe5b..554b21e40298 100644
--- a/drivers/staging/vt6656/main_usb.c
+++ b/drivers/staging/vt6656/main_usb.c
@@ -437,11 +437,8 @@ static bool vnt_alloc_bufs(struct vnt_private *priv)
for (ii = 0; ii < priv->num_rcb; ii++) {
priv->rcb[ii] = kzalloc(sizeof(*priv->rcb[ii]), GFP_KERNEL);
- if (!priv->rcb[ii]) {
- dev_err(&priv->usb->dev,
- "failed to allocate rcb no %d\n", ii);
+ if (!priv->rcb[ii])
goto free_rx_tx;
- }
rcb = priv->rcb[ii];
--
2.15.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging/rtl8192u/ieee80211: Delete an error message for a failed memory allocation in ieee80211_networks_allocate()
From: Markus Elfring
Date: Wed, 13 Dec 2017 15:46:07 +0100
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/staging/rtl8192u/ieee80211/ieee80211_module.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
b/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
index 90a097f2cd4e..f900dee19a7f 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
@@ -67,11 +67,8 @@ static inline int ieee80211_networks_allocate(struct
ieee80211_device *ieee)
ieee->networks = kcalloc(
MAX_NETWORK_COUNT, sizeof(struct ieee80211_network),
GFP_KERNEL);
- if (!ieee->networks) {
- printk(KERN_WARNING "%s: Out of memory allocating beacons\n",
- ieee->dev->name);
+ if (!ieee->networks)
return -ENOMEM;
- }
return 0;
}
--
2.15.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: ks7010: Adjust dev_err() calls in ks7010_sdio_probe()
From: Markus Elfring
Date: Tue, 11 Apr 2017 14:54:35 +0200
The use of the logging function "dev_err" was introduced here on 2016-09-26.
I find the following implementation details worth for another look.
* Reduce expressions for the first input parameter "dev".
* Omit an extra module prefix in passed error messages because the device name
will be displayed by the function "__dev_printk".
Fixes: 9887b5e51fafaf919601ccb8bdae1e0ad749032f ("staging: ks7010: Fix warnings
on printk() usage")
Signed-off-by: Markus Elfring
---
drivers/staging/ks7010/ks7010_sdio.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/ks7010/ks7010_sdio.c
b/drivers/staging/ks7010/ks7010_sdio.c
index b16618b41253..85feb170869b 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -955,12 +955,11 @@ static int ks7010_sdio_probe(struct sdio_func *func,
/* private memory allocate */
netdev = alloc_etherdev(sizeof(*priv));
if (!netdev) {
- dev_err(&card->func->dev, "ks7010 : Unable to alloc new net
device\n");
+ dev_err(&func->dev, "Unable to alloc new net device\n");
goto err_release_irq;
}
if (dev_alloc_name(netdev, "wlan%d") < 0) {
- dev_err(&card->func->dev,
- "ks7010 : Couldn't get name!\n");
+ dev_err(&func->dev, "Couldn't get name!\n");
goto err_free_netdev;
}
@@ -1000,9 +999,9 @@ static int ks7010_sdio_probe(struct sdio_func *func,
/* Upload firmware */
ret = ks7010_upload_firmware(priv, card); /* firmware load */
if (ret) {
- dev_err(&card->func->dev,
- "ks7010: firmware load failed !! return code = %d\n",
-ret);
+ dev_err(&func->dev,
+ "firmware load failed! return code = %d\n",
+ ret);
goto err_free_read_buf;
}
--
2.12.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: staging: ks7010: Adjust dev_err() calls in ks7010_sdio_probe()
> This patch does not apply to Greg's staging-testing branch. Could the proposed changes be applied with a bit of “fuzz” for the implementation of the function “ks7010_sdio_probe”? > Markus a patch was merged the same day you submitted this one that > refactored this code. Do you refer to your contributions from 2017-04-10 here which seem to touch only other functions? https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/log/?h=staging-testing&qt=author&q=Tobin+C.+Harding > The patch was good though, good spotting. Will there any more approaches be needed for a corresponding editing conflict resolution? Which day would be better to dare a resend for my small update suggestion? Regards, Markus ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] android: binder: Use seq_putc() in print_binder_node()
From: Markus Elfring Date: Sun, 7 May 2017 22:07:16 +0200 A single character (line break) should be put into a sequence. Thus use the corresponding function "seq_putc". This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- drivers/android/binder.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/android/binder.c b/drivers/android/binder.c index aae4d8d4be36..041b49b0bbd9 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -3871,7 +3871,7 @@ static void print_binder_node(struct seq_file *m, struct binder_node *node) hlist_for_each_entry(ref, &node->refs, node_entry) seq_printf(m, " %d", ref->proc->pid); } - seq_puts(m, "\n"); + seq_putc(m, '\n'); list_for_each_entry(w, &node->async_todo, entry) print_binder_work(m, "", "pending async transaction", w); -- 2.12.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/4] VMBus: Adjustments for some function implementations
From: Markus Elfring Date: Thu, 11 May 2017 18:00:18 +0200 A few update suggestions were taken into account from static source code analysis. Markus Elfring (4): Improve a size determination in vmbus_device_create() Delete an error message for a failed memory allocation in vmbus_device_create() Fix a typo in a comment line Adjust five checks for null pointers drivers/hv/vmbus_drv.c | 18 -- 1 file changed, 8 insertions(+), 10 deletions(-) -- 2.12.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/4] vmbus: Improve a size determination in vmbus_device_create()
From: Markus Elfring
Date: Thu, 11 May 2017 17:30:10 +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.
Signed-off-by: Markus Elfring
---
drivers/hv/vmbus_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 0087b49095eb..6802d74f162c 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1145,5 +1145,5 @@ struct hv_device *vmbus_device_create(const uuid_le *type,
{
struct hv_device *child_device_obj;
- child_device_obj = kzalloc(sizeof(struct hv_device), GFP_KERNEL);
+ child_device_obj = kzalloc(sizeof(*child_device_obj), GFP_KERNEL);
if (!child_device_obj) {
--
2.12.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/4] vmbus: Delete an error message for a failed memory allocation in vmbus_device_create()
From: Markus Elfring
Date: Thu, 11 May 2017 17:33:14 +0200
Omit an extra message for a memory allocation failure in this function.
This issue was detected by using the Coccinelle software.
Link:
http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
Signed-off-by: Markus Elfring
---
drivers/hv/vmbus_drv.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 6802d74f162c..96328aebae5a 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1149,7 +1149,5 @@ struct hv_device *vmbus_device_create(const uuid_le *type,
- if (!child_device_obj) {
- pr_err("Unable to allocate device object for child device\n");
+ if (!child_device_obj)
return NULL;
- }
child_device_obj->channel = channel;
memcpy(&child_device_obj->dev_type, type, sizeof(uuid_le));
--
2.12.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/4] vmbus: Fix a typo in a comment line
From: Markus Elfring Date: Thu, 11 May 2017 17:43:55 +0200 Add a missing character in this description. Signed-off-by: Markus Elfring --- drivers/hv/vmbus_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 96328aebae5a..ff94b111ed8d 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -991,7 +991,7 @@ static void vmbus_isr(void) /* * Our host is win8 or above. The signaling mechanism * has changed and we can directly look at the event page. -* If bit n is set then we have an interrup on the channel +* If bit n is set then we have an interrupt on the channel * whose id is n. */ handled = true; -- 2.12.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/4] vmbus: Adjust five checks for null pointers
From: Markus Elfring
Date: Thu, 11 May 2017 17:52:38 +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/hv/vmbus_drv.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index ff94b111ed8d..b55b979ecf8a 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -93,7 +93,7 @@ static DEFINE_SEMAPHORE(hyperv_mmio_lock);
static int vmbus_exists(void)
{
- if (hv_acpi_dev == NULL)
+ if (!hv_acpi_dev)
return -ENODEV;
return 0;
@@ -568,7 +568,7 @@ static const struct hv_vmbus_device_id
*hv_vmbus_get_id(struct hv_driver *drv,
return id;
id = drv->id_table;
- if (id == NULL)
+ if (!id)
return NULL; /* empty device table */
for (; !is_null_guid(&id->guid); id++)
@@ -871,7 +871,7 @@ void vmbus_on_msg_dpc(unsigned long data)
entry = &channel_message_table[hdr->msgtype];
if (entry->handler_type == VMHT_BLOCKING) {
ctx = kmalloc(sizeof(*ctx), GFP_ATOMIC);
- if (ctx == NULL)
+ if (!ctx)
return;
INIT_WORK(&ctx->work, vmbus_onmessage_work);
@@ -894,7 +894,7 @@ static void vmbus_channel_isr(struct vmbus_channel *channel)
void (*callback_fn)(void *);
callback_fn = READ_ONCE(channel->onchannel_callback);
- if (likely(callback_fn != NULL))
+ if (likely(callback_fn))
(*callback_fn)(channel->channel_callback_context);
}
@@ -970,7 +970,7 @@ static void vmbus_isr(void)
union hv_synic_event_flags *event;
bool handled = false;
- if (unlikely(page_addr == NULL))
+ if (unlikely(!page_addr))
return;
event = (union hv_synic_event_flags *)page_addr +
--
2.12.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: vmbus: Delete an error message for a failed memory allocation in vmbus_device_create()
> Taking out the message assumes that all callers of this function either log an > error or pass appropriate error code back to userspace. Do you like the default error response by Linux memory allocation functions? Regards, Markus ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: vmbus: Delete an error message for a failed memory allocation in vmbus_device_create()
>> Just because an automated tool says that this needs to change does not >> mean it has to. > > Checkpatch.pl is correct here. This message is useless. It's during > init so it's unlikely to fail ever. In current kernels small kmallocs > are quaranteed to succeed so it can't actually fail currently. The > stack trace is more useful than this message because it tells you a lot > about what memory is free and the whole call tree. > > The error message is dead useless code. Would you like to clarify corresponding software evolution any more? Is there a need for better documentation of the involved programming interfaces? > This patch is not going to be merged because Markus doesn't listen to > feedback and he's blocked but otherwise it's an OK patch. Does this information contain a contradiction? Will patches be picked up also from contributors who got a special development reputation anyhow? Regards, Markus ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Clarification for general change acceptance
> Developer reputation matters for somewhat controversial > patches being applied as well as non-controversial and > obviously correct patches being ignored. I am aware that there are more factors involved. > Your reputation means most all of your patches fall into > the latter category. I hope that this situation will evolve into directions which you would prefer more. > You have produced many trivial patches This is true. I started my concrete contributions to Linux software modules with simple source code search patterns. > that have caused new defects. A few unwanted programming mistakes just happened somehow. > That is simply unacceptable. Glitches are not desired as usual. > Especially when you don't immediately fix the problems you cause. I find my response times reasonable to some degree so far. Remaining open issues can be clarified by a corresponding constructive development dialogue, can't they? > If you would stop producing the trivial and instead > channel your efforts into actual bug fixing and logic > corrections and not just style modifications with no > code impact, your patch acceptance rate would increase. I find your conclusion appropriate. But I will come along source code places where I am going to update details which are also trivial. > I have given you many suggestions for actual structural > improvements to kernel code. I have got an other impression. There were a few occasions where advanced change possibilities were proposed. > You have ignored _all_ of them and I am unlikely to try > to interact with you any longer until your wheat:chaff > ratio changes. Can the efforts for deleting questionable error messages around Linux memory allocation functions improve this situation? Regards, Markus ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Clarification for general change acceptance
> When you make a patch, you are not obliged to eliminate all of the other > checkpatch warnings on the file. Your view is generally fine. > I don't know where you got this idea from. I got used as a professional software developer to some approaches for reducing development warnings to some degree. So I picked further update suggestions up also from this source code analysis tool. Regards, Markus ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: netlogic: Delete an error message for a failed memory allocation in xlr_config_spill()
From: Markus Elfring
Date: Wed, 17 May 2017 19:01:10 +0200
Omit an extra message for a memory allocation failure in this function.
This issue was detected by using the Coccinelle software.
Link:
http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
Signed-off-by: Markus Elfring
---
drivers/staging/netlogic/xlr_net.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/staging/netlogic/xlr_net.c
b/drivers/staging/netlogic/xlr_net.c
index 781ef623233e..864304eb5f36 100644
--- a/drivers/staging/netlogic/xlr_net.c
+++ b/drivers/staging/netlogic/xlr_net.c
@@ -420,10 +420,8 @@ static void *xlr_config_spill(struct xlr_net_priv *priv,
int reg_start_0,
base = priv->base_addr;
spill_size = size;
spill = kmalloc(spill_size + SMP_CACHE_BYTES, GFP_ATOMIC);
- if (!spill) {
- pr_err("Unable to allocate memory for spill area!\n");
+ if (!spill)
return ZERO_SIZE_PTR;
- }
spill = PTR_ALIGN(spill, SMP_CACHE_BYTES);
phys_addr = virt_to_phys(spill);
--
2.13.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: wilc1000: Delete an error message for a failed memory allocation in Handle_Key()
From: Markus Elfring
Date: Wed, 17 May 2017 22:26:07 +0200
Omit an extra message for a memory allocation failure in this function.
This issue was detected by using the Coccinelle software.
Link:
http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
Signed-off-by: Markus Elfring
---
drivers/staging/wilc1000/host_interface.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/staging/wilc1000/host_interface.c
b/drivers/staging/wilc1000/host_interface.c
index c3a8af081880..17a31bbd3665 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -1688,7 +1688,6 @@ static int Handle_Key(struct wilc_vif *vif,
} else if (pstrHostIFkeyAttr->action & ADDKEY) {
pu8keybuf = kmalloc(PTK_KEY_MSG_LEN, GFP_KERNEL);
if (!pu8keybuf) {
- netdev_err(vif->ndev, "No buffer send PTK\n");
ret = -ENOMEM;
goto _WPAPtk_end_case_;
}
--
2.13.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging-EHCA: Delete unnecessary checks before the function call "kmem_cache_destroy"
From: Markus Elfring
Date: Sun, 15 Nov 2015 21:58:45 +0100
The kmem_cache_destroy() function tests whether its argument is NULL
and then returns immediately. Thus the test around the calls is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/staging/rdma/ehca/ehca_av.c | 3 +--
drivers/staging/rdma/ehca/ehca_cq.c | 3 +--
drivers/staging/rdma/ehca/ehca_main.c | 3 +--
drivers/staging/rdma/ehca/ehca_mrmw.c | 6 ++
drivers/staging/rdma/ehca/ehca_pd.c | 3 +--
drivers/staging/rdma/ehca/ehca_qp.c | 3 +--
6 files changed, 7 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/rdma/ehca/ehca_av.c
b/drivers/staging/rdma/ehca/ehca_av.c
index 4659263..04b9398 100644
--- a/drivers/staging/rdma/ehca/ehca_av.c
+++ b/drivers/staging/rdma/ehca/ehca_av.c
@@ -272,6 +272,5 @@ int ehca_init_av_cache(void)
void ehca_cleanup_av_cache(void)
{
- if (av_cache)
- kmem_cache_destroy(av_cache);
+ kmem_cache_destroy(av_cache);
}
diff --git a/drivers/staging/rdma/ehca/ehca_cq.c
b/drivers/staging/rdma/ehca/ehca_cq.c
index ea1b5c1..1aa7931 100644
--- a/drivers/staging/rdma/ehca/ehca_cq.c
+++ b/drivers/staging/rdma/ehca/ehca_cq.c
@@ -393,6 +393,5 @@ int ehca_init_cq_cache(void)
void ehca_cleanup_cq_cache(void)
{
- if (cq_cache)
- kmem_cache_destroy(cq_cache);
+ kmem_cache_destroy(cq_cache);
}
diff --git a/drivers/staging/rdma/ehca/ehca_main.c
b/drivers/staging/rdma/ehca/ehca_main.c
index 8246418..860b974 100644
--- a/drivers/staging/rdma/ehca/ehca_main.c
+++ b/drivers/staging/rdma/ehca/ehca_main.c
@@ -245,8 +245,7 @@ static void ehca_destroy_slab_caches(void)
ehca_cleanup_cq_cache();
ehca_cleanup_pd_cache();
#ifdef CONFIG_PPC_64K_PAGES
- if (ctblk_cache)
- kmem_cache_destroy(ctblk_cache);
+ kmem_cache_destroy(ctblk_cache);
#endif
}
diff --git a/drivers/staging/rdma/ehca/ehca_mrmw.c
b/drivers/staging/rdma/ehca/ehca_mrmw.c
index f914b30..553e883 100644
--- a/drivers/staging/rdma/ehca/ehca_mrmw.c
+++ b/drivers/staging/rdma/ehca/ehca_mrmw.c
@@ -2251,10 +2251,8 @@ int ehca_init_mrmw_cache(void)
void ehca_cleanup_mrmw_cache(void)
{
- if (mr_cache)
- kmem_cache_destroy(mr_cache);
- if (mw_cache)
- kmem_cache_destroy(mw_cache);
+ kmem_cache_destroy(mr_cache);
+ kmem_cache_destroy(mw_cache);
}
static inline int ehca_init_top_bmap(struct ehca_top_bmap *ehca_top_bmap,
diff --git a/drivers/staging/rdma/ehca/ehca_pd.c
b/drivers/staging/rdma/ehca/ehca_pd.c
index 351577a..2a8aae4 100644
--- a/drivers/staging/rdma/ehca/ehca_pd.c
+++ b/drivers/staging/rdma/ehca/ehca_pd.c
@@ -119,6 +119,5 @@ int ehca_init_pd_cache(void)
void ehca_cleanup_pd_cache(void)
{
- if (pd_cache)
- kmem_cache_destroy(pd_cache);
+ kmem_cache_destroy(pd_cache);
}
diff --git a/drivers/staging/rdma/ehca/ehca_qp.c
b/drivers/staging/rdma/ehca/ehca_qp.c
index 2e89356..896c01f 100644
--- a/drivers/staging/rdma/ehca/ehca_qp.c
+++ b/drivers/staging/rdma/ehca/ehca_qp.c
@@ -2252,6 +2252,5 @@ int ehca_init_qp_cache(void)
void ehca_cleanup_qp_cache(void)
{
- if (qp_cache)
- kmem_cache_destroy(qp_cache);
+ kmem_cache_destroy(qp_cache);
}
--
2.6.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/3] staging: lustre: Less function calls in class_register_type() after error detection
Am 25.11.2015 um 17:39 schrieb Greg Kroah-Hartman: > On Thu, Nov 05, 2015 at 01:20:33PM +0100, SF Markus Elfring wrote: >> From: Markus Elfring >> Date: Thu, 5 Nov 2015 12:48:58 +0100 >> >> The functions "kfree" and "kobject_put" were called in a few cases by the >> function "class_register_type" during error handling even if the passed >> variable contained a null pointer. >> >> This implementation detail could be improved by the adjustment of >> jump targets. >> >> Signed-off-by: Markus Elfring >> --- >> drivers/staging/lustre/lustre/obdclass/genops.c | 26 >> +++-- >> 1 file changed, 16 insertions(+), 10 deletions(-) > > Does not apply to my staging-next branch :( I get also a result like the following together with the software "Linux next-20151126". ;-) elfring@Sonne:~/Projekte/Linux/next-patched> LANG=C git apply ~/Projekte/Bau/Linux/scripts/Coccinelle/deletions1/next/20151102/Flicken/0003-staging-lustre-Less-function-calls-in-class_register.patch error: patch failed: drivers/staging/lustre/lustre/obdclass/genops.c:214 error: drivers/staging/lustre/lustre/obdclass/genops.c: patch does not apply Do you try this update suggestion out without integrating the corresponding previous update suggestion "Delete unnecessary checks before two function calls" where I proposed to remove extra checks before a few calls of the function "kobject_put" (which seems to matter for the patch hunk in the shown error message)? https://lkml.org/lkml/2015/11/5/276 https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1013635.html Would you like to reject the first update step from this patch series so that I need to adapt my approach to your software design decision? Regards, Markus ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/3] staging: lustre: Less function calls in class_register_type() after error detection
>> Do you try this update suggestion out without integrating the corresponding >> previous >> update suggestion "Delete unnecessary checks before two function calls" >> where I proposed to remove extra checks before a few calls of the function >> "kobject_put" >> (which seems to matter for the patch hunk in the shown error message)? >> https://lkml.org/lkml/2015/11/5/276 >> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1013635.html > > I guess so, I don't remember, I don't see any patches from you earlier > in my "todo" mbox. I am still waiting for further constructive feedback on a bunch of my update suggestions which are derived from static source code analysis. It can be the usual challenge to get a bit more attention for them. Other software improvements will result in bigger effects than the source code fine-tuning I propose, won't it? I would like to acknowledge that changes like the following from this patch series can still be applied together for the software "Linux next-20151126". * 0001-staging-lustre-Delete-unnecessary-checks-before-two.patch * 0003-staging-lustre-Less-function-calls-in-class_register.patch >> Would you like to reject the first update step from this patch series >> so that I need to adapt my approach to your software design decision? > > I have no idea what you are talking about. I have no recolection of > previous patches or conversations about your patches. * Dan Carpenter expressed his software design concerns around hidden sanity checks a few times. How do you think about to give the proposed changes another chance? * Positive feedback is occasionally increasing by specific subsystem supporters and maintainers. How will our collaboration evolve further? Do you want that I resend any mails/patches from my "waiting queue"? Regards, Markus ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/7] staging-Lustre: Fine-tuning for some function implementations
From: Markus Elfring Date: Sun, 13 Dec 2015 14:40:14 +0100 Several update suggestions were taken into account from static source code analysis. Markus Elfring (7): Delete unnecessary goto statements in six functions Rename a jump label for ptlrpc_req_finished() calls Rename a jump label for a kfree(key) call Delete an unnecessary variable initialisation in mgc_process_recover_log() Less checks in mgc_process_recover_log() after error detection A few checks less in mgc_process_recover_log() after error detection Rename a jump label for module_put() calls drivers/staging/lustre/lustre/llite/file.c | 26 ++--- drivers/staging/lustre/lustre/llite/lloop.c| 8 +- drivers/staging/lustre/lustre/llite/namei.c| 13 +-- drivers/staging/lustre/lustre/llite/xattr.c| 20 ++-- drivers/staging/lustre/lustre/mdc/mdc_request.c| 124 ++--- drivers/staging/lustre/lustre/mgc/mgc_request.c| 53 - drivers/staging/lustre/lustre/osc/osc_request.c| 52 - drivers/staging/lustre/lustre/ptlrpc/llog_client.c | 22 ++-- 8 files changed, 152 insertions(+), 166 deletions(-) -- 2.6.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/7] staging: lustre: Delete unnecessary goto statements in six functions
From: Markus Elfring Date: Sun, 13 Dec 2015 09:30:47 +0100 Six goto statements referred to a source code position directly behind them. Thus omit such unnecessary jumps. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- drivers/staging/lustre/lustre/llite/namei.c | 1 - drivers/staging/lustre/lustre/mdc/mdc_request.c | 7 --- 2 files changed, 8 deletions(-) diff --git a/drivers/staging/lustre/lustre/llite/namei.c b/drivers/staging/lustre/lustre/llite/namei.c index 64db5e8..2113dd4 100644 --- a/drivers/staging/lustre/lustre/llite/namei.c +++ b/drivers/staging/lustre/lustre/llite/namei.c @@ -554,7 +554,6 @@ static struct dentry *ll_lookup_it(struct inode *parent, struct dentry *dentry, retval = NULL; else retval = dentry; - goto out; out: if (req) ptlrpc_req_finished(req); diff --git a/drivers/staging/lustre/lustre/mdc/mdc_request.c b/drivers/staging/lustre/lustre/mdc/mdc_request.c index 294c050..920b1e9 100644 --- a/drivers/staging/lustre/lustre/mdc/mdc_request.c +++ b/drivers/staging/lustre/lustre/mdc/mdc_request.c @@ -1181,7 +1181,6 @@ static int mdc_ioc_hsm_progress(struct obd_export *exp, ptlrpc_request_set_replen(req); rc = mdc_queue_wait(req); - goto out; out: ptlrpc_req_finished(req); return rc; @@ -1216,7 +1215,6 @@ static int mdc_ioc_hsm_ct_register(struct obd_import *imp, __u32 archives) ptlrpc_request_set_replen(req); rc = mdc_queue_wait(req); - goto out; out: ptlrpc_req_finished(req); return rc; @@ -1282,7 +1280,6 @@ static int mdc_ioc_hsm_ct_unregister(struct obd_import *imp) ptlrpc_request_set_replen(req); rc = mdc_queue_wait(req); - goto out; out: ptlrpc_req_finished(req); return rc; @@ -1362,8 +1359,6 @@ static int mdc_ioc_hsm_state_set(struct obd_export *exp, ptlrpc_request_set_replen(req); rc = mdc_queue_wait(req); - goto out; - out: ptlrpc_req_finished(req); return rc; @@ -1427,8 +1422,6 @@ static int mdc_ioc_hsm_request(struct obd_export *exp, ptlrpc_request_set_replen(req); rc = mdc_queue_wait(req); - goto out; - out: ptlrpc_req_finished(req); return rc; -- 2.6.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/7] staging: lustre: Rename a jump label for ptlrpc_req_finished() calls
From: Markus Elfring
Date: Sun, 13 Dec 2015 10:33:38 +0100
This issue was detected by using the Coccinelle software.
Choose a jump label according to the current Linux coding style convention.
I suggest to improve this implementation detail by the reuse of a script
like the following for the semantic patch language.
@rename_jump_label exists@
identifier work;
type return_type;
@@
return_type work(...)
{
... when any
goto
-out
+finish_request
;
... when any
-out
+finish_request
:
ptlrpc_req_finished(...);
... when any
}
Signed-off-by: Markus Elfring
---
drivers/staging/lustre/lustre/llite/file.c | 26 +--
drivers/staging/lustre/lustre/llite/namei.c| 12 ++---
drivers/staging/lustre/lustre/llite/xattr.c| 20
drivers/staging/lustre/lustre/mdc/mdc_request.c| 54 +++---
drivers/staging/lustre/lustre/osc/osc_request.c| 28 +--
drivers/staging/lustre/lustre/ptlrpc/llog_client.c | 22 -
6 files changed, 81 insertions(+), 81 deletions(-)
diff --git a/drivers/staging/lustre/lustre/llite/file.c
b/drivers/staging/lustre/lustre/llite/file.c
index 31cd6b3..b94df54 100644
--- a/drivers/staging/lustre/lustre/llite/file.c
+++ b/drivers/staging/lustre/lustre/llite/file.c
@@ -427,27 +427,27 @@ static int ll_intent_file_open(struct dentry *dentry,
void *lmm,
*/
if (!it_disposition(itp, DISP_OPEN_OPEN) ||
it_open_error(DISP_OPEN_OPEN, itp))
- goto out;
+ goto finish_request;
ll_release_openhandle(inode, itp);
- goto out;
+ goto finish_request;
}
if (it_disposition(itp, DISP_LOOKUP_NEG)) {
rc = -ENOENT;
- goto out;
+ goto finish_request;
}
if (rc != 0 || it_open_error(DISP_OPEN_OPEN, itp)) {
rc = rc ? rc : it_open_error(DISP_OPEN_OPEN, itp);
CDEBUG(D_VFSTRACE, "lock enqueue: err: %d\n", rc);
- goto out;
+ goto finish_request;
}
rc = ll_prep_inode(&inode, req, NULL, itp);
if (!rc && itp->d.lustre.it_lock_mode)
ll_set_lock_data(sbi->ll_md_exp, inode, itp, NULL);
-out:
+finish_request:
ptlrpc_req_finished(req);
ll_intent_drop_lock(itp);
@@ -2900,13 +2900,13 @@ static int __ll_inode_revalidate(struct dentry *dentry,
__u64 ibits)
oit.it_create_mode &= ~M_CHECK_STALE;
if (rc < 0) {
rc = ll_inode_revalidate_fini(inode, rc);
- goto out;
+ goto finish_request;
}
rc = ll_revalidate_it_finish(req, &oit, inode);
if (rc != 0) {
ll_intent_release(&oit);
- goto out;
+ goto finish_request;
}
/* Unlinked? Unhash dentry, so it is not picked up later by
@@ -2946,7 +2946,7 @@ static int __ll_inode_revalidate(struct dentry *dentry,
__u64 ibits)
rc = ll_prep_inode(&inode, req, NULL, NULL);
}
-out:
+finish_request:
ptlrpc_req_finished(req);
return rc;
}
@@ -3315,25 +3315,25 @@ static int ll_layout_fetch(struct inode *inode, struct
ldlm_lock *lock)
body = req_capsule_server_get(&req->rq_pill, &RMF_MDT_BODY);
if (body == NULL) {
rc = -EPROTO;
- goto out;
+ goto finish_request;
}
lmmsize = body->eadatasize;
if (lmmsize == 0) /* empty layout */ {
rc = 0;
- goto out;
+ goto finish_request;
}
lmm = req_capsule_server_sized_get(&req->rq_pill, &RMF_EADATA, lmmsize);
if (lmm == NULL) {
rc = -EFAULT;
- goto out;
+ goto finish_request;
}
lvbdata = libcfs_kvzalloc(lmmsize, GFP_NOFS);
if (lvbdata == NULL) {
rc = -ENOMEM;
- goto out;
+ goto finish_request;
}
memcpy(lvbdata, lmm, lmmsize);
@@ -3345,7 +3345,7 @@ static int ll_layout_fetch(struct inode *inode, struct
ldlm_lock *lock)
lock->l_lvb_len = lmmsize;
unlock_res_and_lock(lock);
-out:
+finish_request:
ptlrpc_req_finished(req);
return rc;
}
diff --git a/drivers/staging/lustre/lustre/llite/namei.c
b/drivers/staging/lustre/lustre/llite/namei.c
index 2113dd4..7501f70 100644
--- a/drivers/staging/lustre/lustre/llite/namei.c
+++ b/drivers/staging/lustre/lustre/llite/namei.c
@@ -686,7 +686,7 @@ static struct inode *ll_create_node(struct inode *dir,
struct lookup_intent *it)
rc = ll_prep_inode(&inode, request, dir->i_sb, it);
if (rc) {
inode = ERR_PTR(rc);
- goto out;
+ goto finish_request;
}
[PATCH 3/7] staging: lustre: Rename a jump label for a kfree(key) call
From: Markus Elfring
Date: Sun, 13 Dec 2015 10:56:35 +0100
This issue was detected by using the Coccinelle software.
Choose a jump label according to the current Linux coding style convention.
Signed-off-by: Markus Elfring
---
drivers/staging/lustre/lustre/mdc/mdc_request.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/lustre/lustre/mdc/mdc_request.c
b/drivers/staging/lustre/lustre/mdc/mdc_request.c
index 2a76685..2085ba6 100644
--- a/drivers/staging/lustre/lustre/mdc/mdc_request.c
+++ b/drivers/staging/lustre/lustre/mdc/mdc_request.c
@@ -1125,7 +1125,7 @@ static int mdc_ioc_fid2path(struct obd_export *exp,
struct getinfo_fid2path *gf)
if (!fid_is_sane(&gf->gf_fid)) {
rc = -EINVAL;
- goto out;
+ goto free_key;
}
/* Val is struct getinfo_fid2path result plus path */
@@ -1133,20 +1133,19 @@ static int mdc_ioc_fid2path(struct obd_export *exp,
struct getinfo_fid2path *gf)
rc = obd_get_info(NULL, exp, keylen, key, &vallen, gf, NULL);
if (rc != 0 && rc != -EREMOTE)
- goto out;
+ goto free_key;
if (vallen <= sizeof(*gf)) {
rc = -EPROTO;
- goto out;
+ goto free_key;
} else if (vallen > sizeof(*gf) + gf->gf_pathlen) {
rc = -EOVERFLOW;
- goto out;
+ goto free_key;
}
CDEBUG(D_IOCTL, "path get "DFID" from %llu #%d\n%s\n",
PFID(&gf->gf_fid), gf->gf_recno, gf->gf_linkno, gf->gf_path);
-
-out:
+free_key:
kfree(key);
return rc;
}
--
2.6.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/7] staging: lustre: Delete an unnecessary variable initialisation in mgc_process_recover_log()
From: Markus Elfring Date: Sun, 13 Dec 2015 12:00:32 +0100 The variable "mne_swab" will eventually be set to an appropriate value from a call of the ptlrpc_rep_need_swab() function. Thus let us omit the explicit initialisation at the beginning. Signed-off-by: Markus Elfring --- drivers/staging/lustre/lustre/mgc/mgc_request.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/lustre/lustre/mgc/mgc_request.c b/drivers/staging/lustre/lustre/mgc/mgc_request.c index 2c48847..da130f4 100644 --- a/drivers/staging/lustre/lustre/mgc/mgc_request.c +++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c @@ -1293,7 +1293,7 @@ static int mgc_process_recover_log(struct obd_device *obd, struct page **pages; int nrpages; bool eof = true; - bool mne_swab = false; + bool mne_swab; int i; int ealen; int rc; -- 2.6.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/7] staging: lustre: Less checks in mgc_process_recover_log() after error detection
From: Markus Elfring
Date: Sun, 13 Dec 2015 12:21:17 +0100
A few checks would be performed by the mgc_process_recover_log() function
even if it is known already that the passed variable "pages" contained
a null pointer.
* Let us return directly if a call of the kcalloc() function failed.
* Move assignments for the variables "eof" and "req" behind
this memory allocation.
* Delete a sanity check then.
Signed-off-by: Markus Elfring
---
drivers/staging/lustre/lustre/mgc/mgc_request.c | 25 -
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/lustre/lustre/mgc/mgc_request.c
b/drivers/staging/lustre/lustre/mgc/mgc_request.c
index da130f4..f3b4c30 100644
--- a/drivers/staging/lustre/lustre/mgc/mgc_request.c
+++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c
@@ -1285,14 +1285,14 @@ static int mgc_apply_recover_logs(struct obd_device
*mgc,
static int mgc_process_recover_log(struct obd_device *obd,
struct config_llog_data *cld)
{
- struct ptlrpc_request *req = NULL;
+ struct ptlrpc_request *req;
struct config_llog_instance *cfg = &cld->cld_cfg;
struct mgs_config_body *body;
struct mgs_config_res *res;
struct ptlrpc_bulk_desc *desc;
struct page **pages;
int nrpages;
- bool eof = true;
+ bool eof;
bool mne_swab;
int i;
int ealen;
@@ -1309,10 +1309,11 @@ static int mgc_process_recover_log(struct obd_device
*obd,
nrpages = CONFIG_READ_NRPAGES_INIT;
pages = kcalloc(nrpages, sizeof(*pages), GFP_KERNEL);
- if (pages == NULL) {
- rc = -ENOMEM;
- goto out;
- }
+ if (!pages)
+ return -ENOMEM;
+
+ req = NULL;
+ eof = true;
for (i = 0; i < nrpages; i++) {
pages[i] = alloc_page(GFP_KERNEL);
@@ -1432,14 +1433,12 @@ out:
if (rc == 0 && !eof)
goto again;
- if (pages) {
- for (i = 0; i < nrpages; i++) {
- if (pages[i] == NULL)
- break;
- __free_page(pages[i]);
- }
- kfree(pages);
+ for (i = 0; i < nrpages; i++) {
+ if (pages[i] == NULL)
+ break;
+ __free_page(pages[i]);
}
+ kfree(pages);
return rc;
}
--
2.6.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 6/7] staging: lustre: A few checks less in mgc_process_recover_log() after error detection
From: Markus Elfring
Date: Sun, 13 Dec 2015 13:03:58 +0100
A few checks would be performed by the mgc_process_recover_log() function
even if it was determined that a call of the alloc_page() function failed.
* This implementation detail could be improved by adjustments
for jump targets according to the Linux coding style convention.
* Move the assignment for the variable "eof" behind the memory allocation.
* Delete another sanity check then.
* The variable "req" will eventually be set to an appropriate pointer
from a call of the ptlrpc_request_alloc() function.
Thus let us omit the explicit initialisation before.
Signed-off-by: Markus Elfring
---
drivers/staging/lustre/lustre/mgc/mgc_request.c | 32 +++--
1 file changed, 14 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/lustre/lustre/mgc/mgc_request.c
b/drivers/staging/lustre/lustre/mgc/mgc_request.c
index f3b4c30..7048722 100644
--- a/drivers/staging/lustre/lustre/mgc/mgc_request.c
+++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c
@@ -1312,17 +1312,15 @@ static int mgc_process_recover_log(struct obd_device
*obd,
if (!pages)
return -ENOMEM;
- req = NULL;
- eof = true;
-
for (i = 0; i < nrpages; i++) {
pages[i] = alloc_page(GFP_KERNEL);
if (pages[i] == NULL) {
rc = -ENOMEM;
- goto out;
+ goto free_pages;
}
}
+ eof = true;
again:
LASSERT(cld_is_recover(cld));
LASSERT(mutex_is_locked(&cld->cld_lock));
@@ -1330,12 +1328,12 @@ again:
&RQF_MGS_CONFIG_READ);
if (req == NULL) {
rc = -ENOMEM;
- goto out;
+ goto free_pages;
}
rc = ptlrpc_request_pack(req, LUSTRE_MGS_VERSION, MGS_CONFIG_READ);
if (rc)
- goto out;
+ goto finish_request;
/* pack request */
body = req_capsule_client_get(&req->rq_pill, &RMF_MGS_CONFIG_BODY);
@@ -1344,7 +1342,7 @@ again:
if (strlcpy(body->mcb_name, cld->cld_logname, sizeof(body->mcb_name))
>= sizeof(body->mcb_name)) {
rc = -E2BIG;
- goto out;
+ goto finish_request;
}
body->mcb_offset = cfg->cfg_last_idx + 1;
body->mcb_type = cld->cld_type;
@@ -1356,7 +1354,7 @@ again:
MGS_BULK_PORTAL);
if (desc == NULL) {
rc = -ENOMEM;
- goto out;
+ goto finish_request;
}
for (i = 0; i < nrpages; i++)
@@ -1365,12 +1363,12 @@ again:
ptlrpc_request_set_replen(req);
rc = ptlrpc_queue_wait(req);
if (rc)
- goto out;
+ goto finish_request;
res = req_capsule_server_get(&req->rq_pill, &RMF_MGS_CONFIG_RES);
if (res->mcr_size < res->mcr_offset) {
rc = -EINVAL;
- goto out;
+ goto finish_request;
}
/* always update the index even though it might have errors with
@@ -1384,18 +1382,18 @@ again:
ealen = sptlrpc_cli_unwrap_bulk_read(req, req->rq_bulk, 0);
if (ealen < 0) {
rc = ealen;
- goto out;
+ goto finish_request;
}
if (ealen > nrpages << PAGE_CACHE_SHIFT) {
rc = -EINVAL;
- goto out;
+ goto finish_request;
}
if (ealen == 0) { /* no logs transferred */
if (!eof)
rc = -EINVAL;
- goto out;
+ goto finish_request;
}
mne_swab = !!ptlrpc_rep_need_swab(req);
@@ -1425,14 +1423,12 @@ again:
ealen -= PAGE_CACHE_SIZE;
}
-
-out:
- if (req)
- ptlrpc_req_finished(req);
+finish_request:
+ ptlrpc_req_finished(req);
if (rc == 0 && !eof)
goto again;
-
+free_pages:
for (i = 0; i < nrpages; i++) {
if (pages[i] == NULL)
break;
--
2.6.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 7/7] staging: lustre: Rename a jump label for module_put() calls
From: Markus Elfring
Date: Sun, 13 Dec 2015 14:05:57 +0100
This issue was detected by using the Coccinelle software.
Choose a jump label according to the current Linux coding style convention.
I suggest to improve this implementation detail by the reuse of a script
like the following for the semantic patch language.
@rename_jump_label exists@
identifier target != put_module, work;
type return_type;
@@
return_type work(...)
{
... when any
goto
-target
+put_module
;
... when any
-target
+put_module
:
module_put(...);
... when any
}
Signed-off-by: Markus Elfring
---
drivers/staging/lustre/lustre/llite/lloop.c | 8 ++--
drivers/staging/lustre/lustre/mdc/mdc_request.c | 52 -
drivers/staging/lustre/lustre/osc/osc_request.c | 24 ++--
3 files changed, 42 insertions(+), 42 deletions(-)
diff --git a/drivers/staging/lustre/lustre/llite/lloop.c
b/drivers/staging/lustre/lustre/llite/lloop.c
index 420d391..ebeef3b 100644
--- a/drivers/staging/lustre/lustre/llite/lloop.c
+++ b/drivers/staging/lustre/lustre/llite/lloop.c
@@ -484,14 +484,14 @@ static int loop_set_fd(struct lloop_device *lo, struct
file *unused,
error = -EBUSY;
if (lo->lo_state != LLOOP_UNBOUND)
- goto out;
+ goto put_module;
mapping = file->f_mapping;
inode = mapping->host;
error = -EINVAL;
if (!S_ISREG(inode->i_mode) || inode->i_sb->s_magic != LL_SUPER_MAGIC)
- goto out;
+ goto put_module;
if (!(file->f_mode & FMODE_WRITE))
lo_flags |= LO_FLAGS_READ_ONLY;
@@ -500,7 +500,7 @@ static int loop_set_fd(struct lloop_device *lo, struct file
*unused,
if ((loff_t)(sector_t)size != size) {
error = -EFBIG;
- goto out;
+ goto put_module;
}
/* remove all pages in cache so as dirty pages not to be existent. */
@@ -542,7 +542,7 @@ static int loop_set_fd(struct lloop_device *lo, struct file
*unused,
down(&lo->lo_sem);
return 0;
-out:
+put_module:
/* This is safe: open() is still holding a reference. */
module_put(THIS_MODULE);
return error;
diff --git a/drivers/staging/lustre/lustre/mdc/mdc_request.c
b/drivers/staging/lustre/lustre/mdc/mdc_request.c
index 2085ba6..eaeca9a 100644
--- a/drivers/staging/lustre/lustre/mdc/mdc_request.c
+++ b/drivers/staging/lustre/lustre/mdc/mdc_request.c
@@ -1734,7 +1734,7 @@ static int mdc_iocontrol(unsigned int cmd, struct
obd_export *exp, int len,
switch (cmd) {
case OBD_IOC_CHANGELOG_SEND:
rc = mdc_ioc_changelog_send(obd, karg);
- goto out;
+ goto put_module;
case OBD_IOC_CHANGELOG_CLEAR: {
struct ioc_changelog *icc = karg;
struct changelog_setinfo cs = {
@@ -1745,47 +1745,47 @@ static int mdc_iocontrol(unsigned int cmd, struct
obd_export *exp, int len,
rc = obd_set_info_async(NULL, exp, strlen(KEY_CHANGELOG_CLEAR),
KEY_CHANGELOG_CLEAR, sizeof(cs), &cs,
NULL);
- goto out;
+ goto put_module;
}
case OBD_IOC_FID2PATH:
rc = mdc_ioc_fid2path(exp, karg);
- goto out;
+ goto put_module;
case LL_IOC_HSM_CT_START:
rc = mdc_ioc_hsm_ct_start(exp, karg);
/* ignore if it was already registered on this MDS. */
if (rc == -EEXIST)
rc = 0;
- goto out;
+ goto put_module;
case LL_IOC_HSM_PROGRESS:
rc = mdc_ioc_hsm_progress(exp, karg);
- goto out;
+ goto put_module;
case LL_IOC_HSM_STATE_GET:
rc = mdc_ioc_hsm_state_get(exp, karg);
- goto out;
+ goto put_module;
case LL_IOC_HSM_STATE_SET:
rc = mdc_ioc_hsm_state_set(exp, karg);
- goto out;
+ goto put_module;
case LL_IOC_HSM_ACTION:
rc = mdc_ioc_hsm_current_action(exp, karg);
- goto out;
+ goto put_module;
case LL_IOC_HSM_REQUEST:
rc = mdc_ioc_hsm_request(exp, karg);
- goto out;
+ goto put_module;
case OBD_IOC_CLIENT_RECOVER:
rc = ptlrpc_recover_import(imp, data->ioc_inlbuf1, 0);
if (rc < 0)
- goto out;
+ goto put_module;
rc = 0;
- goto out;
+ goto put_module;
case IOC_OSC_SET_ACTIVE:
rc = ptlrpc_set_import_active(imp, data->ioc_offset);
- goto out;
+ goto put_module;
case OBD_IOC_POLL_QUOTACHECK:
rc = mdc_quota_poll_check(exp, (struct if_quotacheck *)karg);
-
Re: [PATCH 2/7] staging: lustre: Rename a jump label for ptlrpc_req_finished() calls
> Markus, please stop sending these things to rename out labels unless > there is a bug. CodingStyle allows out labels. How does this feedback fit to information like the following? "… Chapter 7: … … Choose label names which say what the goto does or why the goto exists. … Avoid using GW-BASIC names … …" Regards, Markus ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: staging: lustre: Rename a jump label for ptlrpc_req_finished() calls
>>> Markus, please stop sending these things to rename out labels unless >>> there is a bug. CodingStyle allows out labels. >> >> How does this feedback fit to information like the following? >> >> "… >> Chapter 7: … >> … >> Choose label names which say what the goto does or why the goto exists. > > A lot of people think "out" says what the goto does and why it exists. I have got the impression that this short identifier is only partly appropriate. > I personally don't agree with them I guess that my opinion goes into a similar direction here. > but if you look at when I complain about it, it's almost always > when it causes a bug. I agree that the combination with bug fixing is more appealing than an attempt to improve coding style applications. >> … Avoid using GW-BASIC names … > > Those when people just use numbers for their label names instead of > words like out1, out2, out4, out5. It's a different thing. The difference is not so clear for me as it appears to you. How many software developers can still remember habits around the selection of such identifiers from GW-BASIC times? Regards, Markus ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/7] staging: lustre: Less checks in mgc_process_recover_log() after error detection
>> A few checks would be performed by the mgc_process_recover_log() function >> even if it is known already that the passed variable "pages" contained >> a null pointer. >> >> * Let us return directly if a call of the kcalloc() function failed. >> >> * Move assignments for the variables "eof" and "req" behind >> this memory allocation. > > Why? The positions of their initialisation depends on the selected exception handling implementation, doesn't it? Can you accept the proposed changes around the affected memory allocations? > Then in the next patch it moves again. This detail is a matter of patch granularity. > It's like cup shuffle to read these patches sometimes. Do you prefer to stash any changes together for a bigger update step? Regards, Markus ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: staging: lustre: Less checks in mgc_process_recover_log() after error detection
>> Can you accept the proposed changes around the affected memory allocations? > > Just leave it as-is if there is no reason. I suggest to make the implementation of the function "mgc_process_recover_log" a bit more efficient. >> Do you prefer to stash any changes together for a bigger update step? > > Yes. Patches 5 and 6 would be easier to review if they were folded into > one patch. I do not like patch squashing for my update suggestions here. Regards, Markus ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: staging: lustre: Less checks in mgc_process_recover_log() after error detection
>> I do not like patch squashing for my update suggestions here. > > I am a maintainer in drivers/staging. Thanks for this information. > I am telling you what you need to do if you want us to apply your patch. I am still waiting for a bit more constructive feedback for this patch series. How many days should I wait before I should send adjusted update suggestions for this approach? > What you do with that information is up to you. Our software development dialogue seems to trigger special challenges between us so far. Are you generally willing to change the exception handling for the memory allocations in the function "mgc_process_recover_log" at all? Regards, Markus ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: staging: lustre: Less checks in mgc_process_recover_log() after error detection
> If you were a lustre dev then I would accept these renames definitely. I find this information interesting. Would any more contributors like to share their opinion? > I do not think I have been unfair to you. This view is correct in principle. > There was no element of surprise. I am trying to discuss further "special" update suggestions where the topic focus might evolve to new directions. I got the impression that you had some difficulties already with my previous proposals. So I am unsure about the general change acceptance from you alone. You pointed out that you are maintainer for this software area. I was not so aware about this detail while I noticed that you are very active Linux software developer. (You are not mentioned in the file "MAINTAINERS" for example.) > Part of the reason we have CodingStyle is so that we can tell people > "That's not in CodingStyle, that's just your own opinion so don't redo > code just because you have a different opinion from the maintainer." I find this description reasonable. But I see some challenges to improve the coding style specification. I would appreciate if some items can become less ambiguous and imprecise. I assume that a few recommendations from the script "checkpatch.pl" should also be mentioned there. > >> Are you generally willing to change the exception handling for >> the memory allocations in the function "mgc_process_recover_log" >> at all? > I like the first patch in this series. Thanks for a bit of positive feedback. > I do not like the renames. I guess that this design aspect can be clarified a bit more. > I don't care too much about patches 5 and 6 except that they should be > folded together and you should not move "req" and "eof" around. I can understand this concern better than your first response for these update steps. I might send an adjusted patch series a few days later. > Mostly I wish you would just focus on fixing bugs instead of these sorts > of patches. How often are deviations from the coding style also just ordinary bugs? It seems that changes for this area are occasionally not so attractive in comparison to software improvements for components which are more popular. Regards, Markus ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: staging: lustre: Delete unnecessary goto statements in six functions
> This is the original code: Really …? > result = baz(); > if (result) > goto label; > > label: > go on... I do not see such a source code structure at the six places I propose to clean-up. > I don't find the test->goto label; label: use offensive, > but if he does, I think keeping a blank line in place of > the test->goto might be better. I find this an interesting view on source code layout. Are there any more opinions around such implementation details? Regards, Markus ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: staging: lustre: Delete unnecessary goto statements in six functions
> rc = mdc_queue_wait(req); > goto out; > out: > ptlrpc_req_finished(req); > return rc; > } > - > > I think if the last goto out; is to be removed, > then it should be replaced by a blank line. > > It separates the last operation block from the return. Would you like to point a very specific coding style issue out? How often should jump labels preceded with blank lines? Regards, Markus ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: staging: lustre: Delete unnecessary goto statements in six functions
> I think there should _not_ be a hardened rule. I guess that it can become hard to achieve consensus on a precise rule. > Style is just a guide. Generally nice … > Do what you think appropriate. I'm sorry for my evolving understanding. - But I imagine that your feedback can cause further software development troubles if the acceptance for this update suggestion will really depend on the number of blank lines before a jump label. Regards, Markus ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging-slicoss: Use a signed return type for slic_card_locate()
From: Markus Elfring
Date: Sat, 19 Dec 2015 20:30:39 +0100
The return type "u32" was used by the slic_card_locate() function
despite of the aspect that it will eventually return a negative
error code.
Improve this implementation detail by using the type "s32" instead.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/staging/slicoss/slicoss.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/slicoss/slicoss.c
b/drivers/staging/slicoss/slicoss.c
index b23a2d1..9ba0e4b 100644
--- a/drivers/staging/slicoss/slicoss.c
+++ b/drivers/staging/slicoss/slicoss.c
@@ -2953,7 +2953,7 @@ static const struct net_device_ops slic_netdev_ops = {
.ndo_change_mtu = eth_change_mtu,
};
-static u32 slic_card_locate(struct adapter *adapter)
+static s32 slic_card_locate(struct adapter *adapter)
{
struct sliccard *card = slic_global.slic_card;
struct physcard *physcard = slic_global.phys_card;
--
2.6.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 0/4] staging-Lustre: Fine-tuning for some function implementations
From: Markus Elfring Date: Mon, 21 Dec 2015 20:00:02 +0100 Several update suggestions were taken into account from static source code analysis. Markus Elfring (4): Delete unnecessary goto statements in six functions Delete an unnecessary variable initialisation in mgc_process_recover_log() Less checks in mgc_process_recover_log() after error detection Fix a jump label position in osc_get_info() drivers/staging/lustre/lustre/llite/namei.c | 1 - drivers/staging/lustre/lustre/mdc/mdc_request.c | 7 drivers/staging/lustre/lustre/mgc/mgc_request.c | 53 +++-- drivers/staging/lustre/lustre/osc/osc_request.c | 2 +- 4 files changed, 25 insertions(+), 38 deletions(-) -- 2.6.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 1/4] staging: lustre: Delete unnecessary goto statements in six functions
From: Markus Elfring Date: Mon, 21 Dec 2015 18:15:45 +0100 Six goto statements referred to a source code position directly behind them. Thus omit such unnecessary jumps. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- drivers/staging/lustre/lustre/llite/namei.c | 1 - drivers/staging/lustre/lustre/mdc/mdc_request.c | 7 --- 2 files changed, 8 deletions(-) diff --git a/drivers/staging/lustre/lustre/llite/namei.c b/drivers/staging/lustre/lustre/llite/namei.c index 64db5e8..2113dd4 100644 --- a/drivers/staging/lustre/lustre/llite/namei.c +++ b/drivers/staging/lustre/lustre/llite/namei.c @@ -554,7 +554,6 @@ static struct dentry *ll_lookup_it(struct inode *parent, struct dentry *dentry, retval = NULL; else retval = dentry; - goto out; out: if (req) ptlrpc_req_finished(req); diff --git a/drivers/staging/lustre/lustre/mdc/mdc_request.c b/drivers/staging/lustre/lustre/mdc/mdc_request.c index 294c050..920b1e9 100644 --- a/drivers/staging/lustre/lustre/mdc/mdc_request.c +++ b/drivers/staging/lustre/lustre/mdc/mdc_request.c @@ -1181,7 +1181,6 @@ static int mdc_ioc_hsm_progress(struct obd_export *exp, ptlrpc_request_set_replen(req); rc = mdc_queue_wait(req); - goto out; out: ptlrpc_req_finished(req); return rc; @@ -1216,7 +1215,6 @@ static int mdc_ioc_hsm_ct_register(struct obd_import *imp, __u32 archives) ptlrpc_request_set_replen(req); rc = mdc_queue_wait(req); - goto out; out: ptlrpc_req_finished(req); return rc; @@ -1282,7 +1280,6 @@ static int mdc_ioc_hsm_ct_unregister(struct obd_import *imp) ptlrpc_request_set_replen(req); rc = mdc_queue_wait(req); - goto out; out: ptlrpc_req_finished(req); return rc; @@ -1362,8 +1359,6 @@ static int mdc_ioc_hsm_state_set(struct obd_export *exp, ptlrpc_request_set_replen(req); rc = mdc_queue_wait(req); - goto out; - out: ptlrpc_req_finished(req); return rc; @@ -1427,8 +1422,6 @@ static int mdc_ioc_hsm_request(struct obd_export *exp, ptlrpc_request_set_replen(req); rc = mdc_queue_wait(req); - goto out; - out: ptlrpc_req_finished(req); return rc; -- 2.6.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 2/4] staging: lustre: Delete an unnecessary variable initialisation in mgc_process_recover_log()
From: Markus Elfring Date: Mon, 21 Dec 2015 18:24:45 +0100 The variable "mne_swab" will eventually be set to an appropriate value from a call of the ptlrpc_rep_need_swab() function. Thus let us omit the explicit initialisation at the beginning. Signed-off-by: Markus Elfring --- drivers/staging/lustre/lustre/mgc/mgc_request.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/lustre/lustre/mgc/mgc_request.c b/drivers/staging/lustre/lustre/mgc/mgc_request.c index 2c48847..da130f4 100644 --- a/drivers/staging/lustre/lustre/mgc/mgc_request.c +++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c @@ -1293,7 +1293,7 @@ static int mgc_process_recover_log(struct obd_device *obd, struct page **pages; int nrpages; bool eof = true; - bool mne_swab = false; + bool mne_swab; int i; int ealen; int rc; -- 2.6.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 3/4] staging: lustre: Less checks in mgc_process_recover_log() after error detection
From: Markus Elfring
Date: Mon, 21 Dec 2015 18:58:51 +0100
A few checks would be performed by the mgc_process_recover_log() function
even though it was determined that the passed variable "pages" contained
a null pointer or a call of the alloc_page() function failed.
1. Let us return directly if a call of the kcalloc() function failed.
2. Corresponding implementation details could be improved by adjustments
for jump targets according to the Linux coding style convention.
3. Delete sanity checks then.
4. Move an assignment for the variable "eof" behind memory allocations.
5. The variable "req" will eventually be set to an appropriate pointer
from a call of the ptlrpc_request_alloc() function.
Thus let us omit the explicit initialisation before.
6. Apply a recommendation from the script "checkpatch.pl".
Signed-off-by: Markus Elfring
---
drivers/staging/lustre/lustre/mgc/mgc_request.c | 51 +++--
1 file changed, 23 insertions(+), 28 deletions(-)
diff --git a/drivers/staging/lustre/lustre/mgc/mgc_request.c
b/drivers/staging/lustre/lustre/mgc/mgc_request.c
index da130f4..5f581df 100644
--- a/drivers/staging/lustre/lustre/mgc/mgc_request.c
+++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c
@@ -1285,14 +1285,14 @@ static int mgc_apply_recover_logs(struct obd_device
*mgc,
static int mgc_process_recover_log(struct obd_device *obd,
struct config_llog_data *cld)
{
- struct ptlrpc_request *req = NULL;
+ struct ptlrpc_request *req;
struct config_llog_instance *cfg = &cld->cld_cfg;
struct mgs_config_body *body;
struct mgs_config_res *res;
struct ptlrpc_bulk_desc *desc;
struct page **pages;
int nrpages;
- bool eof = true;
+ bool eof;
bool mne_swab;
int i;
int ealen;
@@ -1309,19 +1309,18 @@ static int mgc_process_recover_log(struct obd_device
*obd,
nrpages = CONFIG_READ_NRPAGES_INIT;
pages = kcalloc(nrpages, sizeof(*pages), GFP_KERNEL);
- if (pages == NULL) {
- rc = -ENOMEM;
- goto out;
- }
+ if (!pages)
+ return -ENOMEM;
for (i = 0; i < nrpages; i++) {
pages[i] = alloc_page(GFP_KERNEL);
if (pages[i] == NULL) {
rc = -ENOMEM;
- goto out;
+ goto free_pages;
}
}
+ eof = true;
again:
LASSERT(cld_is_recover(cld));
LASSERT(mutex_is_locked(&cld->cld_lock));
@@ -1329,12 +1328,12 @@ again:
&RQF_MGS_CONFIG_READ);
if (req == NULL) {
rc = -ENOMEM;
- goto out;
+ goto free_pages;
}
rc = ptlrpc_request_pack(req, LUSTRE_MGS_VERSION, MGS_CONFIG_READ);
if (rc)
- goto out;
+ goto finish_request;
/* pack request */
body = req_capsule_client_get(&req->rq_pill, &RMF_MGS_CONFIG_BODY);
@@ -1343,7 +1342,7 @@ again:
if (strlcpy(body->mcb_name, cld->cld_logname, sizeof(body->mcb_name))
>= sizeof(body->mcb_name)) {
rc = -E2BIG;
- goto out;
+ goto finish_request;
}
body->mcb_offset = cfg->cfg_last_idx + 1;
body->mcb_type = cld->cld_type;
@@ -1355,7 +1354,7 @@ again:
MGS_BULK_PORTAL);
if (desc == NULL) {
rc = -ENOMEM;
- goto out;
+ goto finish_request;
}
for (i = 0; i < nrpages; i++)
@@ -1364,12 +1363,12 @@ again:
ptlrpc_request_set_replen(req);
rc = ptlrpc_queue_wait(req);
if (rc)
- goto out;
+ goto finish_request;
res = req_capsule_server_get(&req->rq_pill, &RMF_MGS_CONFIG_RES);
if (res->mcr_size < res->mcr_offset) {
rc = -EINVAL;
- goto out;
+ goto finish_request;
}
/* always update the index even though it might have errors with
@@ -1383,18 +1382,18 @@ again:
ealen = sptlrpc_cli_unwrap_bulk_read(req, req->rq_bulk, 0);
if (ealen < 0) {
rc = ealen;
- goto out;
+ goto finish_request;
}
if (ealen > nrpages << PAGE_CACHE_SHIFT) {
rc = -EINVAL;
- goto out;
+ goto finish_request;
}
if (ealen == 0) { /* no logs transferred */
if (!eof)
rc = -EINVAL;
- goto out;
+ goto finish_request;
}
mne_swab = !!ptlrpc_rep_need_swab(req);
@@ -1424,22 +1423,18 @@ again:
ealen -= PAGE_CACHE_SIZE;
}
-
-out:
- if (req)
- ptlrpc_req_finished(req);
+finish_request:
+ ptlrpc_req_finished(req);
if (
[PATCH v2 4/4] staging: lustre: Fix a jump label position in osc_get_info()
From: Markus Elfring
Date: Mon, 21 Dec 2015 19:30:42 +0100
The script "checkpatch.pl" pointed out that labels should not be indented.
Thus delete a horizontal tab before the jump label "out"
in the function "osc_get_info".
Signed-off-by: Markus Elfring
---
drivers/staging/lustre/lustre/osc/osc_request.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/lustre/lustre/osc/osc_request.c
b/drivers/staging/lustre/lustre/osc/osc_request.c
index d6c1447..85ab180 100644
--- a/drivers/staging/lustre/lustre/osc/osc_request.c
+++ b/drivers/staging/lustre/lustre/osc/osc_request.c
@@ -2727,7 +2727,7 @@ static int osc_get_info(const struct lu_env *env, struct
obd_export *exp,
}
*((u64 *)val) = *reply;
- out:
+out:
ptlrpc_req_finished(req);
return rc;
} else if (KEY_IS(KEY_FIEMAP)) {
--
2.6.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 3/4] staging: lustre: Less checks in mgc_process_recover_log() after error detection
>> 6. Apply a recommendation from the script "checkpatch.pl". > > That's 6 different things, shouldn't this be 6 different patches? > > please redo. Dan Carpenter requested to squash the previous update steps 5 and 6 into a single patch for better source code review. Now I see further software development challenges to increase the patch granularity even more as you suggest. Which route would Lustre developers like to follow? Regards, Markus ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging-slicoss: Use a signed return type for slic_card_locate()
From: Markus Elfring
Date: Tue, 22 Dec 2015 15:05:16 +0100
The return type "u32" was used by the slic_card_locate() function
even though it will eventually return a negative error code.
Improve this implementation detail by using the type "int" instead.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/staging/slicoss/slicoss.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/slicoss/slicoss.c
b/drivers/staging/slicoss/slicoss.c
index b23a2d1..156e57f 100644
--- a/drivers/staging/slicoss/slicoss.c
+++ b/drivers/staging/slicoss/slicoss.c
@@ -2953,7 +2953,7 @@ static const struct net_device_ops slic_netdev_ops = {
.ndo_change_mtu = eth_change_mtu,
};
-static u32 slic_card_locate(struct adapter *adapter)
+static int slic_card_locate(struct adapter *adapter)
{
struct sliccard *card = slic_global.slic_card;
struct physcard *physcard = slic_global.phys_card;
--
2.6.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging-slicoss: Replace variable initialisations by assignments in slic_if_init()
From: Markus Elfring
Date: Sun, 3 Jan 2016 17:25:59 +0100
Replace explicit initialisation for two local variables at the beginning
by assignments.
Signed-off-by: Markus Elfring
---
drivers/staging/slicoss/slicoss.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/slicoss/slicoss.c
b/drivers/staging/slicoss/slicoss.c
index b23a2d1..8fdcac8 100644
--- a/drivers/staging/slicoss/slicoss.c
+++ b/drivers/staging/slicoss/slicoss.c
@@ -2301,9 +2301,9 @@ static int slic_adapter_allocresources(struct adapter
*adapter,
*/
static int slic_if_init(struct adapter *adapter, unsigned long *flags)
{
- struct sliccard *card = adapter->card;
+ struct sliccard *card;
struct net_device *dev = adapter->netdev;
- __iomem struct slic_regs *slic_regs = adapter->slic_regs;
+ __iomem struct slic_regs *slic_regs;
struct slic_shmem *pshmem;
int rc;
@@ -2348,6 +2348,7 @@ static int slic_if_init(struct adapter *adapter, unsigned
long *flags)
adapter->queues_initialized = 1;
}
+ slic_regs = adapter->slic_regs;
slic_reg32_write(&slic_regs->slic_icr, ICR_INT_OFF, FLUSH);
mdelay(1);
@@ -2374,6 +2375,7 @@ static int slic_if_init(struct adapter *adapter, unsigned
long *flags)
}
adapter->state = ADAPT_UP;
+ card = adapter->card;
if (!card->loadtimerset) {
setup_timer(&card->loadtimer, &slic_timer_load_check,
(ulong)card);
--
2.6.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging-slicoss: Replace variable initialisations by assignments in slic_if_init()
>> Replace explicit initialisation for two local variables at the beginning >> by assignments. > > Why? I prefer that assignments for variables like "card" and "slic_regs" will only be performed immediately before the corresponding content will be read again (after a few condition checks were executed). Another description could be this view: I suggest to move the variable initialisation a bit. Regards, Markus ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
