From: Chen Gang <gang.chen@asianux.com>
To: Al Viro <viro@zeniv.linux.org.uk>,
"Eric W. Biederman" <ebiederm@xmission.com>,
xi.wang@gmail.com, nicolas.dichtel@6wind.com
Cc: Andrew Morton <akpm@linux-foundation.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: [PATCH] kernel/sysctl_binary.c: improve the usage of return value 'result'
Date: Tue, 06 Aug 2013 15:29:42 +0800 [thread overview]
Message-ID: <5200A5E6.9020803@asianux.com> (raw)
Improve the usage of return value 'result', so not only can make code
clearer to readers, but also can improve the performance.
Assign the return error code only when error occurs, so can save some
instructions (some of them are inside looping).
Rewrite the sysctl_getname() to make code clearer to readers, and also
can save some instructions (it is mainly related with the usage of the
variable 'result').
Remove useless variable 'result' for sysctl() and compat sysctl(), when
do_sysctl() fails, it is meaningless to assign 'oldlenp ' with original
same value.
Also modify the related code to pass "./scripts/checkpatch.pl" checking.
Signed-off-by: Chen Gang <gang.chen@asianux.com>
---
kernel/sysctl_binary.c | 142 ++++++++++++++++++++++++------------------------
1 files changed, 72 insertions(+), 70 deletions(-)
diff --git a/kernel/sysctl_binary.c b/kernel/sysctl_binary.c
index b609213..26d7fc0 100644
--- a/kernel/sysctl_binary.c
+++ b/kernel/sysctl_binary.c
@@ -941,15 +941,17 @@ static ssize_t bin_string(struct file *file,
copied = result;
lastp = oldval + copied - 1;
- result = -EFAULT;
- if (get_user(ch, lastp))
+ if (get_user(ch, lastp)) {
+ result = -EFAULT;
goto out;
+ }
/* Trim off the trailing newline */
if (ch == '\n') {
- result = -EFAULT;
- if (put_user('\0', lastp))
+ if (put_user('\0', lastp)) {
+ result = -EFAULT;
goto out;
+ }
copied -= 1;
}
}
@@ -974,10 +976,11 @@ static ssize_t bin_intvec(struct file *file,
char *buffer;
ssize_t result;
- result = -ENOMEM;
buffer = kmalloc(BUFSZ, GFP_KERNEL);
- if (!buffer)
+ if (!buffer) {
+ result = -ENOMEM;
goto out;
+ }
if (oldval && oldlen) {
unsigned __user *vec = oldval;
@@ -999,9 +1002,10 @@ static ssize_t bin_intvec(struct file *file,
while (isspace(*str))
str++;
- result = -EFAULT;
- if (put_user(value, vec + i))
+ if (put_user(value, vec + i)) {
+ result = -EFAULT;
goto out_kfree;
+ }
copied += sizeof(*vec);
if (!isdigit(*str))
@@ -1020,10 +1024,10 @@ static ssize_t bin_intvec(struct file *file,
for (i = 0; i < length; i++) {
unsigned long value;
- result = -EFAULT;
- if (get_user(value, vec + i))
+ if (get_user(value, vec + i)) {
+ result = -EFAULT;
goto out_kfree;
-
+ }
str += snprintf(str, end - str, "%lu\t", value);
}
@@ -1045,10 +1049,11 @@ static ssize_t bin_ulongvec(struct file *file,
char *buffer;
ssize_t result;
- result = -ENOMEM;
buffer = kmalloc(BUFSZ, GFP_KERNEL);
- if (!buffer)
+ if (!buffer) {
+ result = -ENOMEM;
goto out;
+ }
if (oldval && oldlen) {
unsigned long __user *vec = oldval;
@@ -1070,9 +1075,10 @@ static ssize_t bin_ulongvec(struct file *file,
while (isspace(*str))
str++;
- result = -EFAULT;
- if (put_user(value, vec + i))
+ if (put_user(value, vec + i)) {
+ result = -EFAULT;
goto out_kfree;
+ }
copied += sizeof(*vec);
if (!isdigit(*str))
@@ -1091,10 +1097,10 @@ static ssize_t bin_ulongvec(struct file *file,
for (i = 0; i < length; i++) {
unsigned long value;
- result = -EFAULT;
- if (get_user(value, vec + i))
+ if (get_user(value, vec + i)) {
+ result = -EFAULT;
goto out_kfree;
-
+ }
str += snprintf(str, end - str, "%lu\t", value);
}
@@ -1128,10 +1134,10 @@ static ssize_t bin_uuid(struct file *file,
/* Convert the uuid to from a string to binary */
for (i = 0; i < 16; i++) {
- result = -EIO;
- if (!isxdigit(str[0]) || !isxdigit(str[1]))
+ if (!isxdigit(str[0]) || !isxdigit(str[1])) {
+ result = -EIO;
goto out;
-
+ }
uuid[i] = (hex_to_bin(str[0]) << 4) |
hex_to_bin(str[1]);
str += 2;
@@ -1142,10 +1148,10 @@ static ssize_t bin_uuid(struct file *file,
if (oldlen > 16)
oldlen = 16;
- result = -EFAULT;
- if (copy_to_user(oldval, uuid, oldlen))
+ if (copy_to_user(oldval, uuid, oldlen)) {
+ result = -EFAULT;
goto out;
-
+ }
copied = oldlen;
}
result = copied;
@@ -1170,25 +1176,25 @@ static ssize_t bin_dn_node_address(struct file *file,
buf[result] = '\0';
/* Convert the decnet address to binary */
- result = -EIO;
nodep = strchr(buf, '.');
- if (!nodep)
+ if (!nodep) {
+ result = -EIO;
goto out;
+ }
++nodep;
area = simple_strtoul(buf, NULL, 10);
node = simple_strtoul(nodep, NULL, 10);
-
- result = -EIO;
- if ((area > 63)||(node > 1023))
+ if ((area > 63) || (node > 1023)) {
+ result = -EIO;
goto out;
-
+ }
dnaddr = cpu_to_le16((area << 10) | node);
- result = -EFAULT;
- if (put_user(dnaddr, (__le16 __user *)oldval))
+ if (put_user(dnaddr, (__le16 __user *)oldval)) {
+ result = -EFAULT;
goto out;
-
+ }
copied = sizeof(dnaddr);
}
@@ -1197,14 +1203,14 @@ static ssize_t bin_dn_node_address(struct file *file,
char buf[15];
int len;
- result = -EINVAL;
- if (newlen != sizeof(dnaddr))
+ if (newlen != sizeof(dnaddr)) {
+ result = -EINVAL;
goto out;
-
- result = -EFAULT;
- if (get_user(dnaddr, (__le16 __user *)newval))
+ }
+ if (get_user(dnaddr, (__le16 __user *)newval)) {
+ result = -EFAULT;
goto out;
-
+ }
len = snprintf(buf, sizeof(buf), "%hu.%hu",
le16_to_cpu(dnaddr) >> 10,
le16_to_cpu(dnaddr) & 0x3ff);
@@ -1276,19 +1282,20 @@ repeat:
static char *sysctl_getname(const int *name, int nlen, const struct bin_table **tablep)
{
- char *tmp, *result;
-
- result = ERR_PTR(-ENOMEM);
- tmp = __getname();
- if (tmp) {
- const struct bin_table *table = get_sysctl(name, nlen, tmp);
- result = tmp;
- *tablep = table;
- if (IS_ERR(table)) {
- __putname(tmp);
- result = ERR_CAST(table);
- }
+ char *result;
+ const struct bin_table *table;
+
+ result = __getname();
+ if (!result)
+ return ERR_PTR(-ENOMEM);
+
+ table = get_sysctl(name, nlen, result);
+ if (IS_ERR(table)) {
+ __putname(result);
+ return ERR_CAST(table);
}
+ *tablep = table;
+
return result;
}
@@ -1303,9 +1310,10 @@ static ssize_t binary_sysctl(const int *name, int nlen,
int flags;
pathname = sysctl_getname(name, nlen, &table);
- result = PTR_ERR(pathname);
- if (IS_ERR(pathname))
+ if (IS_ERR(pathname)) {
+ result = PTR_ERR(pathname);
goto out;
+ }
/* How should the sysctl be accessed? */
if (oldval && oldlen && newval && newlen) {
@@ -1321,10 +1329,10 @@ static ssize_t binary_sysctl(const int *name, int nlen,
mnt = task_active_pid_ns(current)->proc_mnt;
file = file_open_root(mnt->mnt_root, mnt, pathname, flags);
- result = PTR_ERR(file);
- if (IS_ERR(file))
+ if (IS_ERR(file)) {
+ result = PTR_ERR(file);
goto out_putname;
-
+ }
result = table->convert(file, oldval, oldlen, newval, newlen);
fput(file);
@@ -1420,7 +1428,6 @@ SYSCALL_DEFINE1(sysctl, struct __sysctl_args __user *, args)
{
struct __sysctl_args tmp;
size_t oldlen = 0;
- ssize_t result;
if (copy_from_user(&tmp, args, sizeof(tmp)))
return -EFAULT;
@@ -1431,18 +1438,16 @@ SYSCALL_DEFINE1(sysctl, struct __sysctl_args __user *, args)
if (tmp.oldlenp && get_user(oldlen, tmp.oldlenp))
return -EFAULT;
- result = do_sysctl(tmp.name, tmp.nlen, tmp.oldval, oldlen,
+ oldlen = do_sysctl(tmp.name, tmp.nlen, tmp.oldval, oldlen,
tmp.newval, tmp.newlen);
- if (result >= 0) {
- oldlen = result;
- result = 0;
- }
+ if ((ssize_t)oldlen < 0)
+ return (ssize_t)oldlen;
if (tmp.oldlenp && put_user(oldlen, tmp.oldlenp))
return -EFAULT;
- return result;
+ return 0;
}
@@ -1463,7 +1468,6 @@ COMPAT_SYSCALL_DEFINE1(sysctl, struct compat_sysctl_args __user *, args)
struct compat_sysctl_args tmp;
compat_size_t __user *compat_oldlenp;
size_t oldlen = 0;
- ssize_t result;
if (copy_from_user(&tmp, args, sizeof(tmp)))
return -EFAULT;
@@ -1475,19 +1479,17 @@ COMPAT_SYSCALL_DEFINE1(sysctl, struct compat_sysctl_args __user *, args)
if (compat_oldlenp && get_user(oldlen, compat_oldlenp))
return -EFAULT;
- result = do_sysctl(compat_ptr(tmp.name), tmp.nlen,
+ oldlen = do_sysctl(compat_ptr(tmp.name), tmp.nlen,
compat_ptr(tmp.oldval), oldlen,
compat_ptr(tmp.newval), tmp.newlen);
- if (result >= 0) {
- oldlen = result;
- result = 0;
- }
+ if ((ssize_t)oldlen < 0)
+ return (ssize_t)oldlen;
if (compat_oldlenp && put_user(oldlen, compat_oldlenp))
return -EFAULT;
- return result;
+ return 0;
}
#endif /* CONFIG_COMPAT */
--
1.7.7.6
next reply other threads:[~2013-08-06 7:30 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-06 7:29 Chen Gang [this message]
2013-08-06 21:43 ` [PATCH] kernel/sysctl_binary.c: improve the usage of return value 'result' Andrew Morton
2013-08-06 22:11 ` Joe Perches
2013-08-07 3:53 ` Chen Gang
2013-08-06 22:13 ` Eric W. Biederman
2013-08-07 5:28 ` Chen Gang F T
2013-08-07 5:11 ` Chen Gang
2013-08-06 21:46 ` Eric W. Biederman
2013-08-07 5:07 ` Chen Gang
2013-08-07 5:56 ` Li Zefan
2013-08-07 6:10 ` Joe Perches
2013-08-07 6:29 ` Chen Gang
2013-08-07 6:42 ` Li Zefan
2013-08-07 6:42 ` Li Zefan
2013-08-07 6:57 ` Chen Gang
2013-08-07 6:24 ` Chen Gang
2013-08-07 6:29 ` Andrew Morton
2013-08-07 6:34 ` Chen Gang
2013-08-07 7:02 ` Li Zefan
2013-08-07 8:03 ` Chen Gang
2013-08-07 8:44 ` Li Zefan
2013-08-07 9:13 ` Chen Gang
2013-08-07 7:45 ` Eric W. Biederman
2013-08-07 10:25 ` Chen Gang
2013-08-07 18:38 ` Eric W. Biederman
2013-08-08 3:19 ` 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=5200A5E6.9020803@asianux.com \
--to=gang.chen@asianux.com \
--cc=akpm@linux-foundation.org \
--cc=ebiederm@xmission.com \
--cc=linux-kernel@vger.kernel.org \
--cc=nicolas.dichtel@6wind.com \
--cc=viro@zeniv.linux.org.uk \
--cc=xi.wang@gmail.com \
/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