@@ -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 b
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:
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:
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 adjustme
>> @@ -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(pnet
>> +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
+ 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 speci
> 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
_
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 +-
> 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 com
> 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,
Mar
>> 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) li
> 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
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() a
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/sta
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
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 Elf
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 Elfr
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
---
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
---
driv
>> 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 *he
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_zil
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 mn
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
---
d
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: M
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.
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 n
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 de
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
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 pla
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 ch
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 chan
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 c
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/st
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 d
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
---
driver
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
---
drive
> 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.linuxdrive
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:
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-b
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 soft
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 soft
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: M
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: M
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
>> @@ -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;
>
> wh
> 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
othe
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: M
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 sof
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()
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 insert
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 d
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/dr
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 |
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 inserti
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 fil
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
> 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
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 +-
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_crea
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: Mar
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
Sign
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 96328aebae5
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 pla
> 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 lis
>> 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 curr
> 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
> 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 de
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
Sign
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
Sign
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
--
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 fe
>> 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 pa
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 fo
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/l
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 sem
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 +--
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
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
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 accordi
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 sem
> 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 n
>>> 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 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 "
>> 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 s
>> 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
> 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 tryin
> 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 doe
> 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.
> 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
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
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()
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/l
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
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
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
>> 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 d
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 Cocci
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/drive
>> 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 exe
1 - 100 of 255 matches
Mail list logo