Hi and thanks for reviewing.
On Friday 09 April 2010 13:49:12 you wrote:+ *the function name implies that it is used to parse unsigned long, so
+ * 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;
negative value should not be supported.
My intention was to signal that the argument is unsigned long and that the sign come separate in neg, but I am OK with changing the function name to proc_get_long() if you think that is better.
+ if (!isdigit(*p))It seems that ledding white space should be allowed, so this check
+ return -EINVAL;
isn't needed, and simple_strtoul can handle it.
Leading white space is skipped with proc_skip_space before calling this function. AFAICS simple_strtoul does not handle whitespaces.
+is strspn() better?
+ *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;
I don't think it will work out, \0 is an accepted trailer for many of the function which use this function.
+negative should not be supported too.
+ 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);
We need negatives in proc_dointvec, again we can change the function name if it will clear things up.
<snip>int val = *valp;These functions have so much lines of code. I think you can make them
unsigned long lval;
if (val < 0) {
- *negp = -1;
+ *negp = 1;
lval = (unsigned long)-val;
} else {
*negp = 0;
less. Please refer to strsep().
Hmm, the input its pretty permissive and maybe this is why it looks so fat, we need to account for quite a few cases.