linux-kernel - Re: [PATCH v2] kernel/itimer.c: beautify code, not need check 'value', so save one instruction, simpler and easier for readers.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 20 Jun 2013 15:42:07 +0200 (CEST)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Chen Gang <gang.chen@...anux.com>
cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] kernel/itimer.c: beautify code, not need check 'value',
 so save one instruction, simpler and easier for readers.

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.

> 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. 

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.

> 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.

Can you please start to think about, why YOU thought that returning
-EINVAL is the proper return value for that case?

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.

And I'm neither accepting sloppy patches nor am I accepting sloppy
changelogs which make false claims.

Thanks,

	tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/