Re: [PATCH 1/1] s390/net: Deletion of unnecessary checks before two function calls

2015-01-19 Thread Brian Norris
I went digging through some of Markus's old patch history, and noticed this... On Mon, Nov 03, 2014 at 12:50:59PM +0300, Dan Carpenter wrote: > This one is buggy. > > I'm sorry, but please stop sending these. I'm tending to concur. > For kfree(), at least we all know that kfree() accepts NULL

Re: [PATCH 1/1] s390/net: Deletion of unnecessary checks before two function calls

2015-01-19 Thread Brian Norris
I went digging through some of Markus's old patch history, and noticed this... On Mon, Nov 03, 2014 at 12:50:59PM +0300, Dan Carpenter wrote: This one is buggy. I'm sorry, but please stop sending these. I'm tending to concur. For kfree(), at least we all know that kfree() accepts NULL

Re: s390/net: Deletion of unnecessary checks before two function calls

2014-11-17 Thread SF Markus Elfring
> What do you want me to clarify? Do you still not see the bug? Would you like to point out that you notice the warning message "fsm: kfree_fsm called with NULL argument" after my update suggestion? How do you think about to share a bit more information about the corresponding function call

Re: s390/net: Deletion of unnecessary checks before two function calls

2014-11-17 Thread SF Markus Elfring
What do you want me to clarify? Do you still not see the bug? Would you like to point out that you notice the warning message fsm: kfree_fsm called with NULL argument after my update suggestion? How do you think about to share a bit more information about the corresponding function call graph

Re: s390/net: Deletion of unnecessary checks before two function calls

2014-11-03 Thread SF Markus Elfring
> If you can benchmark the code and the new code is faster then, yes, this > patch is good and we will apply it. I guess that I do not have enough resources myself to measure different run time effects in a S390 environment. > If you have no benchmarks then do not send the patch. Are other

Re: s390/net: Deletion of unnecessary checks before two function calls

2014-11-03 Thread Dan Carpenter
On Mon, Nov 03, 2014 at 05:50:48PM +0100, SF Markus Elfring wrote: > > After your patch then it will print warning messages. > > To which messages do you refer to? > Open your eyeballs up and read the code. > > > The truth is I think that all these patches are bad and they make the > > code

Re: s390/net: Deletion of unnecessary checks before two function calls

2014-11-03 Thread Julia Lawall
> > After your patch then it will print warning messages. > > After: You have to remember that rtw_free_netdev() accepts NULL > > pointers but free_netdev() does not accept NULL pointers. > > Are any improvements needed for the corresponding documentation to make it > better accessible

Re: s390/net: Deletion of unnecessary checks before two function calls

2014-11-03 Thread SF Markus Elfring
> After your patch then it will print warning messages. To which messages do you refer to? > The truth is I think that all these patches are bad and they make the > code harder to read. > > Before: The code is clear and there is no NULL dereference. Where do you stumble on a null pointer

Re: s390/net: Deletion of unnecessary checks before two function calls

2014-11-03 Thread Dan Carpenter
On Mon, Nov 03, 2014 at 05:10:35PM +0100, SF Markus Elfring wrote: > > I agree with your proposed debug_unregister() changes, but not with your > > kfree_fsm() change. > > Why do you want to keep an additional null pointer check before the call > of the kfree_fsm() function within the

Re: s390/net: Deletion of unnecessary checks before two function calls

2014-11-03 Thread Dan Carpenter
On Mon, Nov 03, 2014 at 04:55:12PM +0100, SF Markus Elfring wrote: > > This one is buggy. > > I am still interested to clarify this opinion a bit more. > After your patch then it will print warning messages. The truth is I think that all these patches are bad and they make the code harder to

Re: s390/net: Deletion of unnecessary checks before two function calls

2014-11-03 Thread SF Markus Elfring
> I agree with your proposed debug_unregister() changes, but not with your > kfree_fsm() change. Why do you want to keep an additional null pointer check before the call of the kfree_fsm() function within the implementation of the netiucv_free_netdevice() function? Regards, Markus -- To

Re: s390/net: Deletion of unnecessary checks before two function calls

2014-11-03 Thread SF Markus Elfring
> This one is buggy. I am still interested to clarify this opinion a bit more. > I'm sorry, but please stop sending these. I am going to improve more implementation details in affected source files. > But for this one: > 1) I don't know what the functions do so I have to look at the code. I

Re: [PATCH 1/1] s390/net: Deletion of unnecessary checks before two function calls

2014-11-03 Thread Ursula Braun
I agree with your proposed debug_unregister() changes, but not with your kfree_fsm() change. Regards, Ursula Braun On Fri, 2014-10-31 at 18:40 +0100, SF Markus Elfring wrote: > The functions debug_unregister() and kfree_fsm() test whether their argument > is NULL and then return immediately.

Re: [PATCH 1/1] s390/net: Deletion of unnecessary checks before two function calls

2014-11-03 Thread Dan Carpenter
This one is buggy. I'm sorry, but please stop sending these. For kfree(), at least we all know that kfree() accepts NULL pointer. But for this one: 1) I don't know what the functions do so I have to look at the code. 2) It's in a arch that I don't compile so cscope isn't set up meaning it's

Re: [PATCH 1/1] s390/net: Deletion of unnecessary checks before two function calls

2014-11-03 Thread Dan Carpenter
This one is buggy. I'm sorry, but please stop sending these. For kfree(), at least we all know that kfree() accepts NULL pointer. But for this one: 1) I don't know what the functions do so I have to look at the code. 2) It's in a arch that I don't compile so cscope isn't set up meaning it's

Re: [PATCH 1/1] s390/net: Deletion of unnecessary checks before two function calls

2014-11-03 Thread Ursula Braun
I agree with your proposed debug_unregister() changes, but not with your kfree_fsm() change. Regards, Ursula Braun On Fri, 2014-10-31 at 18:40 +0100, SF Markus Elfring wrote: The functions debug_unregister() and kfree_fsm() test whether their argument is NULL and then return immediately. Thus

Re: s390/net: Deletion of unnecessary checks before two function calls

2014-11-03 Thread SF Markus Elfring
This one is buggy. I am still interested to clarify this opinion a bit more. I'm sorry, but please stop sending these. I am going to improve more implementation details in affected source files. But for this one: 1) I don't know what the functions do so I have to look at the code. I

Re: s390/net: Deletion of unnecessary checks before two function calls

2014-11-03 Thread SF Markus Elfring
I agree with your proposed debug_unregister() changes, but not with your kfree_fsm() change. Why do you want to keep an additional null pointer check before the call of the kfree_fsm() function within the implementation of the netiucv_free_netdevice() function? Regards, Markus -- To

Re: s390/net: Deletion of unnecessary checks before two function calls

2014-11-03 Thread Dan Carpenter
On Mon, Nov 03, 2014 at 04:55:12PM +0100, SF Markus Elfring wrote: This one is buggy. I am still interested to clarify this opinion a bit more. After your patch then it will print warning messages. The truth is I think that all these patches are bad and they make the code harder to read.

Re: s390/net: Deletion of unnecessary checks before two function calls

2014-11-03 Thread Dan Carpenter
On Mon, Nov 03, 2014 at 05:10:35PM +0100, SF Markus Elfring wrote: I agree with your proposed debug_unregister() changes, but not with your kfree_fsm() change. Why do you want to keep an additional null pointer check before the call of the kfree_fsm() function within the implementation of

Re: s390/net: Deletion of unnecessary checks before two function calls

2014-11-03 Thread SF Markus Elfring
After your patch then it will print warning messages. To which messages do you refer to? The truth is I think that all these patches are bad and they make the code harder to read. Before: The code is clear and there is no NULL dereference. Where do you stumble on a null pointer access?

Re: s390/net: Deletion of unnecessary checks before two function calls

2014-11-03 Thread Julia Lawall
After your patch then it will print warning messages. After: You have to remember that rtw_free_netdev() accepts NULL pointers but free_netdev() does not accept NULL pointers. Are any improvements needed for the corresponding documentation to make it better accessible besides the

Re: s390/net: Deletion of unnecessary checks before two function calls

2014-11-03 Thread Dan Carpenter
On Mon, Nov 03, 2014 at 05:50:48PM +0100, SF Markus Elfring wrote: After your patch then it will print warning messages. To which messages do you refer to? Open your eyeballs up and read the code. The truth is I think that all these patches are bad and they make the code harder to

Re: s390/net: Deletion of unnecessary checks before two function calls

2014-11-03 Thread SF Markus Elfring
If you can benchmark the code and the new code is faster then, yes, this patch is good and we will apply it. I guess that I do not have enough resources myself to measure different run time effects in a S390 environment. If you have no benchmarks then do not send the patch. Are other

Re: [PATCH 1/1] s390/net: Deletion of unnecessary checks before two function calls

2014-10-31 Thread SF Markus Elfring
The functions debug_unregister() and kfree_fsm() 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/s390/net/claw.c | 6 ++

Re: [PATCH 1/1] s390/net: Deletion of unnecessary checks before two function calls

2014-10-31 Thread SF Markus Elfring
The functions debug_unregister() and kfree_fsm() 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 elfr...@users.sourceforge.net ---