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
| ||
|
Date: Wed, 07 Aug 2013 13:11:33 +0800 From: Chen Gang <gang.chen@...anux.com> To: Andrew Morton <akpm@...ux-foundation.org> CC: Al Viro <viro@...iv.linux.org.uk>, "Eric W. Biederman" <ebiederm@...ssion.com>, xi.wang@...il.com, nicolas.dichtel@...nd.com, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org> Subject: Re: [PATCH] kernel/sysctl_binary.c: improve the usage of return value 'result' Thank you for your reply in details, especially you are very busy. My original opinion about optimization is incorrect. Thanks. On 08/07/2013 05:43 AM, Andrew Morton wrote: > On Tue, 06 Aug 2013 15:29:42 +0800 Chen Gang <gang.chen@...anux.com> wrote: > >> Improve the usage of return value 'result', so not only can make code >> clearer to readers, but also can improve the performance. > > It used to be pervasive kernel style do to > > ret = -ENOMEM; > foo = alloc(...); > if (!foo) > goto out; > > whereas nowadays people usually do the more straightforward > > foo = alloc(...); > if (!foo) { > ret = -ENOMEM; > goto out; > } > > The thinking was that the old style generated better code, but for the > life of me I can't remember why :( > > Your patch switches from old-style to new-style. And it appears to > have increased the text size. I did this, to switch three sites back > to old-style: > > --- a/kernel/sysctl_binary.c~kernel-sysctl_binaryc-improve-the-usage-of-return-value-result-fix > +++ a/kernel/sysctl_binary.c > @@ -941,17 +941,15 @@ static ssize_t bin_string(struct file *f > copied = result; > lastp = oldval + copied - 1; > > - if (get_user(ch, lastp)) { > - result = -EFAULT; > + result = -EFAULT; > + if (get_user(ch, lastp)) > goto out; > - } > > /* Trim off the trailing newline */ > if (ch == '\n') { > - if (put_user('\0', lastp)) { > - result = -EFAULT; > + result = -EFAULT; > + if (put_user('\0', lastp)) > goto out; > - } > copied -= 1; > } > } > @@ -976,11 +974,10 @@ static ssize_t bin_intvec(struct file *f > char *buffer; > ssize_t result; > > + result = -ENOMEM; > buffer = kmalloc(BUFSZ, GFP_KERNEL); > - if (!buffer) { > - result = -ENOMEM; > + if (!buffer) > goto out; > - } > > if (oldval && oldlen) { > unsigned __user *vec = oldval; > _ > > and kernel/sysctl_binary.o's .text got six bytes smaller. > > Now, smaller text doesn't mean faster code. But it probably means > larger cache footprint, which can mean slower code. > > IOW, it isn't obvious that this was an improvement. > > -- Chen Gang -- 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/
Powered by blists - more mailing lists