From: Chen Gang <gang.chen@asianux.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] kernel/itimer.c: beautify code, not need check 'value', so save one instruction, simpler and easier for readers.t
Date: Tue, 25 Jun 2013 08:58:41 +0800 [thread overview]
Message-ID: <51C8EB41.6060501@asianux.com> (raw)
In-Reply-To: <alpine.DEB.2.02.1306242242480.4013@ionos.tec.linutronix.de>
On 06/25/2013 07:28 AM, Thomas Gleixner wrote:
> On Fri, 21 Jun 2013, Chen Gang wrote:
>>>>> > >> > 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 ? ;-)
> My apologies for being so impolite. Let me rephrase it. Here is a
> "sample" changelog for your patch:
>
It doesn't matter, I really don't (shouldn't) care about it.
Next time, I should try to send patch carefully, so may save the
maintainers' timer resource.
And excuse me for my poor English and either not familiar with kernel, I
am trying to improve them, and keep improving them.
> Subject: itimers: Remove bogus NULL pointer check in sys_getitimer()
>
> People might be tricked into assuming that the return value for a
> failed NULL pointer check should be -EINVAL instead of -EFAULT.
>
> Remove the misleading NULL pointer check to fix this nuisance.
>
> Aside of that this patch fixes the problem of NOMMU kernels, where
> a NULL pointer dereference is a valid operation. This allows to
> boot NOMMU kernels without working around the shortcomings of the
> getitimer() system call, which have been ignored since this NULL
> pointer check was introduced in Linux 0.96a.
>
Really very good comments, at least for me now, I really can not write a
comment like that.
>
> Please resubmit.
I will send patch v4 (patch v3 has sent, and should be obsoleted)
Thanks.
--
Chen Gang
Asianux Corporation
next prev parent reply other threads:[~2013-06-25 0:59 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-06-20 11:26 [PATCH v2] kernel/itimer.c: beautify code, not need check 'value', so save one instruction, simpler and easier for readers Chen Gang
2013-06-20 12:02 ` Chen Gang
2013-06-20 12:55 ` Thomas Gleixner
2013-06-21 1:24 ` Chen Gang
2013-06-20 13:42 ` Thomas Gleixner
2013-06-21 2:04 ` Chen Gang
2013-06-21 10:31 ` [PATCH v3] kernel/itimer.c: remove the checking 'value' statement Chen Gang
2013-06-24 23:28 ` [PATCH v2] kernel/itimer.c: beautify code, not need check 'value', so save one instruction, simpler and easier for readers.t Thomas Gleixner
2013-06-25 0:58 ` Chen Gang [this message]
2013-06-25 1:16 ` [PATH v4] itimers: Remove bogus NULL pointer check in sys_getitimer() Chen Gang
2013-07-05 1:28 ` Chen Gang
2013-07-22 2:45 ` Chen Gang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=51C8EB41.6060501@asianux.com \
--to=gang.chen@asianux.com \
--cc=linux-kernel@vger.kernel.org \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox