Re: [Patch 1/3] sysctl: refactor integer handling proc code

From: Changli Gao
Date: Fri Apr 09 2010 - 06:49:58 EST


On Fri, Apr 9, 2010 at 6:11 PM, Amerigo Wang <amwang@xxxxxxxxxx> wrote:
>
> From: Octavian Purdila <opurdila@xxxxxxxxxxx>
>
> As we are about to add another integer handling proc function a little
> bit of cleanup is in order: add a few helper functions to improve code
> readability and decrease code duplication.
>
> In the process a bug is also fixed: if the user specifies a number
> with more then 20 digits it will be interpreted as two integers
> (e.g. 10000...13 will be interpreted as 100.... and 13).
>
> Behavior for EFAULT handling was changed as well. Previous to this
> patch, when an EFAULT error occurred in the middle of a write
> operation, although some of the elements were set, that was not
> acknowledged to the user (by shorting the write and returning the
> number of bytes accepted). EFAULT is now treated just like any other
> errors by acknowledging the amount of bytes accepted.
>
> Signed-off-by: Octavian Purdila <opurdila@xxxxxxxxxxx>
> Signed-off-by: WANG Cong <amwang@xxxxxxxxxx>
> Cc: Eric W. Biederman <ebiederm@xxxxxxxxxxxx>
> ---
>
> Index: linux-2.6/kernel/sysctl.c
> ===================================================================
> --- linux-2.6.orig/kernel/sysctl.c
> +++ linux-2.6/kernel/sysctl.c
> @@ -2040,8 +2040,148 @@ int proc_dostring(struct ctl_table *tabl
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â buffer, lenp, ppos);
> Â}
>
> +static int proc_skip_wspace(char __user **buf, size_t *size)
> +{
> + Â Â Â char c;
> +
> + Â Â Â while (*size) {
> + Â Â Â Â Â Â Â if (get_user(c, *buf))
> + Â Â Â Â Â Â Â Â Â Â Â return -EFAULT;
> + Â Â Â Â Â Â Â if (!isspace(c))
> + Â Â Â Â Â Â Â Â Â Â Â break;
> + Â Â Â Â Â Â Â (*size)--;
> + Â Â Â Â Â Â Â (*buf)++;
> + Â Â Â }
> +
> + Â Â Â return 0;
> +}
> +
> +static bool isanyof(char c, const char *v, unsigned len)
> +{
> + Â Â Â int i;
> +
> + Â Â Â if (!len)
> + Â Â Â Â Â Â Â return false;
> +
> + Â Â Â for (i = 0; i < len; i++)
> + Â Â Â Â Â Â Â if (c == v[i])
> + Â Â Â Â Â Â Â Â Â Â Â break;
> + Â Â Â if (i == len)
> + Â Â Â Â Â Â Â return false;
> +
> + Â Â Â return true;
> +}
> +
> +#define TMPBUFLEN 22
> +/**
> + * proc_get_ulong - reads an ASCII formated integer from a user buffer
> + *
> + * @buf - user buffer
> + * @size - size of the user buffer
> + * @val - this is where the number will be stored
> + * @neg - set to %TRUE if number is negative
> + * @perm_tr - a vector which contains the allowed trailers
> + * @perm_tr_len - size of the perm_tr vector
> + * @tr - pointer to store the trailer character
> + *
> + * In case of success 0 is returned and buf and size are updated with
> + * the amount of bytes read. If tr is non NULL and a trailing
> + * character exist (size is non zero after returning from this
> + * function) tr is updated with the trailing character.
> + */
> +static int proc_get_ulong(char __user **buf, size_t *size,
> + Â Â Â Â Â Â Â Â Â Â Â Â unsigned long *val, bool *neg,
> + Â Â Â Â Â Â Â Â Â Â Â Â const char *perm_tr, unsigned perm_tr_len, char *tr)
> +{
> + Â Â Â int len;
> + Â Â Â char *p, tmp[TMPBUFLEN];
> +
> + Â Â Â if (!*size)
> + Â Â Â Â Â Â Â return -EINVAL;
> +
> + Â Â Â len = *size;
> + Â Â Â if (len > TMPBUFLEN-1)
> + Â Â Â Â Â Â Â len = TMPBUFLEN-1;
> +
> + Â Â Â if (copy_from_user(tmp, *buf, len))
> + Â Â Â Â Â Â Â return -EFAULT;
> +
> + Â Â Â tmp[len] = 0;
> + Â Â Â p = tmp;
> + Â Â Â if (*p == '-' && *size > 1) {
> + Â Â Â Â Â Â Â *neg = 1;
> + Â Â Â Â Â Â Â p++;
> + Â Â Â } else
> + Â Â Â Â Â Â Â *neg = 0;

the function name implies that it is used to parse unsigned long, so
negative value should not be supported.

> + Â Â Â if (!isdigit(*p))
> + Â Â Â Â Â Â Â return -EINVAL;

It seems that ledding white space should be allowed, so this check
isn't needed, and simple_strtoul can handle it.

> +
> + Â Â Â *val = simple_strtoul(p, &p, 0);
> +
> + Â Â Â len = p - tmp;
> +
> + Â Â Â /* We don't know if the next char is whitespace thus we may accept
> + Â Â Â Â* invalid integers (e.g. 1234...a) or two integers instead of one
> + Â Â Â Â* (e.g. 123...1). So lets not allow such large numbers. */
> + Â Â Â if (len == TMPBUFLEN - 1)
> + Â Â Â Â Â Â Â return -EINVAL;
> +
> + Â Â Â if (len < *size && perm_tr_len && !isanyof(*p, perm_tr, perm_tr_len))
> + Â Â Â Â Â Â Â return -EINVAL;

is strspn() better?

> +
> + Â Â Â if (tr && (len < *size))
> + Â Â Â Â Â Â Â *tr = *p;
> +
> + Â Â Â *buf += len;
> + Â Â Â *size -= len;
> +
> + Â Â Â return 0;
> +}
> +
> +/**
> + * proc_put_ulong - coverts an integer to a decimal ASCII formated string
> + *
> + * @buf - the user buffer
> + * @size - the size of the user buffer
> + * @val - the integer to be converted
> + * @neg - sign of the number, %TRUE for negative
> + * @first - if %FALSE will insert a separator character before the number
> + * @separator - the separator character
> + *
> + * In case of success 0 is returned and buf and size are updated with
> + * the amount of bytes read.
> + */
> +static int proc_put_ulong(char __user **buf, size_t *size, unsigned long val,
> + Â Â Â Â Â Â Â Â Â Â Â Â bool neg, bool first, char separator)
> +{
> + Â Â Â int len;
> + Â Â Â char tmp[TMPBUFLEN], *p = tmp;
> +
> + Â Â Â if (!first)
> + Â Â Â Â Â Â Â *p++ = separator;
> + Â Â Â sprintf(p, "%s%lu", neg ? "-" : "", val);

negative should not be supported too.

> + Â Â Â len = strlen(tmp);
> + Â Â Â if (len > *size)
> + Â Â Â Â Â Â Â len = *size;
> + Â Â Â if (copy_to_user(*buf, tmp, len))
> + Â Â Â Â Â Â Â return -EFAULT;
> + Â Â Â *size -= len;
> + Â Â Â *buf += len;
> + Â Â Â return 0;
> +}
> +#undef TMPBUFLEN
> +
> +static int proc_put_char(char __user **buf, size_t *size, char c)
> +{
> + Â Â Â if (*size) {
> + Â Â Â Â Â Â Â if (put_user(c, *buf))
> + Â Â Â Â Â Â Â Â Â Â Â return -EFAULT;
> + Â Â Â Â Â Â Â (*size)--, (*buf)++;
> + Â Â Â }
> + Â Â Â return 0;
> +}
>
> -static int do_proc_dointvec_conv(int *negp, unsigned long *lvalp,
> +static int do_proc_dointvec_conv(bool *negp, unsigned long *lvalp,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â int *valp,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â int write, void *data)
> Â{
> @@ -2050,7 +2190,7 @@ static int do_proc_dointvec_conv(int *ne
> Â Â Â Â} else {
> Â Â Â Â Â Â Â Âint val = *valp;
> Â Â Â Â Â Â Â Âif (val < 0) {
> - Â Â Â Â Â Â Â Â Â Â Â *negp = -1;
> + Â Â Â Â Â Â Â Â Â Â Â *negp = 1;
> Â Â Â Â Â Â Â Â Â Â Â Â*lvalp = (unsigned long)-val;
> Â Â Â Â Â Â Â Â} else {
> Â Â Â Â Â Â Â Â Â Â Â Â*negp = 0;
> @@ -2060,20 +2200,18 @@ static int do_proc_dointvec_conv(int *ne
> Â Â Â Âreturn 0;
> Â}
>
> +static const char proc_wspace_sep[] = { ' ', '\t', '\n', 0 };
> +
> Âstatic int __do_proc_dointvec(void *tbl_data, struct ctl_table *table,
> - Â Â Â Â Â Â Â Â int write, void __user *buffer,
> + Â Â Â Â Â Â Â Â int write, void __user *_buffer,
> Â Â Â Â Â Â Â Â Âsize_t *lenp, loff_t *ppos,
> - Â Â Â Â Â Â Â Â int (*conv)(int *negp, unsigned long *lvalp, int *valp,
> + Â Â Â Â Â Â Â Â int (*conv)(bool *negp, unsigned long *lvalp, int *valp,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âint write, void *data),
> Â Â Â Â Â Â Â Â Âvoid *data)
> Â{
> -#define TMPBUFLEN 21
> - Â Â Â int *i, vleft, first = 1, neg;
> - Â Â Â unsigned long lval;
> - Â Â Â size_t left, len;
> -
> - Â Â Â char buf[TMPBUFLEN], *p;
> - Â Â Â char __user *s = buffer;
> + Â Â Â int *i, vleft, first = 1, err = 0;
> + Â Â Â size_t left;
> + Â Â Â char __user *buffer = (char __user *) _buffer;
>
> Â Â Â Âif (!tbl_data || !table->maxlen || !*lenp ||
> Â Â Â Â Â Â(*ppos && !write)) {
> @@ -2089,88 +2227,48 @@ static int __do_proc_dointvec(void *tbl_
> Â Â Â Â Â Â Â Âconv = do_proc_dointvec_conv;
>
> Â Â Â Âfor (; left && vleft--; i++, first=0) {
> - Â Â Â Â Â Â Â if (write) {
> - Â Â Â Â Â Â Â Â Â Â Â while (left) {
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â char c;
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (get_user(c, s))
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â return -EFAULT;
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (!isspace(c))
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â break;
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â left--;
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â s++;
> - Â Â Â Â Â Â Â Â Â Â Â }
> - Â Â Â Â Â Â Â Â Â Â Â if (!left)
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â break;
> - Â Â Â Â Â Â Â Â Â Â Â neg = 0;
> - Â Â Â Â Â Â Â Â Â Â Â len = left;
> - Â Â Â Â Â Â Â Â Â Â Â if (len > sizeof(buf) - 1)
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â len = sizeof(buf) - 1;
> - Â Â Â Â Â Â Â Â Â Â Â if (copy_from_user(buf, s, len))
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â return -EFAULT;
> - Â Â Â Â Â Â Â Â Â Â Â buf[len] = 0;
> - Â Â Â Â Â Â Â Â Â Â Â p = buf;
> - Â Â Â Â Â Â Â Â Â Â Â if (*p == '-' && left > 1) {
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â neg = 1;
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â p++;
> - Â Â Â Â Â Â Â Â Â Â Â }
> - Â Â Â Â Â Â Â Â Â Â Â if (*p < '0' || *p > '9')
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â break;
> -
> - Â Â Â Â Â Â Â Â Â Â Â lval = simple_strtoul(p, &p, 0);
> + Â Â Â Â Â Â Â unsigned long lval;
> + Â Â Â Â Â Â Â bool neg;
>
> - Â Â Â Â Â Â Â Â Â Â Â len = p-buf;
> - Â Â Â Â Â Â Â Â Â Â Â if ((len < left) && *p && !isspace(*p))
> + Â Â Â Â Â Â Â if (write) {
> + Â Â Â Â Â Â Â Â Â Â Â err = proc_skip_wspace(&buffer, &left);
> + Â Â Â Â Â Â Â Â Â Â Â if (err)
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â return err;
> + Â Â Â Â Â Â Â Â Â Â Â err = proc_get_ulong(&buffer, &left, &lval, &neg,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âproc_wspace_sep,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âsizeof(proc_wspace_sep), NULL);
> + Â Â Â Â Â Â Â Â Â Â Â if (err)
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âbreak;
> - Â Â Â Â Â Â Â Â Â Â Â s += len;
> - Â Â Â Â Â Â Â Â Â Â Â left -= len;
> -
> - Â Â Â Â Â Â Â Â Â Â Â if (conv(&neg, &lval, i, 1, data))
> + Â Â Â Â Â Â Â Â Â Â Â if (conv(&neg, &lval, i, 1, data)) {
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â err = -EINVAL;
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âbreak;
> + Â Â Â Â Â Â Â Â Â Â Â }
> Â Â Â Â Â Â Â Â} else {
> - Â Â Â Â Â Â Â Â Â Â Â p = buf;
> - Â Â Â Â Â Â Â Â Â Â Â if (!first)
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â *p++ = '\t';
> -
> - Â Â Â Â Â Â Â Â Â Â Â if (conv(&neg, &lval, i, 0, data))
> + Â Â Â Â Â Â Â Â Â Â Â if (conv(&neg, &lval, i, 0, data)) {
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â err = -EINVAL;
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âbreak;
> -
> - Â Â Â Â Â Â Â Â Â Â Â sprintf(p, "%s%lu", neg ? "-" : "", lval);
> - Â Â Â Â Â Â Â Â Â Â Â len = strlen(buf);
> - Â Â Â Â Â Â Â Â Â Â Â if (len > left)
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â len = left;
> - Â Â Â Â Â Â Â Â Â Â Â if(copy_to_user(s, buf, len))
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â return -EFAULT;
> - Â Â Â Â Â Â Â Â Â Â Â left -= len;
> - Â Â Â Â Â Â Â Â Â Â Â s += len;
> - Â Â Â Â Â Â Â }
> - Â Â Â }
> -
> - Â Â Â if (!write && !first && left) {
> - Â Â Â Â Â Â Â if(put_user('\n', s))
> - Â Â Â Â Â Â Â Â Â Â Â return -EFAULT;
> - Â Â Â Â Â Â Â left--, s++;
> - Â Â Â }
> - Â Â Â if (write) {
> - Â Â Â Â Â Â Â while (left) {
> - Â Â Â Â Â Â Â Â Â Â Â char c;
> - Â Â Â Â Â Â Â Â Â Â Â if (get_user(c, s++))
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â return -EFAULT;
> - Â Â Â Â Â Â Â Â Â Â Â if (!isspace(c))
> + Â Â Â Â Â Â Â Â Â Â Â }
> + Â Â Â Â Â Â Â Â Â Â Â err = proc_put_ulong(&buffer, &left, lval, neg, first,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â'\t');
> + Â Â Â Â Â Â Â Â Â Â Â if (err)
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âbreak;
> - Â Â Â Â Â Â Â Â Â Â Â left--;
> Â Â Â Â Â Â Â Â}
> Â Â Â Â}
> +
> + Â Â Â if (!write && !first && left && !err)
> + Â Â Â Â Â Â Â err = proc_put_char(&buffer, &left, '\n');
> + Â Â Â if (write && !err)
> + Â Â Â Â Â Â Â err = proc_skip_wspace(&buffer, &left);
> Â Â Â Âif (write && first)
> - Â Â Â Â Â Â Â return -EINVAL;
> + Â Â Â Â Â Â Â return err ? : -EINVAL;
> Â Â Â Â*lenp -= left;
> Â Â Â Â*ppos += *lenp;
> Â Â Â Âreturn 0;
> -#undef TMPBUFLEN
> Â}
>
> Âstatic int do_proc_dointvec(struct ctl_table *table, int write,
> Â Â Â Â Â Â Â Â Âvoid __user *buffer, size_t *lenp, loff_t *ppos,
> - Â Â Â Â Â Â Â Â int (*conv)(int *negp, unsigned long *lvalp, int *valp,
> + Â Â Â Â Â Â Â Â int (*conv)(bool *negp, unsigned long *lvalp, int *valp,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âint write, void *data),
> Â Â Â Â Â Â Â Â Âvoid *data)
> Â{
> @@ -2238,8 +2336,8 @@ struct do_proc_dointvec_minmax_conv_para
> Â Â Â Âint *max;
> Â};
>
> -static int do_proc_dointvec_minmax_conv(int *negp, unsigned long *lvalp,
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â int *valp,
> +static int do_proc_dointvec_minmax_conv(bool *negp, unsigned long *lvalp,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â int *valp,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âint write, void *data)
> Â{
> Â Â Â Âstruct do_proc_dointvec_minmax_conv_param *param = data;
> @@ -2252,7 +2350,7 @@ static int do_proc_dointvec_minmax_conv(
> Â Â Â Â} else {
> Â Â Â Â Â Â Â Âint val = *valp;
> Â Â Â Â Â Â Â Âif (val < 0) {
> - Â Â Â Â Â Â Â Â Â Â Â *negp = -1;
> + Â Â Â Â Â Â Â Â Â Â Â *negp = 1;
> Â Â Â Â Â Â Â Â Â Â Â Â*lvalp = (unsigned long)-val;
> Â Â Â Â Â Â Â Â} else {
> Â Â Â Â Â Â Â Â Â Â Â Â*negp = 0;
> @@ -2290,17 +2388,15 @@ int proc_dointvec_minmax(struct ctl_tabl
> Â}
>
> Âstatic int __do_proc_doulongvec_minmax(void *data, struct ctl_table *table, int write,
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âvoid __user *buffer,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âvoid __user *_buffer,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â size_t *lenp, loff_t *ppos,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â unsigned long convmul,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â unsigned long convdiv)
> Â{
> -#define TMPBUFLEN 21
> - Â Â Â unsigned long *i, *min, *max, val;
> - Â Â Â int vleft, first=1, neg;
> - Â Â Â size_t len, left;
> - Â Â Â char buf[TMPBUFLEN], *p;
> - Â Â Â char __user *s = buffer;
> + Â Â Â unsigned long *i, *min, *max;
> + Â Â Â int vleft, first = 1, err = 0;
> + Â Â Â size_t left;
> + Â Â Â char __user *buffer = (char __user *) _buffer;
>
> Â Â Â Âif (!data || !table->maxlen || !*lenp ||
> Â Â Â Â Â Â(*ppos && !write)) {
> @@ -2315,82 +2411,42 @@ static int __do_proc_doulongvec_minmax(v
> Â Â Â Âleft = *lenp;
>
> Â Â Â Âfor (; left && vleft--; i++, min++, max++, first=0) {
> + Â Â Â Â Â Â Â unsigned long val;
> +
> Â Â Â Â Â Â Â Âif (write) {
> - Â Â Â Â Â Â Â Â Â Â Â while (left) {
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â char c;
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (get_user(c, s))
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â return -EFAULT;
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (!isspace(c))
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â break;
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â left--;
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â s++;
> - Â Â Â Â Â Â Â Â Â Â Â }
> - Â Â Â Â Â Â Â Â Â Â Â if (!left)
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â break;
> - Â Â Â Â Â Â Â Â Â Â Â neg = 0;
> - Â Â Â Â Â Â Â Â Â Â Â len = left;
> - Â Â Â Â Â Â Â Â Â Â Â if (len > TMPBUFLEN-1)
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â len = TMPBUFLEN-1;
> - Â Â Â Â Â Â Â Â Â Â Â if (copy_from_user(buf, s, len))
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â return -EFAULT;
> - Â Â Â Â Â Â Â Â Â Â Â buf[len] = 0;
> - Â Â Â Â Â Â Â Â Â Â Â p = buf;
> - Â Â Â Â Â Â Â Â Â Â Â if (*p == '-' && left > 1) {
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â neg = 1;
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â p++;
> - Â Â Â Â Â Â Â Â Â Â Â }
> - Â Â Â Â Â Â Â Â Â Â Â if (*p < '0' || *p > '9')
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â break;
> - Â Â Â Â Â Â Â Â Â Â Â val = simple_strtoul(p, &p, 0) * convmul / convdiv ;
> - Â Â Â Â Â Â Â Â Â Â Â len = p-buf;
> - Â Â Â Â Â Â Â Â Â Â Â if ((len < left) && *p && !isspace(*p))
> + Â Â Â Â Â Â Â Â Â Â Â bool neg;
> +
> + Â Â Â Â Â Â Â Â Â Â Â err = proc_skip_wspace(&buffer, &left);
> + Â Â Â Â Â Â Â Â Â Â Â if (err)
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â return err;
> + Â Â Â Â Â Â Â Â Â Â Â err = proc_get_ulong(&buffer, &left, &val, &neg,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âproc_wspace_sep,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âsizeof(proc_wspace_sep), NULL);
> + Â Â Â Â Â Â Â Â Â Â Â if (err)
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âbreak;
> Â Â Â Â Â Â Â Â Â Â Â Âif (neg)
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â val = -val;
> - Â Â Â Â Â Â Â Â Â Â Â s += len;
> - Â Â Â Â Â Â Â Â Â Â Â left -= len;
> -
> - Â Â Â Â Â Â Â Â Â Â Â if(neg)
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âcontinue;
> Â Â Â Â Â Â Â Â Â Â Â Âif ((min && val < *min) || (max && val > *max))
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âcontinue;
> Â Â Â Â Â Â Â Â Â Â Â Â*i = val;
> Â Â Â Â Â Â Â Â} else {
> - Â Â Â Â Â Â Â Â Â Â Â p = buf;
> - Â Â Â Â Â Â Â Â Â Â Â if (!first)
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â *p++ = '\t';
> - Â Â Â Â Â Â Â Â Â Â Â sprintf(p, "%lu", convdiv * (*i) / convmul);
> - Â Â Â Â Â Â Â Â Â Â Â len = strlen(buf);
> - Â Â Â Â Â Â Â Â Â Â Â if (len > left)
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â len = left;
> - Â Â Â Â Â Â Â Â Â Â Â if(copy_to_user(s, buf, len))
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â return -EFAULT;
> - Â Â Â Â Â Â Â Â Â Â Â left -= len;
> - Â Â Â Â Â Â Â Â Â Â Â s += len;
> - Â Â Â Â Â Â Â }
> - Â Â Â }
> -
> - Â Â Â if (!write && !first && left) {
> - Â Â Â Â Â Â Â if(put_user('\n', s))
> - Â Â Â Â Â Â Â Â Â Â Â return -EFAULT;
> - Â Â Â Â Â Â Â left--, s++;
> - Â Â Â }
> - Â Â Â if (write) {
> - Â Â Â Â Â Â Â while (left) {
> - Â Â Â Â Â Â Â Â Â Â Â char c;
> - Â Â Â Â Â Â Â Â Â Â Â if (get_user(c, s++))
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â return -EFAULT;
> - Â Â Â Â Â Â Â Â Â Â Â if (!isspace(c))
> + Â Â Â Â Â Â Â Â Â Â Â val = convdiv * (*i) / convmul;
> + Â Â Â Â Â Â Â Â Â Â Â err = proc_put_ulong(&buffer, &left, val, 0, first,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â'\t');
> + Â Â Â Â Â Â Â Â Â Â Â if (err)
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âbreak;
> - Â Â Â Â Â Â Â Â Â Â Â left--;
> Â Â Â Â Â Â Â Â}
> Â Â Â Â}
> +
> + Â Â Â if (!write && !first && left && !err)
> + Â Â Â Â Â Â Â err = proc_put_char(&buffer, &left, '\n');
> + Â Â Â if (write && !err)
> + Â Â Â Â Â Â Â err = proc_skip_wspace(&buffer, &left);
> Â Â Â Âif (write && first)
> - Â Â Â Â Â Â Â return -EINVAL;
> + Â Â Â Â Â Â Â return err ? : -EINVAL;
> Â Â Â Â*lenp -= left;
> Â Â Â Â*ppos += *lenp;
> Â Â Â Âreturn 0;
> -#undef TMPBUFLEN
> Â}
>
> Âstatic int do_proc_doulongvec_minmax(struct ctl_table *table, int write,
> @@ -2451,7 +2507,7 @@ int proc_doulongvec_ms_jiffies_minmax(st
> Â}
>
>
> -static int do_proc_dointvec_jiffies_conv(int *negp, unsigned long *lvalp,
> +static int do_proc_dointvec_jiffies_conv(bool *negp, unsigned long *lvalp,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â int *valp,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â int write, void *data)
> Â{
> @@ -2463,7 +2519,7 @@ static int do_proc_dointvec_jiffies_conv
> Â Â Â Â Â Â Â Âint val = *valp;
> Â Â Â Â Â Â Â Âunsigned long lval;
> Â Â Â Â Â Â Â Âif (val < 0) {
> - Â Â Â Â Â Â Â Â Â Â Â *negp = -1;
> + Â Â Â Â Â Â Â Â Â Â Â *negp = 1;
> Â Â Â Â Â Â Â Â Â Â Â Âlval = (unsigned long)-val;
> Â Â Â Â Â Â Â Â} else {
> Â Â Â Â Â Â Â Â Â Â Â Â*negp = 0;
> @@ -2474,7 +2530,7 @@ static int do_proc_dointvec_jiffies_conv
> Â Â Â Âreturn 0;
> Â}
>
> -static int do_proc_dointvec_userhz_jiffies_conv(int *negp, unsigned long *lvalp,
> +static int do_proc_dointvec_userhz_jiffies_conv(bool *negp, unsigned long *lvalp,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âint *valp,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âint write, void *data)
> Â{
> @@ -2486,7 +2542,7 @@ static int do_proc_dointvec_userhz_jiffi
> Â Â Â Â Â Â Â Âint val = *valp;
> Â Â Â Â Â Â Â Âunsigned long lval;
> Â Â Â Â Â Â Â Âif (val < 0) {
> - Â Â Â Â Â Â Â Â Â Â Â *negp = -1;
> + Â Â Â Â Â Â Â Â Â Â Â *negp = 1;
> Â Â Â Â Â Â Â Â Â Â Â Âlval = (unsigned long)-val;
> Â Â Â Â Â Â Â Â} else {
> Â Â Â Â Â Â Â Â Â Â Â Â*negp = 0;
> @@ -2497,7 +2553,7 @@ static int do_proc_dointvec_userhz_jiffi
> Â Â Â Âreturn 0;
> Â}
>
> -static int do_proc_dointvec_ms_jiffies_conv(int *negp, unsigned long *lvalp,
> +static int do_proc_dointvec_ms_jiffies_conv(bool *negp, unsigned long *lvalp,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âint *valp,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âint write, void *data)
> Â{
> @@ -2507,7 +2563,7 @@ static int do_proc_dointvec_ms_jiffies_c
> Â Â Â Â Â Â Â Âint val = *valp;
> Â Â Â Â Â Â Â Âunsigned long lval;
> Â Â Â Â Â Â Â Âif (val < 0) {
> - Â Â Â Â Â Â Â Â Â Â Â *negp = -1;
> + Â Â Â Â Â Â Â Â Â Â Â *negp = 1;
> Â Â Â Â Â Â Â Â Â Â Â Âlval = (unsigned long)-val;
> Â Â Â Â Â Â Â Â} else {
> Â Â Â Â Â Â Â Â Â Â Â Â*negp = 0;

These functions have so much lines of code. I think you can make them
less. Please refer to strsep().

--
Regardsï
Changli Gao(xiaosuo@xxxxxxxxx)
¢éì®&Þ~º&¶¬–+-±éÝ¥Šw®žË±Êâmébžìdz¹Þ)í…æèw*jg¬±¨¶‰šŽŠÝj/êäz¹ÞŠà2ŠÞ¨è­Ú&¢)ß«a¶Úþø®G«éh®æj:+v‰¨Šwè†Ù>Wš±êÞiÛaxPjØm¶Ÿÿà -»+ƒùdš_