Re: 2.6.13-rc6-mm1

From: Jon Smirl
Date: Mon Aug 22 2005 - 15:46:50 EST


On 8/22/05, Benoit Boissinot <benoit.boissinot@xxxxxxxxxxxx> wrote:
> On Sun, Aug 21, 2005 at 06:34:48PM -0400, Jon Smirl wrote:
> > This should fix it, but I'm not on a machine where I can test it. Can
> > you give it a try and let me know?
> >
>
> it works ok.
> But there is still at least one problem: if ops->store returns an error,
> then there will be a substraction and the write will loop (i could do it
> with a store wich returned EINVAL and a 22 length string).
>
> I don't know if you can put a '\0' at buffer->page[count] if
> count == PAGE_SIZE.
>
> Moreover, i think it is more correct to add only the leading
> whitespace from the count because if the ops->store doesn't read
> everything it will do something weird:
>
> For example, if we have ' 123 ' and ops->store read only one char,
> then the function will return 7 (1 leading + 4 trailing + 1 read). For
> the next call the buffer will be filled only by spaces which is
> incorrect (it should be '23 ').

The attached version tries to fix these issues. I am still not
somewhere where I can test, so please check it out.

--
Jon Smirl
jonsmirl@xxxxxxxxx
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -6,6 +6,7 @@
#include <linux/fsnotify.h>
#include <linux/kobject.h>
#include <linux/namei.h>
+#include <linux/ctype.h>
#include <asm/uaccess.h>
#include <asm/semaphore.h>

@@ -207,8 +208,41 @@ flush_write_buffer(struct dentry * dentr
struct attribute * attr = to_attr(dentry);
struct kobject * kobj = to_kobj(dentry->d_parent);
struct sysfs_ops * ops = buffer->ops;
+ size_t ws_count = count, leading = 0;
+ int ret = 0;
+ char *x;

- return ops->store(kobj,attr,buffer->page,count);
+ /* locate trailing white space */
+ while ((ws_count > 0) && isspace(buffer->page[ws_count - 1]))
+ ws_count--;
+ if (ws_count == 0)
+ return count;
+
+ /* locate leading white space */
+ x = buffer->page;
+ while (isspace(*x))
+ x++;
+ leading = x - buffer->page;
+ ws_count -= leading;
+
+ /* interface is still ambigous about this */
+ /* string is both passed by length and terminated */
+ if (ws_count != PAGE_SIZE)
+ x[ws_count] = '\0';
+
+ ret = ops->store(kobj, attr, x, ws_count);
+
+ /* is it an error? */
+ if (ret < 0)
+ return ret;
+
+ /* the whole string was consumed */
+ if (ret == ws_count)
+ return count;
+
+ /* only part of the string was consumed */
+ /* return count can not include trailing space */
+ return leading + ret;
}