lkml.org 
[lkml]   [2004]   [Jan]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] bitmap parsing/printing routines, version 4
Joe Korty wrote:
>>>However, IMHO you added too many comments. Unlike Andrew, I do believe
>>>one can have too many comments. Comments become 'too many' when they
>>>dilute to the point that the code can no longer be clearly read.
>>>
>>>If you reduce the comments to just those that say something not easily
>>>deduced from the code, then they would be acceptable to me, and would
>>>make a useful addition IMO. That would be all but three, or perhaps four,
>>>of them.
>>>
>>>Andrew, if you do like the fully commented version, then please remove
>>>my name from the comment in the patch. The dilute style of coding is
>>>not one I wish to have my name associated with.
>>>
>>>Thanks,
>>>Joe
>>
>>I'm sorry you feel that way, Joe. I had no intention of "diluting" your
>>code, and I certainly don't want you to remove your name from good code
>>you spent significant time & effort on. I'm just about to go to sleep,
>>so I made this patch pretty quickly. I think the 4 comments I kept are
>>the most useful and non-obvious. Let me know if this looks acceptable
>>to you. As I said, I have no desire to have you pull your name from the
>>code, especially since I feel it is good code!
>>
>>Andrew, once Joe and I work out an acceptable patch, we'll make sure you
>>get a copy.
>
>
> Much better, Matthew. I can live with this latest patch:)
> Thanks,
> Joe

Great! Ok Andrew, I've rediffed this patch against -mm5, and the
attatched patch should apply cleanly.

Joe & Paul, kudos on the code. Looks great!

-Matt

diff -Nurp --exclude-from=/home/mcd/.dontdiff linux-2.6.1-mm5/lib/bitmap.c linux-2.6.1-bitmap_readability/lib/bitmap.c
--- linux-2.6.1-mm5/lib/bitmap.c Tue Jan 20 08:56:40 2004
+++ linux-2.6.1-bitmap_readability/lib/bitmap.c Tue Jan 20 09:00:28 2004
@@ -210,13 +210,13 @@ EXPORT_SYMBOL(bitmap_snprintf);
* bits of the resultant bitmask. No chunk may specify a value larger
* than 32 bits (-EOVERFLOW), and if a chunk specifies a smaller value
* then leading 0-bits are prepended. -EINVAL is returned for illegal
- * characters and for grouping errors such as "1,,5", ,44", "," and "".
+ * characters and for grouping errors such as "1,,5", ",44", "," and "".
* Leading and trailing whitespace accepted, but not embedded whitespace.
*/
int bitmap_parse(const char __user *ubuf, unsigned int ubuflen,
unsigned long *maskp, int nmaskbits)
{
- int i, c, oc, ndigits, totaldigits, nchunks, nbits;
+ int i, c, old_c, totaldigits, ndigits, nchunks, nbits;
u32 chunk;

bitmap_clear(maskp, nmaskbits);
@@ -224,21 +224,39 @@ int bitmap_parse(const char __user *ubuf
nchunks = nbits = totaldigits = c = 0;
do {
chunk = ndigits = 0;
+
+ /* Get the next chunk of the bitmap */
while (ubuflen) {
- oc = c;
+ old_c = c;
if (get_user(c, ubuf++))
return -EFAULT;
ubuflen--;
if (isspace(c))
continue;
- if (totaldigits && c && isspace(oc))
+
+ /*
+ * If the last character was a space and the current
+ * character isn't '\0', we've got embedded whitespace.
+ * This is a no-no, so throw an error.
+ */
+ if (totaldigits && c && isspace(old_c))
return -EINVAL;
- if (!c || c == ',')
+
+ /* A '\0' or a ',' signal the end of the chunk */
+ if (c == '\0' || c == ',')
break;
+
if (!isxdigit(c))
return -EINVAL;
+
+ /*
+ * Make sure there are at least 4 free bits in 'chunk'.
+ * If not, this hexdigit will overflow 'chunk', so
+ * throw an error.
+ */
if (chunk & ~((1UL << (CHUNKSZ - 4)) - 1))
return -EOVERFLOW;
+
chunk = (chunk << 4) | unhex(c);
ndigits++; totaldigits++;
}
@@ -246,6 +264,7 @@ int bitmap_parse(const char __user *ubuf
return -EINVAL;
if (nchunks == 0 && chunk == 0)
continue;
+
bitmap_shift_right(maskp, maskp, CHUNKSZ, nmaskbits);
for (i = 0; i < CHUNKSZ; i++)
if (chunk & (1 << i))
@@ -256,8 +275,6 @@ int bitmap_parse(const char __user *ubuf
return -EOVERFLOW;
} while (ubuflen && c == ',');

- if (totaldigits == 0)
- return -EINVAL;
return 0;
}
EXPORT_SYMBOL(bitmap_parse);
\
 
 \ /
  Last update: 2005-03-22 14:00    [W:1.800 / U:0.076 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site