Firstly, I guess: since you can spend your time resource to reply, that means "you also think this patch is valuable, but the comments need improving"
Is it correct ? On 06/20/2013 09:42 PM, Thomas Gleixner wrote: > On Thu, 20 Jun 2013, Chen Gang wrote: > > kernel/itimer.c: beautify code, not need check 'value', so > save one instruction, simpler and easier for readers. > > That's an essay and not a proper subject line for a patch. > > See Documentation/SubmittingPatches and look at the other patch > subject lines in git log. > How about "kernel/itimer.c: remove the checking 'value' statement". Or please provide your samples for the subject. >> > Since copy_to_user() will check 'value', we do not need check it outside >> > again, so can save one comparing instruction at least. > copy_to_user() does not check value, it will fault due to a NULL > pointer dereference and execute the exception fixup. > Change it to: Since copy_to_user() will process "bad address" internally, we need not check 'value' again, then can save one comparing instruction at least. > That's a massive difference which wants to be documented and argued > why it's ok to do so. > > Aside of that, please line break the changelog lines around 78 > characters. > I use Thunderbird mail client, enable world wrap, is it OK ? >> > Also can let code simpler and easier for readers: if checking parameter >> > 'value', it will easily lead readers to think about why not return >> > -EINVAL instead of -EFAULT, when checking parameter failed. > So you are seriously claiming, that the check for !value makes people > think that the return value should be -EINVAL? > > That's hillarious. > That seems not a quite polite word, is it ? ;-) > Can you please start to think about, why YOU thought that returning > -EINVAL is the proper return value for that case? > If you check the parameter, and find it invalid, and want to return with failure, every one can assume you want to return -EINVAL. Hmm... in some of embedded system which NOMMU, 'NULL' does not means "bad address" (at least can write). > Simply because in your rush to submit patches according to your self > defined contribution plan, you fail to sit down and carefully study > the code and the according documentation (man page). Instead of that > you see some random snippet of code which looks wrong to you and you > send out patches without care. After someone points out your failure > you claim that the code is misleading to readers. > > The code is not misleading to careful readers, it's only misleading to > sloppy readers. > Do you mean this patch can not make the code simpler and clearer ? I guess, that is not your meaning, so how about this improving: "after remove the code, also can let it simpler and clearer for readers" Is it OK ? > And I'm neither accepting sloppy patches nor am I accepting sloppy > changelogs which make false claims. > That is one of the reason for why we need reviewer, the work flow need be "providing patch --> review --> apply". BTW: Can you guess how many my patches have been applied by upstream, since this year ? That seems most of appliers are very polite, I wish that will include you. ;-) Thanks. -- Chen Gang Asianux Corporation -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
