Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On 7/28/05, Jon Smirl <[EMAIL PROTECTED]> wrote:
> Even simpler version
>
> --
> Jon Smirl
> [EMAIL PROTECTED]
>
> Remove leading and trailing whitespace when text sysfs attribute is set
> signed-off-by: Jon Smirl <[EMAIL PROTECTED]>
>
> 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
> #include
> #include
> +#include
> #include
> #include
>
> @@ -207,8 +208,23 @@ 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;
> + char *x;
>
> - return ops->store(kobj,attr,buffer->page,count);
> + /* locate trailing white space */
> + while ((count > 0) && isspace(buffer->page[count - 1]))
> + count--;
> +
> + /* locate leading white space */
> + x = buffer->page;
> + if (count > 0) {
> + while (isspace(*x))
> + x++;
> + count -= (x - buffer->page);
> + }
> + /* terminate the string */
> + x[count] = '\0';
Should we add a check for a NULL string here? It seems not all
drivers were prepared to handle a zero length store().
If (count == 0)
return 0;
> +
> + return ops->store(kobj, attr, x, count);
> }
>
--
Jon Smirl
[EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On 7/28/05, Jon Smirl [EMAIL PROTECTED] wrote:
Even simpler version
--
Jon Smirl
[EMAIL PROTECTED]
Remove leading and trailing whitespace when text sysfs attribute is set
signed-off-by: Jon Smirl [EMAIL PROTECTED]
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,23 @@ 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;
+ char *x;
- return ops-store(kobj,attr,buffer-page,count);
+ /* locate trailing white space */
+ while ((count 0) isspace(buffer-page[count - 1]))
+ count--;
+
+ /* locate leading white space */
+ x = buffer-page;
+ if (count 0) {
+ while (isspace(*x))
+ x++;
+ count -= (x - buffer-page);
+ }
+ /* terminate the string */
+ x[count] = '\0';
Should we add a check for a NULL string here? It seems not all
drivers were prepared to handle a zero length store().
If (count == 0)
return 0;
+
+ return ops-store(kobj, attr, x, count);
}
--
Jon Smirl
[EMAIL PROTECTED]
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Hi! > > > It is not a work around. These are text attributes meant for human > > > use. Humans have a hard time cleaning up things they can't see. And > > > the failure mode for this is awful, your attribute won't set but > > > everything on the screen looks fine. > > > > Kernel is not a place to be user friendly. Or do you propose stripping > > whitespace > > for open(), too? File called "foo.txt" certainly *is* going to be > > confusing, but it should be allowed at kernel level. > > open is not made for human use, it is used by programs and only > indirectly by humans. sysfs variables are used by directly humans. Both are kernel interfaces; I can use open() by hand just fine... > > Now... echo foo > /sys/var does not properly report errors. Thats bad, but > > it needs to > > be fixed in bash. > > It is going to take a lot more code to return an error that a > parameter didn't match because of extra white space that it would take > to simply remove the whitespace. I believe we do correctly report errors in write() calls already. Bash not reporting them to the user is different problem. Pavel -- if you have sharp zaurus hardware you don't need... you know my address - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On 8/7/05, Pavel Machek <[EMAIL PROTECTED]> wrote: > Hi! > > > > > > Could you tell me why you don't just fail the operation if malformed > > > > > input is supplied? > > > > > > > > Leading/trailing white space should be allowed. For example echo > > > > appends '\n' unless you know to use -n. It is easier to fix the kernel > > > > than to teach everyone to use -n. > > > > > > Please, NO! echo -n is the right thing to do, and users will eventually > > > learn. > > > We are not going to add such workarounds all over the kernel... > > > > It is not a work around. These are text attributes meant for human > > use. Humans have a hard time cleaning up things they can't see. And > > the failure mode for this is awful, your attribute won't set but > > everything on the screen looks fine. > > Kernel is not a place to be user friendly. Or do you propose stripping > whitespace > for open(), too? File called "foo.txt" certainly *is* going to be > confusing, but it should be allowed at kernel level. open is not made for human use, it is used by programs and only indirectly by humans. sysfs variables are used by directly humans. > > Now... echo foo > /sys/var does not properly report errors. Thats bad, but it > needs to > be fixed in bash. It is going to take a lot more code to return an error that a parameter didn't match because of extra white space that it would take to simply remove the whitespace. > -- > 64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms > > -- Jon Smirl [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Hi! > > > > Could you tell me why you don't just fail the operation if malformed > > > > input is supplied? > > > > > > Leading/trailing white space should be allowed. For example echo > > > appends '\n' unless you know to use -n. It is easier to fix the kernel > > > than to teach everyone to use -n. > > > > Please, NO! echo -n is the right thing to do, and users will eventually > > learn. > > We are not going to add such workarounds all over the kernel... > > It is not a work around. These are text attributes meant for human > use. Humans have a hard time cleaning up things they can't see. And > the failure mode for this is awful, your attribute won't set but > everything on the screen looks fine. Kernel is not a place to be user friendly. Or do you propose stripping whitespace for open(), too? File called "foo.txt" certainly *is* going to be confusing, but it should be allowed at kernel level. Now... echo foo > /sys/var does not properly report errors. Thats bad, but it needs to be fixed in bash. -- 64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Hi! > > > If we are going back to needing helper scripts then I should just > > > remove the entire sysfs graphics interface and switch back to using > > > ioctls and a helper app. Of could no one can ever find the helper app > > > or remember how it works. I thought one of the main reasons behind the > > > sysfs interface was to eliminate these helper apps. > > > > The point is that you _can_ do it with echo, not that it is _easy_. > > Nor is sysfs a solution in any case. > > > > > Without doing whitespace cleanup there is a 100% probability that this > > > will generate bug reports. I know this for a fact because I am already > > > getting them. > > > > Stupid users are not a reason for kernel bloat. > > You have a very wrapped sense of kernel bloat. This is nine lines of > code whose absence is guaranteed to generate a bunch of bug reports. > Not having it is also causing various implementers to implement > attribute processing differently. Some are stripping white space in > their implementations and some are not. If you want to attack kernel Can you point place where we do strip whitespace in kernel sysfs handlers? Pavel -- 64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Hi! If we are going back to needing helper scripts then I should just remove the entire sysfs graphics interface and switch back to using ioctls and a helper app. Of could no one can ever find the helper app or remember how it works. I thought one of the main reasons behind the sysfs interface was to eliminate these helper apps. The point is that you _can_ do it with echo, not that it is _easy_. Nor is sysfs a solution in any case. Without doing whitespace cleanup there is a 100% probability that this will generate bug reports. I know this for a fact because I am already getting them. Stupid users are not a reason for kernel bloat. You have a very wrapped sense of kernel bloat. This is nine lines of code whose absence is guaranteed to generate a bunch of bug reports. Not having it is also causing various implementers to implement attribute processing differently. Some are stripping white space in their implementations and some are not. If you want to attack kernel Can you point place where we do strip whitespace in kernel sysfs handlers? Pavel -- 64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Hi! Could you tell me why you don't just fail the operation if malformed input is supplied? Leading/trailing white space should be allowed. For example echo appends '\n' unless you know to use -n. It is easier to fix the kernel than to teach everyone to use -n. Please, NO! echo -n is the right thing to do, and users will eventually learn. We are not going to add such workarounds all over the kernel... It is not a work around. These are text attributes meant for human use. Humans have a hard time cleaning up things they can't see. And the failure mode for this is awful, your attribute won't set but everything on the screen looks fine. Kernel is not a place to be user friendly. Or do you propose stripping whitespace for open(), too? File called foo.txt certainly *is* going to be confusing, but it should be allowed at kernel level. Now... echo foo /sys/var does not properly report errors. Thats bad, but it needs to be fixed in bash. -- 64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On 8/7/05, Pavel Machek [EMAIL PROTECTED] wrote: Hi! Could you tell me why you don't just fail the operation if malformed input is supplied? Leading/trailing white space should be allowed. For example echo appends '\n' unless you know to use -n. It is easier to fix the kernel than to teach everyone to use -n. Please, NO! echo -n is the right thing to do, and users will eventually learn. We are not going to add such workarounds all over the kernel... It is not a work around. These are text attributes meant for human use. Humans have a hard time cleaning up things they can't see. And the failure mode for this is awful, your attribute won't set but everything on the screen looks fine. Kernel is not a place to be user friendly. Or do you propose stripping whitespace for open(), too? File called foo.txt certainly *is* going to be confusing, but it should be allowed at kernel level. open is not made for human use, it is used by programs and only indirectly by humans. sysfs variables are used by directly humans. Now... echo foo /sys/var does not properly report errors. Thats bad, but it needs to be fixed in bash. It is going to take a lot more code to return an error that a parameter didn't match because of extra white space that it would take to simply remove the whitespace. -- 64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms -- Jon Smirl [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Hi! It is not a work around. These are text attributes meant for human use. Humans have a hard time cleaning up things they can't see. And the failure mode for this is awful, your attribute won't set but everything on the screen looks fine. Kernel is not a place to be user friendly. Or do you propose stripping whitespace for open(), too? File called foo.txt certainly *is* going to be confusing, but it should be allowed at kernel level. open is not made for human use, it is used by programs and only indirectly by humans. sysfs variables are used by directly humans. Both are kernel interfaces; I can use open() by hand just fine... Now... echo foo /sys/var does not properly report errors. Thats bad, but it needs to be fixed in bash. It is going to take a lot more code to return an error that a parameter didn't match because of extra white space that it would take to simply remove the whitespace. I believe we do correctly report errors in write() calls already. Bash not reporting them to the user is different problem. Pavel -- if you have sharp zaurus hardware you don't need... you know my address - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
> > Stupid users are not a reason for kernel bloat. > > You have a very wrapped sense of kernel bloat. This is nine lines of > code whose absence is guaranteed to generate a bunch of bug reports. They are supposed to be present, but not in the kernel. > Not having it is also causing various implementers to implement > attribute processing differently. Some are stripping white space in > their implementations and some are not. If you want to attack kernel > bloat there are much more productive areas. If somebody is stripping whitespace in the kernel, find and destroy. > If whitespace cleanup is rejected I believe we should eliminate text > based sysfs attributes in general and make them all binary. I'll > probably remove the fbdev sysfs interface because I don't want to deal > with the bug reports reporting the same problem over and over. You are seeing this wrong. The problem is not binary vs. text. If binary were the problem we'd have a generic ioctl tool that takes arguments in any number format you'd want. The problem is untyped data. ASCII-strings without leading or trailing whitespece is a clearly defined datatype. Requiring a tool ensuring this format is met or care is used if other tools are used is perfectly in order. The kernel does not guess what an input is supposed to mean. Anything else is bloat. Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Stupid users are not a reason for kernel bloat. You have a very wrapped sense of kernel bloat. This is nine lines of code whose absence is guaranteed to generate a bunch of bug reports. They are supposed to be present, but not in the kernel. Not having it is also causing various implementers to implement attribute processing differently. Some are stripping white space in their implementations and some are not. If you want to attack kernel bloat there are much more productive areas. If somebody is stripping whitespace in the kernel, find and destroy. If whitespace cleanup is rejected I believe we should eliminate text based sysfs attributes in general and make them all binary. I'll probably remove the fbdev sysfs interface because I don't want to deal with the bug reports reporting the same problem over and over. You are seeing this wrong. The problem is not binary vs. text. If binary were the problem we'd have a generic ioctl tool that takes arguments in any number format you'd want. The problem is untyped data. ASCII-strings without leading or trailing whitespece is a clearly defined datatype. Requiring a tool ensuring this format is met or care is used if other tools are used is perfectly in order. The kernel does not guess what an input is supposed to mean. Anything else is bloat. Regards Oliver - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On 8/5/05, Greg KH <[EMAIL PROTECTED]> wrote: > On Fri, Jul 29, 2005 at 02:50:44PM -0400, Jon Smirl wrote: > > Greg, is this ok for your tree now or does it need more work? > > It's in my queue, will add it to the tree next week. Sorry for the > delay, was at OSCON this week... Glad to see this. After it lands in the tree I'll fix up about ten places where I have attribute processing wrong because of this. Is there some place in the sysfs doc that this can be mentioned? Another thing that should be cleared up is if the attributes are described by length or zero termination. Right now they get both. I suspect the right answer here is supposed to be by length. -- Jon Smirl [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On Fri, Jul 29, 2005 at 02:50:44PM -0400, Jon Smirl wrote: > Greg, is this ok for your tree now or does it need more work? It's in my queue, will add it to the tree next week. Sorry for the delay, was at OSCON this week... thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On Tue, Jan 01, 2002 at 08:53:39AM +0100, Pavel Machek wrote: > Hi! > > > > > New, simplified version of the sysfs whitespace strip patch... > > > > > > Could you tell me why you don't just fail the operation if malformed > > > input is supplied? > > > > Leading/trailing white space should be allowed. For example echo > > appends '\n' unless you know to use -n. It is easier to fix the kernel > > than to teach everyone to use -n. > > Please, NO! echo -n is the right thing to do, and users will eventually learn. > We are not going to add such workarounds all over the kernel... Ahhh, this would be so much easier if people just got used to using printf instead of echo when doing text output. =) Regards: David Weinehall -- /) David Weinehall <[EMAIL PROTECTED]> /) Northern lights wander (\ // Maintainer of the v2.0 kernel // Dance across the winter sky // \) http://www.acc.umu.se/~tao/(/ Full colour fire (/ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On 8/5/05, Oliver Neukum <[EMAIL PROTECTED]> wrote: > Am Freitag, 5. August 2005 20:47 schrieb Jon Smirl: > > On 8/5/05, Oliver Neukum <[EMAIL PROTECTED]> wrote: > > > Am Freitag, 5. August 2005 20:14 schrieb Jon Smirl: > > > > On 8/5/05, Oliver Neukum <[EMAIL PROTECTED]> wrote: > > > > > Am Freitag, 5. August 2005 15:32 schrieb Jon Smirl: > > > > > > On 1/1/02, Pavel Machek <[EMAIL PROTECTED]> wrote: > > > > > > > Hi! > > > > > > > > > > > > > > > > > New, simplified version of the sysfs whitespace strip > > > > > > > > > > patch... > > > > > > > > > > > > > > > > > > Could you tell me why you don't just fail the operation if > > > > > > > > > malformed > > > > > > > > > input is supplied? > > > > > > > > > > > > > > > > Leading/trailing white space should be allowed. For example echo > > > > > > > > appends '\n' unless you know to use -n. It is easier to fix the > > > > > > > > kernel > > > > > > > > than to teach everyone to use -n. > > > > > > > > > > > > > > Please, NO! echo -n is the right thing to do, and users will > > > > > > > eventually learn. > > > > > > > We are not going to add such workarounds all over the kernel... > > > > > > > > > > > > It is not a work around. These are text attributes meant for human > > > > > > use. Humans have a hard time cleaning up things they can't see. And > > > > > > the failure mode for this is awful, your attribute won't set but > > > > > > everything on the screen looks fine. > > > > > > > > > > The average user has no place poking sysfs. Root should know when > > > > > to use -n, as should shell scripts. > > > > > > > > So the average user never needs to change their console mode? Check > > > > out /sys/class/graphics/fb/modes and mode. > > > > > > That is what we have helper scripts for. > > > > If we are going back to needing helper scripts then I should just > > remove the entire sysfs graphics interface and switch back to using > > ioctls and a helper app. Of could no one can ever find the helper app > > or remember how it works. I thought one of the main reasons behind the > > sysfs interface was to eliminate these helper apps. > > The point is that you _can_ do it with echo, not that it is _easy_. > Nor is sysfs a solution in any case. > > > Without doing whitespace cleanup there is a 100% probability that this > > will generate bug reports. I know this for a fact because I am already > > getting them. > > Stupid users are not a reason for kernel bloat. You have a very wrapped sense of kernel bloat. This is nine lines of code whose absence is guaranteed to generate a bunch of bug reports. Not having it is also causing various implementers to implement attribute processing differently. Some are stripping white space in their implementations and some are not. If you want to attack kernel bloat there are much more productive areas. If whitespace cleanup is rejected I believe we should eliminate text based sysfs attributes in general and make them all binary. I'll probably remove the fbdev sysfs interface because I don't want to deal with the bug reports reporting the same problem over and over. -- Jon Smirl [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Am Freitag, 5. August 2005 20:47 schrieb Jon Smirl: > On 8/5/05, Oliver Neukum <[EMAIL PROTECTED]> wrote: > > Am Freitag, 5. August 2005 20:14 schrieb Jon Smirl: > > > On 8/5/05, Oliver Neukum <[EMAIL PROTECTED]> wrote: > > > > Am Freitag, 5. August 2005 15:32 schrieb Jon Smirl: > > > > > On 1/1/02, Pavel Machek <[EMAIL PROTECTED]> wrote: > > > > > > Hi! > > > > > > > > > > > > > > > New, simplified version of the sysfs whitespace strip patch... > > > > > > > > > > > > > > > > Could you tell me why you don't just fail the operation if > > > > > > > > malformed > > > > > > > > input is supplied? > > > > > > > > > > > > > > Leading/trailing white space should be allowed. For example echo > > > > > > > appends '\n' unless you know to use -n. It is easier to fix the > > > > > > > kernel > > > > > > > than to teach everyone to use -n. > > > > > > > > > > > > Please, NO! echo -n is the right thing to do, and users will > > > > > > eventually learn. > > > > > > We are not going to add such workarounds all over the kernel... > > > > > > > > > > It is not a work around. These are text attributes meant for human > > > > > use. Humans have a hard time cleaning up things they can't see. And > > > > > the failure mode for this is awful, your attribute won't set but > > > > > everything on the screen looks fine. > > > > > > > > The average user has no place poking sysfs. Root should know when > > > > to use -n, as should shell scripts. > > > > > > So the average user never needs to change their console mode? Check > > > out /sys/class/graphics/fb/modes and mode. > > > > That is what we have helper scripts for. > > If we are going back to needing helper scripts then I should just > remove the entire sysfs graphics interface and switch back to using > ioctls and a helper app. Of could no one can ever find the helper app > or remember how it works. I thought one of the main reasons behind the > sysfs interface was to eliminate these helper apps. The point is that you _can_ do it with echo, not that it is _easy_. Nor is sysfs a solution in any case. > Without doing whitespace cleanup there is a 100% probability that this > will generate bug reports. I know this for a fact because I am already > getting them. Stupid users are not a reason for kernel bloat. Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On 8/5/05, Oliver Neukum <[EMAIL PROTECTED]> wrote: > Am Freitag, 5. August 2005 20:14 schrieb Jon Smirl: > > On 8/5/05, Oliver Neukum <[EMAIL PROTECTED]> wrote: > > > Am Freitag, 5. August 2005 15:32 schrieb Jon Smirl: > > > > On 1/1/02, Pavel Machek <[EMAIL PROTECTED]> wrote: > > > > > Hi! > > > > > > > > > > > > > New, simplified version of the sysfs whitespace strip patch... > > > > > > > > > > > > > > Could you tell me why you don't just fail the operation if > > > > > > > malformed > > > > > > > input is supplied? > > > > > > > > > > > > Leading/trailing white space should be allowed. For example echo > > > > > > appends '\n' unless you know to use -n. It is easier to fix the > > > > > > kernel > > > > > > than to teach everyone to use -n. > > > > > > > > > > Please, NO! echo -n is the right thing to do, and users will > > > > > eventually learn. > > > > > We are not going to add such workarounds all over the kernel... > > > > > > > > It is not a work around. These are text attributes meant for human > > > > use. Humans have a hard time cleaning up things they can't see. And > > > > the failure mode for this is awful, your attribute won't set but > > > > everything on the screen looks fine. > > > > > > The average user has no place poking sysfs. Root should know when > > > to use -n, as should shell scripts. > > > > So the average user never needs to change their console mode? Check > > out /sys/class/graphics/fb/modes and mode. > > That is what we have helper scripts for. If we are going back to needing helper scripts then I should just remove the entire sysfs graphics interface and switch back to using ioctls and a helper app. Of could no one can ever find the helper app or remember how it works. I thought one of the main reasons behind the sysfs interface was to eliminate these helper apps. Without doing whitespace cleanup there is a 100% probability that this will generate bug reports. I know this for a fact because I am already getting them. -- Jon Smirl [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Am Freitag, 5. August 2005 20:14 schrieb Jon Smirl: > On 8/5/05, Oliver Neukum <[EMAIL PROTECTED]> wrote: > > Am Freitag, 5. August 2005 15:32 schrieb Jon Smirl: > > > On 1/1/02, Pavel Machek <[EMAIL PROTECTED]> wrote: > > > > Hi! > > > > > > > > > > > New, simplified version of the sysfs whitespace strip patch... > > > > > > > > > > > > Could you tell me why you don't just fail the operation if malformed > > > > > > input is supplied? > > > > > > > > > > Leading/trailing white space should be allowed. For example echo > > > > > appends '\n' unless you know to use -n. It is easier to fix the kernel > > > > > than to teach everyone to use -n. > > > > > > > > Please, NO! echo -n is the right thing to do, and users will eventually > > > > learn. > > > > We are not going to add such workarounds all over the kernel... > > > > > > It is not a work around. These are text attributes meant for human > > > use. Humans have a hard time cleaning up things they can't see. And > > > the failure mode for this is awful, your attribute won't set but > > > everything on the screen looks fine. > > > > The average user has no place poking sysfs. Root should know when > > to use -n, as should shell scripts. > > So the average user never needs to change their console mode? Check > out /sys/class/graphics/fb/modes and mode. That is what we have helper scripts for. Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On 8/5/05, Oliver Neukum <[EMAIL PROTECTED]> wrote: > Am Freitag, 5. August 2005 15:32 schrieb Jon Smirl: > > On 1/1/02, Pavel Machek <[EMAIL PROTECTED]> wrote: > > > Hi! > > > > > > > > > New, simplified version of the sysfs whitespace strip patch... > > > > > > > > > > Could you tell me why you don't just fail the operation if malformed > > > > > input is supplied? > > > > > > > > Leading/trailing white space should be allowed. For example echo > > > > appends '\n' unless you know to use -n. It is easier to fix the kernel > > > > than to teach everyone to use -n. > > > > > > Please, NO! echo -n is the right thing to do, and users will eventually > > > learn. > > > We are not going to add such workarounds all over the kernel... > > > > It is not a work around. These are text attributes meant for human > > use. Humans have a hard time cleaning up things they can't see. And > > the failure mode for this is awful, your attribute won't set but > > everything on the screen looks fine. > > The average user has no place poking sysfs. Root should know when > to use -n, as should shell scripts. So the average user never needs to change their console mode? Check out /sys/class/graphics/fb/modes and mode. > > Regards > Oliver > -- Jon Smirl [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Am Freitag, 5. August 2005 15:32 schrieb Jon Smirl: > On 1/1/02, Pavel Machek <[EMAIL PROTECTED]> wrote: > > Hi! > > > > > > > New, simplified version of the sysfs whitespace strip patch... > > > > > > > > Could you tell me why you don't just fail the operation if malformed > > > > input is supplied? > > > > > > Leading/trailing white space should be allowed. For example echo > > > appends '\n' unless you know to use -n. It is easier to fix the kernel > > > than to teach everyone to use -n. > > > > Please, NO! echo -n is the right thing to do, and users will eventually > > learn. > > We are not going to add such workarounds all over the kernel... > > It is not a work around. These are text attributes meant for human > use. Humans have a hard time cleaning up things they can't see. And > the failure mode for this is awful, your attribute won't set but > everything on the screen looks fine. The average user has no place poking sysfs. Root should know when to use -n, as should shell scripts. Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On 1/1/02, Pavel Machek <[EMAIL PROTECTED]> wrote: > Hi! > > > > > New, simplified version of the sysfs whitespace strip patch... > > > > > > Could you tell me why you don't just fail the operation if malformed > > > input is supplied? > > > > Leading/trailing white space should be allowed. For example echo > > appends '\n' unless you know to use -n. It is easier to fix the kernel > > than to teach everyone to use -n. > > Please, NO! echo -n is the right thing to do, and users will eventually learn. > We are not going to add such workarounds all over the kernel... It is not a work around. These are text attributes meant for human use. Humans have a hard time cleaning up things they can't see. And the failure mode for this is awful, your attribute won't set but everything on the screen looks fine. echo is not the only source of problems. I had some trailing whitespace in a shell script and it took me a couple of hours to discover why the attribute set was failing. I finally had to add debug code to the kernel and reboot to located the problem. Normal users are never going to figure this out. Binary attributes are for program use, they should not get cleaned up. If you dont want the whitespace cleaning switch to a binary one. > Pavel > -- > 64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms > > -- Jon Smirl [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Hi! > > > New, simplified version of the sysfs whitespace strip patch... > > > > Could you tell me why you don't just fail the operation if malformed > > input is supplied? > > Leading/trailing white space should be allowed. For example echo > appends '\n' unless you know to use -n. It is easier to fix the kernel > than to teach everyone to use -n. Please, NO! echo -n is the right thing to do, and users will eventually learn. We are not going to add such workarounds all over the kernel... Pavel -- 64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Hi! New, simplified version of the sysfs whitespace strip patch... Could you tell me why you don't just fail the operation if malformed input is supplied? Leading/trailing white space should be allowed. For example echo appends '\n' unless you know to use -n. It is easier to fix the kernel than to teach everyone to use -n. Please, NO! echo -n is the right thing to do, and users will eventually learn. We are not going to add such workarounds all over the kernel... Pavel -- 64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On 1/1/02, Pavel Machek [EMAIL PROTECTED] wrote: Hi! New, simplified version of the sysfs whitespace strip patch... Could you tell me why you don't just fail the operation if malformed input is supplied? Leading/trailing white space should be allowed. For example echo appends '\n' unless you know to use -n. It is easier to fix the kernel than to teach everyone to use -n. Please, NO! echo -n is the right thing to do, and users will eventually learn. We are not going to add such workarounds all over the kernel... It is not a work around. These are text attributes meant for human use. Humans have a hard time cleaning up things they can't see. And the failure mode for this is awful, your attribute won't set but everything on the screen looks fine. echo is not the only source of problems. I had some trailing whitespace in a shell script and it took me a couple of hours to discover why the attribute set was failing. I finally had to add debug code to the kernel and reboot to located the problem. Normal users are never going to figure this out. Binary attributes are for program use, they should not get cleaned up. If you dont want the whitespace cleaning switch to a binary one. Pavel -- 64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms -- Jon Smirl [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Am Freitag, 5. August 2005 15:32 schrieb Jon Smirl: On 1/1/02, Pavel Machek [EMAIL PROTECTED] wrote: Hi! New, simplified version of the sysfs whitespace strip patch... Could you tell me why you don't just fail the operation if malformed input is supplied? Leading/trailing white space should be allowed. For example echo appends '\n' unless you know to use -n. It is easier to fix the kernel than to teach everyone to use -n. Please, NO! echo -n is the right thing to do, and users will eventually learn. We are not going to add such workarounds all over the kernel... It is not a work around. These are text attributes meant for human use. Humans have a hard time cleaning up things they can't see. And the failure mode for this is awful, your attribute won't set but everything on the screen looks fine. The average user has no place poking sysfs. Root should know when to use -n, as should shell scripts. Regards Oliver - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On 8/5/05, Oliver Neukum [EMAIL PROTECTED] wrote: Am Freitag, 5. August 2005 15:32 schrieb Jon Smirl: On 1/1/02, Pavel Machek [EMAIL PROTECTED] wrote: Hi! New, simplified version of the sysfs whitespace strip patch... Could you tell me why you don't just fail the operation if malformed input is supplied? Leading/trailing white space should be allowed. For example echo appends '\n' unless you know to use -n. It is easier to fix the kernel than to teach everyone to use -n. Please, NO! echo -n is the right thing to do, and users will eventually learn. We are not going to add such workarounds all over the kernel... It is not a work around. These are text attributes meant for human use. Humans have a hard time cleaning up things they can't see. And the failure mode for this is awful, your attribute won't set but everything on the screen looks fine. The average user has no place poking sysfs. Root should know when to use -n, as should shell scripts. So the average user never needs to change their console mode? Check out /sys/class/graphics/fb/modes and mode. Regards Oliver -- Jon Smirl [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Am Freitag, 5. August 2005 20:14 schrieb Jon Smirl: On 8/5/05, Oliver Neukum [EMAIL PROTECTED] wrote: Am Freitag, 5. August 2005 15:32 schrieb Jon Smirl: On 1/1/02, Pavel Machek [EMAIL PROTECTED] wrote: Hi! New, simplified version of the sysfs whitespace strip patch... Could you tell me why you don't just fail the operation if malformed input is supplied? Leading/trailing white space should be allowed. For example echo appends '\n' unless you know to use -n. It is easier to fix the kernel than to teach everyone to use -n. Please, NO! echo -n is the right thing to do, and users will eventually learn. We are not going to add such workarounds all over the kernel... It is not a work around. These are text attributes meant for human use. Humans have a hard time cleaning up things they can't see. And the failure mode for this is awful, your attribute won't set but everything on the screen looks fine. The average user has no place poking sysfs. Root should know when to use -n, as should shell scripts. So the average user never needs to change their console mode? Check out /sys/class/graphics/fb/modes and mode. That is what we have helper scripts for. Regards Oliver - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On 8/5/05, Oliver Neukum [EMAIL PROTECTED] wrote: Am Freitag, 5. August 2005 20:14 schrieb Jon Smirl: On 8/5/05, Oliver Neukum [EMAIL PROTECTED] wrote: Am Freitag, 5. August 2005 15:32 schrieb Jon Smirl: On 1/1/02, Pavel Machek [EMAIL PROTECTED] wrote: Hi! New, simplified version of the sysfs whitespace strip patch... Could you tell me why you don't just fail the operation if malformed input is supplied? Leading/trailing white space should be allowed. For example echo appends '\n' unless you know to use -n. It is easier to fix the kernel than to teach everyone to use -n. Please, NO! echo -n is the right thing to do, and users will eventually learn. We are not going to add such workarounds all over the kernel... It is not a work around. These are text attributes meant for human use. Humans have a hard time cleaning up things they can't see. And the failure mode for this is awful, your attribute won't set but everything on the screen looks fine. The average user has no place poking sysfs. Root should know when to use -n, as should shell scripts. So the average user never needs to change their console mode? Check out /sys/class/graphics/fb/modes and mode. That is what we have helper scripts for. If we are going back to needing helper scripts then I should just remove the entire sysfs graphics interface and switch back to using ioctls and a helper app. Of could no one can ever find the helper app or remember how it works. I thought one of the main reasons behind the sysfs interface was to eliminate these helper apps. Without doing whitespace cleanup there is a 100% probability that this will generate bug reports. I know this for a fact because I am already getting them. -- Jon Smirl [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Am Freitag, 5. August 2005 20:47 schrieb Jon Smirl: On 8/5/05, Oliver Neukum [EMAIL PROTECTED] wrote: Am Freitag, 5. August 2005 20:14 schrieb Jon Smirl: On 8/5/05, Oliver Neukum [EMAIL PROTECTED] wrote: Am Freitag, 5. August 2005 15:32 schrieb Jon Smirl: On 1/1/02, Pavel Machek [EMAIL PROTECTED] wrote: Hi! New, simplified version of the sysfs whitespace strip patch... Could you tell me why you don't just fail the operation if malformed input is supplied? Leading/trailing white space should be allowed. For example echo appends '\n' unless you know to use -n. It is easier to fix the kernel than to teach everyone to use -n. Please, NO! echo -n is the right thing to do, and users will eventually learn. We are not going to add such workarounds all over the kernel... It is not a work around. These are text attributes meant for human use. Humans have a hard time cleaning up things they can't see. And the failure mode for this is awful, your attribute won't set but everything on the screen looks fine. The average user has no place poking sysfs. Root should know when to use -n, as should shell scripts. So the average user never needs to change their console mode? Check out /sys/class/graphics/fb/modes and mode. That is what we have helper scripts for. If we are going back to needing helper scripts then I should just remove the entire sysfs graphics interface and switch back to using ioctls and a helper app. Of could no one can ever find the helper app or remember how it works. I thought one of the main reasons behind the sysfs interface was to eliminate these helper apps. The point is that you _can_ do it with echo, not that it is _easy_. Nor is sysfs a solution in any case. Without doing whitespace cleanup there is a 100% probability that this will generate bug reports. I know this for a fact because I am already getting them. Stupid users are not a reason for kernel bloat. Regards Oliver - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On 8/5/05, Oliver Neukum [EMAIL PROTECTED] wrote: Am Freitag, 5. August 2005 20:47 schrieb Jon Smirl: On 8/5/05, Oliver Neukum [EMAIL PROTECTED] wrote: Am Freitag, 5. August 2005 20:14 schrieb Jon Smirl: On 8/5/05, Oliver Neukum [EMAIL PROTECTED] wrote: Am Freitag, 5. August 2005 15:32 schrieb Jon Smirl: On 1/1/02, Pavel Machek [EMAIL PROTECTED] wrote: Hi! New, simplified version of the sysfs whitespace strip patch... Could you tell me why you don't just fail the operation if malformed input is supplied? Leading/trailing white space should be allowed. For example echo appends '\n' unless you know to use -n. It is easier to fix the kernel than to teach everyone to use -n. Please, NO! echo -n is the right thing to do, and users will eventually learn. We are not going to add such workarounds all over the kernel... It is not a work around. These are text attributes meant for human use. Humans have a hard time cleaning up things they can't see. And the failure mode for this is awful, your attribute won't set but everything on the screen looks fine. The average user has no place poking sysfs. Root should know when to use -n, as should shell scripts. So the average user never needs to change their console mode? Check out /sys/class/graphics/fb/modes and mode. That is what we have helper scripts for. If we are going back to needing helper scripts then I should just remove the entire sysfs graphics interface and switch back to using ioctls and a helper app. Of could no one can ever find the helper app or remember how it works. I thought one of the main reasons behind the sysfs interface was to eliminate these helper apps. The point is that you _can_ do it with echo, not that it is _easy_. Nor is sysfs a solution in any case. Without doing whitespace cleanup there is a 100% probability that this will generate bug reports. I know this for a fact because I am already getting them. Stupid users are not a reason for kernel bloat. You have a very wrapped sense of kernel bloat. This is nine lines of code whose absence is guaranteed to generate a bunch of bug reports. Not having it is also causing various implementers to implement attribute processing differently. Some are stripping white space in their implementations and some are not. If you want to attack kernel bloat there are much more productive areas. If whitespace cleanup is rejected I believe we should eliminate text based sysfs attributes in general and make them all binary. I'll probably remove the fbdev sysfs interface because I don't want to deal with the bug reports reporting the same problem over and over. -- Jon Smirl [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On Tue, Jan 01, 2002 at 08:53:39AM +0100, Pavel Machek wrote: Hi! New, simplified version of the sysfs whitespace strip patch... Could you tell me why you don't just fail the operation if malformed input is supplied? Leading/trailing white space should be allowed. For example echo appends '\n' unless you know to use -n. It is easier to fix the kernel than to teach everyone to use -n. Please, NO! echo -n is the right thing to do, and users will eventually learn. We are not going to add such workarounds all over the kernel... Ahhh, this would be so much easier if people just got used to using printf instead of echo when doing text output. =) Regards: David Weinehall -- /) David Weinehall [EMAIL PROTECTED] /) Northern lights wander (\ // Maintainer of the v2.0 kernel // Dance across the winter sky // \) http://www.acc.umu.se/~tao/(/ Full colour fire (/ - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On Fri, Jul 29, 2005 at 02:50:44PM -0400, Jon Smirl wrote: Greg, is this ok for your tree now or does it need more work? It's in my queue, will add it to the tree next week. Sorry for the delay, was at OSCON this week... thanks, greg k-h - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On 8/5/05, Greg KH [EMAIL PROTECTED] wrote: On Fri, Jul 29, 2005 at 02:50:44PM -0400, Jon Smirl wrote: Greg, is this ok for your tree now or does it need more work? It's in my queue, will add it to the tree next week. Sorry for the delay, was at OSCON this week... Glad to see this. After it lands in the tree I'll fix up about ten places where I have attribute processing wrong because of this. Is there some place in the sysfs doc that this can be mentioned? Another thing that should be cleared up is if the attributes are described by length or zero termination. Right now they get both. I suspect the right answer here is supposed to be by length. -- Jon Smirl [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Greg, is this ok for your tree now or does it need more work?
On 7/28/05, Jon Smirl <[EMAIL PROTECTED]> wrote:
> Even simpler version
>
> --
> Jon Smirl
> [EMAIL PROTECTED]
>
> Remove leading and trailing whitespace when text sysfs attribute is set
> signed-off-by: Jon Smirl <[EMAIL PROTECTED]>
>
> 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
> #include
> #include
> +#include
> #include
> #include
>
> @@ -207,8 +208,23 @@ 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;
> + char *x;
>
> - return ops->store(kobj,attr,buffer->page,count);
> + /* locate trailing white space */
> + while ((count > 0) && isspace(buffer->page[count - 1]))
> + count--;
> +
> + /* locate leading white space */
> + x = buffer->page;
> + if (count > 0) {
> + while (isspace(*x))
> + x++;
> + count -= (x - buffer->page);
> + }
> + /* terminate the string */
> + x[count] = '\0';
> +
> + return ops->store(kobj, attr, x, count);
> }
>
--
Jon Smirl
[EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Greg, is this ok for your tree now or does it need more work?
On 7/28/05, Jon Smirl [EMAIL PROTECTED] wrote:
Even simpler version
--
Jon Smirl
[EMAIL PROTECTED]
Remove leading and trailing whitespace when text sysfs attribute is set
signed-off-by: Jon Smirl [EMAIL PROTECTED]
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,23 @@ 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;
+ char *x;
- return ops-store(kobj,attr,buffer-page,count);
+ /* locate trailing white space */
+ while ((count 0) isspace(buffer-page[count - 1]))
+ count--;
+
+ /* locate leading white space */
+ x = buffer-page;
+ if (count 0) {
+ while (isspace(*x))
+ x++;
+ count -= (x - buffer-page);
+ }
+ /* terminate the string */
+ x[count] = '\0';
+
+ return ops-store(kobj, attr, x, count);
}
--
Jon Smirl
[EMAIL PROTECTED]
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Am Donnerstag, 28. Juli 2005 21:57 schrieb Jon Smirl: > New, simplified version of the sysfs whitespace strip patch... Could you tell me why you don't just fail the operation if malformed input is supplied? Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Am Donnerstag, 28. Juli 2005 23:12 schrieb Jon Smirl: > On 7/28/05, Oliver Neukum <[EMAIL PROTECTED]> wrote: > > Am Donnerstag, 28. Juli 2005 21:57 schrieb Jon Smirl: > > > New, simplified version of the sysfs whitespace strip patch... > > > > Could you tell me why you don't just fail the operation if malformed > > input is supplied? > > Leading/trailing white space should be allowed. For example echo > appends '\n' unless you know to use -n. It is easier to fix the kernel > than to teach everyone to use -n. Why? If you can do it in user space, do so. Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On 7/28/05, Oliver Neukum <[EMAIL PROTECTED]> wrote: > Am Donnerstag, 28. Juli 2005 21:57 schrieb Jon Smirl: > > New, simplified version of the sysfs whitespace strip patch... > > Could you tell me why you don't just fail the operation if malformed > input is supplied? Leading/trailing white space should be allowed. For example echo appends '\n' unless you know to use -n. It is easier to fix the kernel than to teach everyone to use -n. > > Regards > Oliver > -- Jon Smirl [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Still one nitpick: Jon Smirl wrote: > + while (isspace(*x) && (x - buffer->page < count)) > + x++; I think you can just do: if (count > 0) while (isspace(*x)) x++; If the passed-in string was fully whitespace then the trailing-whitespace stripping would already reduce it to count==0. So as long as you check for that case you can be sure that the while() will stop before x falls off the end of the string. Other than that it looks fine to me now. -Mitch - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Even simpler version
--
Jon Smirl
[EMAIL PROTECTED]
Remove leading and trailing whitespace when text sysfs attribute is set
signed-off-by: Jon Smirl <[EMAIL PROTECTED]>
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
#include
#include
+#include
#include
#include
@@ -207,8 +208,23 @@ 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;
+ char *x;
- return ops->store(kobj,attr,buffer->page,count);
+ /* locate trailing white space */
+ while ((count > 0) && isspace(buffer->page[count - 1]))
+ count--;
+
+ /* locate leading white space */
+ x = buffer->page;
+ if (count > 0) {
+ while (isspace(*x))
+ x++;
+ count -= (x - buffer->page);
+ }
+ /* terminate the string */
+ x[count] = '\0';
+
+ return ops->store(kobj, attr, x, count);
}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
New, simplified version of the sysfs whitespace strip patch... -- Jon Smirl [EMAIL PROTECTED] Remove leading and trailing whitespace when text sysfs attribute is set signed-off-by: Jon Smirl <[EMAIL PROTECTED]> 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 #include #include +#include #include #include @@ -207,8 +208,22 @@ 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; + char *x; - return ops->store(kobj,attr,buffer->page,count); + /* locate trailing white space */ + while ((count > 0) && isspace(buffer->page[count - 1])) + count--; + + /* locate leading white space */ + x = buffer->page; + while (isspace(*x) && (x - buffer->page < count)) + x++; + count -= (x - buffer->page); + + /* terminate the string */ + x[count] = '\0'; + + return ops->store(kobj, attr, x, count); } - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On Thu, Jul 28, 2005 at 08:54:53AM -0400, Jon Smirl wrote:
> On 7/28/05, Mitchell Blank Jr <[EMAIL PROTECTED]> wrote:
> > Greg KH wrote:
> > > > + /* locate trailng white space */
> > > > + z = y = x;
> > > > + while (y - buffer->page < count) {
> > > > + y++;
> > > > + z = y;
> > > > + while (isspace(*y) && (y - buffer->page < count)) {
> > > > + y++;
> > > > + }
> > > > + }
> > > > + count = z - x;
> > >
> > > Hm, I _think_ this works, but I need someone else to verify this...
> > > Anyone else?
> >
> > It looks sane-ish to me, but also more complicated than need be. Why can't
> > you just do something like:
> >
> > while (count > 0 && isspace(x[count - 1]))
> > count--;
> >
> > -Mitch
> >
>
> Do we need to deal with UTF8 here? I did the forward loop because you
> can't parse UTF8 backwards. If UTF8 is possible I need to change the
> pointer inc function.
No, we don't need to handle UTF8. Or if we do, then we better not make
a general filter function, as it will be a mess.
thanks,
greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Jon Smirl wrote:
> Do we need to deal with UTF8 here? I did the forward loop because you
> can't parse UTF8 backwards. If UTF8 is possible I need to change the
> pointer inc function.
As others have mentioned there shouldn't be a UTF8 problem with isspace().
However, even if you wanted to scan going forwards you can do it without
the complexity of a nested loop:
char *real_end;
for (real_end = x; x - buffer->page < count; x++)
if (!isspace(*x))
real_end = x + 1;
*real_end = '\0';
// then fix "count"
BTW, my code I posted yesterday is incomplete since I neglected to notice
that the "count = z - x" at the end is used to repair "count" damage from
both the leading and trailing whitespace stripping. Its actually easier
to strip the trailing whitespace first, like:
while (count > 0 && isspace(buffer->page[count - 1]))
count--;/* strip trailing whitespace */
if (count > 0 && isspace(buffer->page[0])) {
/*
* We have leading whitespace but also at least one
* non-whitespace character
*/
const char *x = buffer->page;
do {
x++;
count--;
} while (isspace(*x));
memmove(buffer->page, x, count);
}
buffer->page[count] = '\0';
-Mitch
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Jon Smirl wrote: On 7/28/05, Mitchell Blank Jr <[EMAIL PROTECTED]> wrote: [...] It looks sane-ish to me, but also more complicated than need be. Why can't you just do something like: while (count > 0 && isspace(x[count - 1])) count--; Do we need to deal with UTF8 here? I did the forward loop because you can't parse UTF8 backwards. If UTF8 is possible I need to change the pointer inc function. I don't think it matters here. Even with UTF8, any char that makes isspace return true, can't be part of a multi-byte char, as every byte in a multi-byte char in UTF8 has the MSB set, i.e., >= 0x80. -- Paulo Marques - www.grupopie.com It is a mistake to think you can solve any major problems just with potatoes. Douglas Adams - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Am Donnerstag, 28. Juli 2005 14:54 schrieb Jon Smirl:
> On 7/28/05, Mitchell Blank Jr <[EMAIL PROTECTED]> wrote:
> > Greg KH wrote:
> > > > + /* locate trailng white space */
> > > > + z = y = x;
> > > > + while (y - buffer->page < count) {
> > > > + y++;
> > > > + z = y;
> > > > + while (isspace(*y) && (y - buffer->page < count)) {
> > > > + y++;
> > > > + }
> > > > + }
> > > > + count = z - x;
> > >
> > > Hm, I _think_ this works, but I need someone else to verify this...
> > > Anyone else?
> >
> > It looks sane-ish to me, but also more complicated than need be. Why can't
> > you just do something like:
> >
> > while (count > 0 && isspace(x[count - 1]))
> > count--;
> >
> > -Mitch
> >
>
> Do we need to deal with UTF8 here? I did the forward loop because you
> can't parse UTF8 backwards. If UTF8 is possible I need to change the
> pointer inc function.
This considerations show that having parsing code in kernel space is
a questionable idea. Are you absolutely sure that you cannot do with less?
Regards
Oliver
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On 7/28/05, Mitchell Blank Jr <[EMAIL PROTECTED]> wrote:
> Greg KH wrote:
> > > + /* locate trailng white space */
> > > + z = y = x;
> > > + while (y - buffer->page < count) {
> > > + y++;
> > > + z = y;
> > > + while (isspace(*y) && (y - buffer->page < count)) {
> > > + y++;
> > > + }
> > > + }
> > > + count = z - x;
> >
> > Hm, I _think_ this works, but I need someone else to verify this...
> > Anyone else?
>
> It looks sane-ish to me, but also more complicated than need be. Why can't
> you just do something like:
>
> while (count > 0 && isspace(x[count - 1]))
> count--;
>
> -Mitch
>
Do we need to deal with UTF8 here? I did the forward loop because you
can't parse UTF8 backwards. If UTF8 is possible I need to change the
pointer inc function.
--
Jon Smirl
[EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On 7/28/05, Greg KH <[EMAIL PROTECTED]> wrote:
> On Thu, Jul 28, 2005 at 12:49:21AM -0400, Jon Smirl wrote:
> > @@ -207,6 +208,28 @@ 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;
> > + char *x, *y, *z;
> > +
> > + /* locate leading white space */
> > + x = buffer->page;
> > + while (isspace(*x) && (x - buffer->page < count))
> > + x++;
>
> Ok, I can follow this. For example
> buffer->page = " foo "
>
> then x = "foo " at the end of that .
>
> > + /* locate trailng white space */
> > + z = y = x;
> > + while (y - buffer->page < count) {
> > + y++;
> > + z = y;
> > + while (isspace(*y) && (y - buffer->page < count)) {
> > + y++;
> > + }
> > + }
> > + count = z - x;
>
> Hm, I _think_ this works, but I need someone else to verify this...
> Anyone else?
>
>
> > +
> > + /* strip the white space */
> > + if (buffer->page != x)
> > + memmove(buffer->page, x, count);
> > + buffer->page[count] = '\0';
>
> Why move the buffer? Why not just pass in a pointer to the start of the
> "non-whitespace filled" buffer to the store() function?
That will work if you say it does. I didn't know what happens with
buffer->page in other parts of the code.
>
> thanks,
>
> greg k-h
>
--
Jon Smirl
[EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Greg KH wrote:
> > + /* locate trailng white space */
> > + z = y = x;
> > + while (y - buffer->page < count) {
> > + y++;
> > + z = y;
> > + while (isspace(*y) && (y - buffer->page < count)) {
> > + y++;
> > + }
> > + }
> > + count = z - x;
>
> Hm, I _think_ this works, but I need someone else to verify this...
> Anyone else?
It looks sane-ish to me, but also more complicated than need be. Why can't
you just do something like:
while (count > 0 && isspace(x[count - 1]))
count--;
-Mitch
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On Thu, Jul 28, 2005 at 12:49:21AM -0400, Jon Smirl wrote:
> @@ -207,6 +208,28 @@ 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;
> + char *x, *y, *z;
> +
> + /* locate leading white space */
> + x = buffer->page;
> + while (isspace(*x) && (x - buffer->page < count))
> + x++;
Ok, I can follow this. For example
buffer->page = " foo "
then x = "foo " at the end of that .
> + /* locate trailng white space */
> + z = y = x;
> + while (y - buffer->page < count) {
> + y++;
> + z = y;
> + while (isspace(*y) && (y - buffer->page < count)) {
> + y++;
> + }
> + }
> + count = z - x;
Hm, I _think_ this works, but I need someone else to verify this...
Anyone else?
> +
> + /* strip the white space */
> + if (buffer->page != x)
> + memmove(buffer->page, x, count);
> + buffer->page[count] = '\0';
Why move the buffer? Why not just pass in a pointer to the start of the
"non-whitespace filled" buffer to the store() function?
thanks,
greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On Thu, Jul 28, 2005 at 12:49:21AM -0400, Jon Smirl wrote:
@@ -207,6 +208,28 @@ 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;
+ char *x, *y, *z;
+
+ /* locate leading white space */
+ x = buffer-page;
+ while (isspace(*x) (x - buffer-page count))
+ x++;
Ok, I can follow this. For example
buffer-page = foo
then x = foo at the end of that .
+ /* locate trailng white space */
+ z = y = x;
+ while (y - buffer-page count) {
+ y++;
+ z = y;
+ while (isspace(*y) (y - buffer-page count)) {
+ y++;
+ }
+ }
+ count = z - x;
Hm, I _think_ this works, but I need someone else to verify this...
Anyone else?
+
+ /* strip the white space */
+ if (buffer-page != x)
+ memmove(buffer-page, x, count);
+ buffer-page[count] = '\0';
Why move the buffer? Why not just pass in a pointer to the start of the
non-whitespace filled buffer to the store() function?
thanks,
greg k-h
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Greg KH wrote:
+ /* locate trailng white space */
+ z = y = x;
+ while (y - buffer-page count) {
+ y++;
+ z = y;
+ while (isspace(*y) (y - buffer-page count)) {
+ y++;
+ }
+ }
+ count = z - x;
Hm, I _think_ this works, but I need someone else to verify this...
Anyone else?
It looks sane-ish to me, but also more complicated than need be. Why can't
you just do something like:
while (count 0 isspace(x[count - 1]))
count--;
-Mitch
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On 7/28/05, Greg KH [EMAIL PROTECTED] wrote:
On Thu, Jul 28, 2005 at 12:49:21AM -0400, Jon Smirl wrote:
@@ -207,6 +208,28 @@ 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;
+ char *x, *y, *z;
+
+ /* locate leading white space */
+ x = buffer-page;
+ while (isspace(*x) (x - buffer-page count))
+ x++;
Ok, I can follow this. For example
buffer-page = foo
then x = foo at the end of that .
+ /* locate trailng white space */
+ z = y = x;
+ while (y - buffer-page count) {
+ y++;
+ z = y;
+ while (isspace(*y) (y - buffer-page count)) {
+ y++;
+ }
+ }
+ count = z - x;
Hm, I _think_ this works, but I need someone else to verify this...
Anyone else?
+
+ /* strip the white space */
+ if (buffer-page != x)
+ memmove(buffer-page, x, count);
+ buffer-page[count] = '\0';
Why move the buffer? Why not just pass in a pointer to the start of the
non-whitespace filled buffer to the store() function?
That will work if you say it does. I didn't know what happens with
buffer-page in other parts of the code.
thanks,
greg k-h
--
Jon Smirl
[EMAIL PROTECTED]
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On 7/28/05, Mitchell Blank Jr [EMAIL PROTECTED] wrote:
Greg KH wrote:
+ /* locate trailng white space */
+ z = y = x;
+ while (y - buffer-page count) {
+ y++;
+ z = y;
+ while (isspace(*y) (y - buffer-page count)) {
+ y++;
+ }
+ }
+ count = z - x;
Hm, I _think_ this works, but I need someone else to verify this...
Anyone else?
It looks sane-ish to me, but also more complicated than need be. Why can't
you just do something like:
while (count 0 isspace(x[count - 1]))
count--;
-Mitch
Do we need to deal with UTF8 here? I did the forward loop because you
can't parse UTF8 backwards. If UTF8 is possible I need to change the
pointer inc function.
--
Jon Smirl
[EMAIL PROTECTED]
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Am Donnerstag, 28. Juli 2005 14:54 schrieb Jon Smirl:
On 7/28/05, Mitchell Blank Jr [EMAIL PROTECTED] wrote:
Greg KH wrote:
+ /* locate trailng white space */
+ z = y = x;
+ while (y - buffer-page count) {
+ y++;
+ z = y;
+ while (isspace(*y) (y - buffer-page count)) {
+ y++;
+ }
+ }
+ count = z - x;
Hm, I _think_ this works, but I need someone else to verify this...
Anyone else?
It looks sane-ish to me, but also more complicated than need be. Why can't
you just do something like:
while (count 0 isspace(x[count - 1]))
count--;
-Mitch
Do we need to deal with UTF8 here? I did the forward loop because you
can't parse UTF8 backwards. If UTF8 is possible I need to change the
pointer inc function.
This considerations show that having parsing code in kernel space is
a questionable idea. Are you absolutely sure that you cannot do with less?
Regards
Oliver
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Jon Smirl wrote: On 7/28/05, Mitchell Blank Jr [EMAIL PROTECTED] wrote: [...] It looks sane-ish to me, but also more complicated than need be. Why can't you just do something like: while (count 0 isspace(x[count - 1])) count--; Do we need to deal with UTF8 here? I did the forward loop because you can't parse UTF8 backwards. If UTF8 is possible I need to change the pointer inc function. I don't think it matters here. Even with UTF8, any char that makes isspace return true, can't be part of a multi-byte char, as every byte in a multi-byte char in UTF8 has the MSB set, i.e., = 0x80. -- Paulo Marques - www.grupopie.com It is a mistake to think you can solve any major problems just with potatoes. Douglas Adams - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Jon Smirl wrote:
Do we need to deal with UTF8 here? I did the forward loop because you
can't parse UTF8 backwards. If UTF8 is possible I need to change the
pointer inc function.
As others have mentioned there shouldn't be a UTF8 problem with isspace().
However, even if you wanted to scan going forwards you can do it without
the complexity of a nested loop:
char *real_end;
for (real_end = x; x - buffer-page count; x++)
if (!isspace(*x))
real_end = x + 1;
*real_end = '\0';
// then fix count
BTW, my code I posted yesterday is incomplete since I neglected to notice
that the count = z - x at the end is used to repair count damage from
both the leading and trailing whitespace stripping. Its actually easier
to strip the trailing whitespace first, like:
while (count 0 isspace(buffer-page[count - 1]))
count--;/* strip trailing whitespace */
if (count 0 isspace(buffer-page[0])) {
/*
* We have leading whitespace but also at least one
* non-whitespace character
*/
const char *x = buffer-page;
do {
x++;
count--;
} while (isspace(*x));
memmove(buffer-page, x, count);
}
buffer-page[count] = '\0';
-Mitch
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On Thu, Jul 28, 2005 at 08:54:53AM -0400, Jon Smirl wrote:
On 7/28/05, Mitchell Blank Jr [EMAIL PROTECTED] wrote:
Greg KH wrote:
+ /* locate trailng white space */
+ z = y = x;
+ while (y - buffer-page count) {
+ y++;
+ z = y;
+ while (isspace(*y) (y - buffer-page count)) {
+ y++;
+ }
+ }
+ count = z - x;
Hm, I _think_ this works, but I need someone else to verify this...
Anyone else?
It looks sane-ish to me, but also more complicated than need be. Why can't
you just do something like:
while (count 0 isspace(x[count - 1]))
count--;
-Mitch
Do we need to deal with UTF8 here? I did the forward loop because you
can't parse UTF8 backwards. If UTF8 is possible I need to change the
pointer inc function.
No, we don't need to handle UTF8. Or if we do, then we better not make
a general filter function, as it will be a mess.
thanks,
greg k-h
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
New, simplified version of the sysfs whitespace strip patch... -- Jon Smirl [EMAIL PROTECTED] Remove leading and trailing whitespace when text sysfs attribute is set signed-off-by: Jon Smirl [EMAIL PROTECTED] 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,22 @@ 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; + char *x; - return ops-store(kobj,attr,buffer-page,count); + /* locate trailing white space */ + while ((count 0) isspace(buffer-page[count - 1])) + count--; + + /* locate leading white space */ + x = buffer-page; + while (isspace(*x) (x - buffer-page count)) + x++; + count -= (x - buffer-page); + + /* terminate the string */ + x[count] = '\0'; + + return ops-store(kobj, attr, x, count); } - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Even simpler version
--
Jon Smirl
[EMAIL PROTECTED]
Remove leading and trailing whitespace when text sysfs attribute is set
signed-off-by: Jon Smirl [EMAIL PROTECTED]
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,23 @@ 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;
+ char *x;
- return ops-store(kobj,attr,buffer-page,count);
+ /* locate trailing white space */
+ while ((count 0) isspace(buffer-page[count - 1]))
+ count--;
+
+ /* locate leading white space */
+ x = buffer-page;
+ if (count 0) {
+ while (isspace(*x))
+ x++;
+ count -= (x - buffer-page);
+ }
+ /* terminate the string */
+ x[count] = '\0';
+
+ return ops-store(kobj, attr, x, count);
}
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Still one nitpick: Jon Smirl wrote: + while (isspace(*x) (x - buffer-page count)) + x++; I think you can just do: if (count 0) while (isspace(*x)) x++; If the passed-in string was fully whitespace then the trailing-whitespace stripping would already reduce it to count==0. So as long as you check for that case you can be sure that the while() will stop before x falls off the end of the string. Other than that it looks fine to me now. -Mitch - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On 7/28/05, Oliver Neukum [EMAIL PROTECTED] wrote: Am Donnerstag, 28. Juli 2005 21:57 schrieb Jon Smirl: New, simplified version of the sysfs whitespace strip patch... Could you tell me why you don't just fail the operation if malformed input is supplied? Leading/trailing white space should be allowed. For example echo appends '\n' unless you know to use -n. It is easier to fix the kernel than to teach everyone to use -n. Regards Oliver -- Jon Smirl [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Am Donnerstag, 28. Juli 2005 23:12 schrieb Jon Smirl: On 7/28/05, Oliver Neukum [EMAIL PROTECTED] wrote: Am Donnerstag, 28. Juli 2005 21:57 schrieb Jon Smirl: New, simplified version of the sysfs whitespace strip patch... Could you tell me why you don't just fail the operation if malformed input is supplied? Leading/trailing white space should be allowed. For example echo appends '\n' unless you know to use -n. It is easier to fix the kernel than to teach everyone to use -n. Why? If you can do it in user space, do so. Regards Oliver - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Am Donnerstag, 28. Juli 2005 21:57 schrieb Jon Smirl: New, simplified version of the sysfs whitespace strip patch... Could you tell me why you don't just fail the operation if malformed input is supplied? Regards Oliver - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Change log and signed off
--
Jon Smirl
[EMAIL PROTECTED]
Remove leading and trailing whitespace when text sysfs attributes are
assigned a value.
Signed-off-by: Jon Smirl <[EMAIL PROTECTED]>
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
#include
#include
+#include
#include
#include
@@ -207,6 +208,28 @@ 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;
+ char *x, *y, *z;
+
+ /* locate leading white space */
+ x = buffer->page;
+ while (isspace(*x) && (x - buffer->page < count))
+ x++;
+
+ /* locate trailng white space */
+ z = y = x;
+ while (y - buffer->page < count) {
+ y++;
+ z = y;
+ while (isspace(*y) && (y - buffer->page < count)) {
+ y++;
+ }
+ }
+ count = z - x;
+
+ /* strip the white space */
+ if (buffer->page != x)
+ memmove(buffer->page, x, count);
+ buffer->page[count] = '\0';
return ops->store(kobj,attr,buffer->page,count);
}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On Wed, Jul 27, 2005 at 11:59:11PM -0400, Jon Smirl wrote: > New patch with fixed whitespace. But no changelog info or signed-off-by line :( Care to try it again? thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
New patch with fixed whitespace.
--
Jon Smirl
[EMAIL PROTECTED]
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
#include
#include
+#include
#include
#include
@@ -207,6 +208,28 @@ 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;
+ char *x, *y, *z;
+
+ /* locate leading white space */
+ x = buffer->page;
+ while (isspace(*x) && (x - buffer->page < count))
+ x++;
+
+ /* locate trailng white space */
+ z = y = x;
+ while (y - buffer->page < count) {
+ y++;
+ z = y;
+ while (isspace(*y) && (y - buffer->page < count)) {
+ y++;
+ }
+ }
+ count = z - x;
+
+ /* strip the white space */
+ if (buffer->page != x)
+ memmove(buffer->page, x, count);
+ buffer->page[count] = '\0';
return ops->store(kobj,attr,buffer->page,count);
}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On Wed, Jul 27, 2005 at 10:05:34PM -0400, Jon Smirl wrote: > Any comments on this? I'll fix up the whitespace issues if everyone > agrees that the code works. I'll add the patch to my tree and -mm if you clean up the whitespace issues :) thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Any comments on this? I'll fix up the whitespace issues if everyone
agrees that the code works.
This patch will break all of the fbdev attributes since I was making
wrong assumptions. I have another patch ready to fix them after this
one goes in.
On 7/25/05, Jon Smirl <[EMAIL PROTECTED]> wrote:
> On 7/25/05, Greg KH <[EMAIL PROTECTED]> wrote:
> > > I'll put one together to trim leading/trailing white space from the
> > > buffer before it is passed into the attribute functions. Now that I
> > > think about this I believe the attributes should have always had the
> > > leading/trailing white space removed. If we don't do it in the sysfs
> > > code then every driver has to do it.
> >
> > Ok, sounds good.
>
> How does this look? This is a count based interface but a lot of
> attributes don't work unless I add the terminating zero. This
> interface should be documented: count or zero terminated, white space
> stripped or not, etc. Are these strings ASCII, UTF8, Unicode?
>
> 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
> #include
> #include
> +#include
> #include
> #include
>
> @@ -207,6 +208,28 @@ 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;
> + char *x, *y, *z;
> +
> + /* locate leading white space */
> + x = buffer->page;
> + while( isspace(*x) && (x - buffer->page < count))
> + x++;
> +
> + /* locate trailng white space */
> + z = y = x;
> + while (y - buffer->page < count) {
> + y++;
> + z = y;
> + while( isspace(*y) && (y - buffer->page < count)) {
> + y++;
> + }
> + }
> + count = z - x;
> +
> + /* strip the white space */
> + if (buffer->page != x)
> + memmove(buffer->page, x, count);
> + buffer->page[count] = '\0';
>
> return ops->store(kobj,attr,buffer->page,count);
> }
>
>
> --
> Jon Smirl
> [EMAIL PROTECTED]
>
--
Jon Smirl
[EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Any comments on this? I'll fix up the whitespace issues if everyone
agrees that the code works.
This patch will break all of the fbdev attributes since I was making
wrong assumptions. I have another patch ready to fix them after this
one goes in.
On 7/25/05, Jon Smirl [EMAIL PROTECTED] wrote:
On 7/25/05, Greg KH [EMAIL PROTECTED] wrote:
I'll put one together to trim leading/trailing white space from the
buffer before it is passed into the attribute functions. Now that I
think about this I believe the attributes should have always had the
leading/trailing white space removed. If we don't do it in the sysfs
code then every driver has to do it.
Ok, sounds good.
How does this look? This is a count based interface but a lot of
attributes don't work unless I add the terminating zero. This
interface should be documented: count or zero terminated, white space
stripped or not, etc. Are these strings ASCII, UTF8, Unicode?
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,6 +208,28 @@ 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;
+ char *x, *y, *z;
+
+ /* locate leading white space */
+ x = buffer-page;
+ while( isspace(*x) (x - buffer-page count))
+ x++;
+
+ /* locate trailng white space */
+ z = y = x;
+ while (y - buffer-page count) {
+ y++;
+ z = y;
+ while( isspace(*y) (y - buffer-page count)) {
+ y++;
+ }
+ }
+ count = z - x;
+
+ /* strip the white space */
+ if (buffer-page != x)
+ memmove(buffer-page, x, count);
+ buffer-page[count] = '\0';
return ops-store(kobj,attr,buffer-page,count);
}
--
Jon Smirl
[EMAIL PROTECTED]
--
Jon Smirl
[EMAIL PROTECTED]
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On Wed, Jul 27, 2005 at 10:05:34PM -0400, Jon Smirl wrote: Any comments on this? I'll fix up the whitespace issues if everyone agrees that the code works. I'll add the patch to my tree and -mm if you clean up the whitespace issues :) thanks, greg k-h - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
New patch with fixed whitespace.
--
Jon Smirl
[EMAIL PROTECTED]
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,6 +208,28 @@ 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;
+ char *x, *y, *z;
+
+ /* locate leading white space */
+ x = buffer-page;
+ while (isspace(*x) (x - buffer-page count))
+ x++;
+
+ /* locate trailng white space */
+ z = y = x;
+ while (y - buffer-page count) {
+ y++;
+ z = y;
+ while (isspace(*y) (y - buffer-page count)) {
+ y++;
+ }
+ }
+ count = z - x;
+
+ /* strip the white space */
+ if (buffer-page != x)
+ memmove(buffer-page, x, count);
+ buffer-page[count] = '\0';
return ops-store(kobj,attr,buffer-page,count);
}
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On Wed, Jul 27, 2005 at 11:59:11PM -0400, Jon Smirl wrote: New patch with fixed whitespace. But no changelog info or signed-off-by line :( Care to try it again? thanks, greg k-h - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
Change log and signed off
--
Jon Smirl
[EMAIL PROTECTED]
Remove leading and trailing whitespace when text sysfs attributes are
assigned a value.
Signed-off-by: Jon Smirl [EMAIL PROTECTED]
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,6 +208,28 @@ 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;
+ char *x, *y, *z;
+
+ /* locate leading white space */
+ x = buffer-page;
+ while (isspace(*x) (x - buffer-page count))
+ x++;
+
+ /* locate trailng white space */
+ z = y = x;
+ while (y - buffer-page count) {
+ y++;
+ z = y;
+ while (isspace(*y) (y - buffer-page count)) {
+ y++;
+ }
+ }
+ count = z - x;
+
+ /* strip the white space */
+ if (buffer-page != x)
+ memmove(buffer-page, x, count);
+ buffer-page[count] = '\0';
return ops-store(kobj,attr,buffer-page,count);
}
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On Monday 25 July 2005 22:15, Jon Smirl wrote:
> + while( isspace(*x) && (x - buffer->page < count))
> + x++;
> +
> + /* locate trailng white space */
> + z = y = x;
> + while (y - buffer->page < count) {
> + y++;
> + z = y;
> + while( isspace(*y) && (y - buffer->page < count)) {
> + y++;
Can we have consistent space vs. paren placement, pretty please?
--
Dmitry
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On 7/25/05, Greg KH <[EMAIL PROTECTED]> wrote:
> > I'll put one together to trim leading/trailing white space from the
> > buffer before it is passed into the attribute functions. Now that I
> > think about this I believe the attributes should have always had the
> > leading/trailing white space removed. If we don't do it in the sysfs
> > code then every driver has to do it.
>
> Ok, sounds good.
How does this look? This is a count based interface but a lot of
attributes don't work unless I add the terminating zero. This
interface should be documented: count or zero terminated, white space
stripped or not, etc. Are these strings ASCII, UTF8, Unicode?
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
#include
#include
+#include
#include
#include
@@ -207,6 +208,28 @@ 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;
+ char *x, *y, *z;
+
+ /* locate leading white space */
+ x = buffer->page;
+ while( isspace(*x) && (x - buffer->page < count))
+ x++;
+
+ /* locate trailng white space */
+ z = y = x;
+ while (y - buffer->page < count) {
+ y++;
+ z = y;
+ while( isspace(*y) && (y - buffer->page < count)) {
+ y++;
+ }
+ }
+ count = z - x;
+
+ /* strip the white space */
+ if (buffer->page != x)
+ memmove(buffer->page, x, count);
+ buffer->page[count] = '\0';
return ops->store(kobj,attr,buffer->page,count);
}
--
Jon Smirl
[EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On Mon, Jul 25, 2005 at 08:56:17PM -0400, Jon Smirl wrote: > On 7/25/05, Greg KH <[EMAIL PROTECTED]> wrote: > > On Mon, Jul 25, 2005 at 08:28:10PM -0400, Jon Smirl wrote: > > > I didn't realize that echo was adding the CR, I thought that it always > > > appeared on the end of a sysfs attribute set. So now I have to go add > > > white space stripping to a dozen fbdev/drm sysfs attribute > > > implementations. Given that the param is const I may have to allocate > > > new buffers and copy. I also wonder how many other people have made > > > the same mistake. > > > > Nah, just zero out that \n character :) > > The input buffer is "const char * buf". I will have to override the > const to zero it out. Yeah, hence the ":)" above. > > > Are you sure it would break other things? These are supposed to be > > > text attributes, not binary ones. > > > > I agree, I don't know what would break. Care to make a patch so we > > could find out? > > I'll put one together to trim leading/trailing white space from the > buffer before it is passed into the attribute functions. Now that I > think about this I believe the attributes should have always had the > leading/trailing white space removed. If we don't do it in the sysfs > code then every driver has to do it. Ok, sounds good. thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On 7/25/05, Greg KH <[EMAIL PROTECTED]> wrote: > On Mon, Jul 25, 2005 at 08:28:10PM -0400, Jon Smirl wrote: > > I didn't realize that echo was adding the CR, I thought that it always > > appeared on the end of a sysfs attribute set. So now I have to go add > > white space stripping to a dozen fbdev/drm sysfs attribute > > implementations. Given that the param is const I may have to allocate > > new buffers and copy. I also wonder how many other people have made > > the same mistake. > > Nah, just zero out that \n character :) The input buffer is "const char * buf". I will have to override the const to zero it out. > > > Are you sure it would break other things? These are supposed to be > > text attributes, not binary ones. > > I agree, I don't know what would break. Care to make a patch so we > could find out? I'll put one together to trim leading/trailing white space from the buffer before it is passed into the attribute functions. Now that I think about this I believe the attributes should have always had the leading/trailing white space removed. If we don't do it in the sysfs code then every driver has to do it. -- Jon Smirl [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On Mon, Jul 25, 2005 at 08:28:10PM -0400, Jon Smirl wrote: > On 7/25/05, Greg KH <[EMAIL PROTECTED]> wrote: > > On Mon, Jul 25, 2005 at 12:30:43PM -0400, Jon Smirl wrote: > > > On 7/25/05, Dmitry Torokhov <[EMAIL PROTECTED]> wrote: > > > > On 7/25/05, Jon Smirl <[EMAIL PROTECTED]> wrote: > > > > > On 7/25/05, Dmitry Torokhov <[EMAIL PROTECTED]> wrote: > > > > > > On Sunday 24 July 2005 23:09, Jon Smirl wrote: > > > > > > > I just pulled from GIT to test bind/unbind. I couldn't get it to > > > > > > > work; > > > > > > > it isn't taking into account the CR on the end of the input value > > > > > > > of > > > > > > > the sysfs attribute. This patch will fix it but I'm sure there > > > > > > > is a > > > > > > > cleaner solution. > > > > > > > > > > > > > > > > > > > "echo -n" should take care of this problem I think. > > > > > > > > > > That will work around it but I think we should fix it. Changing to > > > > > strncmp() fixes most cases. > > > > > > > > > > - if (strcmp(name, dev->bus_id) == 0) > > > > > + if (strncmp(name, dev->bus_id, strlen(dev->bus_id)) == 0) > > > > > > > > > > > > > This will produce "interesting results" if you have both "blah-1" and > > > > "blah-10" devices on the bus. > > > > Yes, not a good thing for USB devices specifically. > > > > > Then the better solution is to fix the generic attribute set code to > > > strip leading and trailing white space. > > > > No, that might break other things as we have not been doing this from > > day one. I'd rather just change these two places, if it's that big of a > > deal. It was documented (in a lwn.net article) and the changelog entry, > > that you should use "echo -n". > > I didn't realize that echo was adding the CR, I thought that it always > appeared on the end of a sysfs attribute set. So now I have to go add > white space stripping to a dozen fbdev/drm sysfs attribute > implementations. Given that the param is const I may have to allocate > new buffers and copy. I also wonder how many other people have made > the same mistake. Nah, just zero out that \n character :) > Are you sure it would break other things? These are supposed to be > text attributes, not binary ones. I agree, I don't know what would break. Care to make a patch so we could find out? thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On 7/25/05, Greg KH <[EMAIL PROTECTED]> wrote: > On Mon, Jul 25, 2005 at 12:30:43PM -0400, Jon Smirl wrote: > > On 7/25/05, Dmitry Torokhov <[EMAIL PROTECTED]> wrote: > > > On 7/25/05, Jon Smirl <[EMAIL PROTECTED]> wrote: > > > > On 7/25/05, Dmitry Torokhov <[EMAIL PROTECTED]> wrote: > > > > > On Sunday 24 July 2005 23:09, Jon Smirl wrote: > > > > > > I just pulled from GIT to test bind/unbind. I couldn't get it to > > > > > > work; > > > > > > it isn't taking into account the CR on the end of the input value of > > > > > > the sysfs attribute. This patch will fix it but I'm sure there is a > > > > > > cleaner solution. > > > > > > > > > > > > > > > > "echo -n" should take care of this problem I think. > > > > > > > > That will work around it but I think we should fix it. Changing to > > > > strncmp() fixes most cases. > > > > > > > > - if (strcmp(name, dev->bus_id) == 0) > > > > + if (strncmp(name, dev->bus_id, strlen(dev->bus_id)) == 0) > > > > > > > > > > This will produce "interesting results" if you have both "blah-1" and > > > "blah-10" devices on the bus. > > Yes, not a good thing for USB devices specifically. > > > Then the better solution is to fix the generic attribute set code to > > strip leading and trailing white space. > > No, that might break other things as we have not been doing this from > day one. I'd rather just change these two places, if it's that big of a > deal. It was documented (in a lwn.net article) and the changelog entry, > that you should use "echo -n". I didn't realize that echo was adding the CR, I thought that it always appeared on the end of a sysfs attribute set. So now I have to go add white space stripping to a dozen fbdev/drm sysfs attribute implementations. Given that the param is const I may have to allocate new buffers and copy. I also wonder how many other people have made the same mistake. Are you sure it would break other things? These are supposed to be text attributes, not binary ones. -- Jon Smirl [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On Mon, Jul 25, 2005 at 12:30:43PM -0400, Jon Smirl wrote: > On 7/25/05, Dmitry Torokhov <[EMAIL PROTECTED]> wrote: > > On 7/25/05, Jon Smirl <[EMAIL PROTECTED]> wrote: > > > On 7/25/05, Dmitry Torokhov <[EMAIL PROTECTED]> wrote: > > > > On Sunday 24 July 2005 23:09, Jon Smirl wrote: > > > > > I just pulled from GIT to test bind/unbind. I couldn't get it to work; > > > > > it isn't taking into account the CR on the end of the input value of > > > > > the sysfs attribute. This patch will fix it but I'm sure there is a > > > > > cleaner solution. > > > > > > > > > > > > > "echo -n" should take care of this problem I think. > > > > > > That will work around it but I think we should fix it. Changing to > > > strncmp() fixes most cases. > > > > > > - if (strcmp(name, dev->bus_id) == 0) > > > + if (strncmp(name, dev->bus_id, strlen(dev->bus_id)) == 0) > > > > > > > This will produce "interesting results" if you have both "blah-1" and > > "blah-10" devices on the bus. Yes, not a good thing for USB devices specifically. > Then the better solution is to fix the generic attribute set code to > strip leading and trailing white space. No, that might break other things as we have not been doing this from day one. I'd rather just change these two places, if it's that big of a deal. It was documented (in a lwn.net article) and the changelog entry, that you should use "echo -n". thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On 7/25/05, Dmitry Torokhov <[EMAIL PROTECTED]> wrote: > On 7/25/05, Jon Smirl <[EMAIL PROTECTED]> wrote: > > On 7/25/05, Dmitry Torokhov <[EMAIL PROTECTED]> wrote: > > > On Sunday 24 July 2005 23:09, Jon Smirl wrote: > > > > I just pulled from GIT to test bind/unbind. I couldn't get it to work; > > > > it isn't taking into account the CR on the end of the input value of > > > > the sysfs attribute. This patch will fix it but I'm sure there is a > > > > cleaner solution. > > > > > > > > > > "echo -n" should take care of this problem I think. > > > > That will work around it but I think we should fix it. Changing to > > strncmp() fixes most cases. > > > > - if (strcmp(name, dev->bus_id) == 0) > > + if (strncmp(name, dev->bus_id, strlen(dev->bus_id)) == 0) > > > > This will produce "interesting results" if you have both "blah-1" and > "blah-10" devices on the bus. Then the better solution is to fix the generic attribute set code to strip leading and trailing white space. -- Jon Smirl [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On 7/25/05, Jon Smirl <[EMAIL PROTECTED]> wrote: > On 7/25/05, Dmitry Torokhov <[EMAIL PROTECTED]> wrote: > > On Sunday 24 July 2005 23:09, Jon Smirl wrote: > > > I just pulled from GIT to test bind/unbind. I couldn't get it to work; > > > it isn't taking into account the CR on the end of the input value of > > > the sysfs attribute. This patch will fix it but I'm sure there is a > > > cleaner solution. > > > > > > > "echo -n" should take care of this problem I think. > > That will work around it but I think we should fix it. Changing to > strncmp() fixes most cases. > > - if (strcmp(name, dev->bus_id) == 0) > + if (strncmp(name, dev->bus_id, strlen(dev->bus_id)) == 0) > This will produce "interesting results" if you have both "blah-1" and "blah-10" devices on the bus. -- Dmitry - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On 7/25/05, Dmitry Torokhov <[EMAIL PROTECTED]> wrote: > On Sunday 24 July 2005 23:09, Jon Smirl wrote: > > I just pulled from GIT to test bind/unbind. I couldn't get it to work; > > it isn't taking into account the CR on the end of the input value of > > the sysfs attribute. This patch will fix it but I'm sure there is a > > cleaner solution. > > > > "echo -n" should take care of this problem I think. That will work around it but I think we should fix it. Changing to strncmp() fixes most cases. - if (strcmp(name, dev->bus_id) == 0) + if (strncmp(name, dev->bus_id, strlen(dev->bus_id)) == 0) I work in this area and I couldn't figure out why it was silently not working. I had to add the printk to the source before I could figure it out. I suspect most people are going to have this trouble. This has also made me realize that I have created other places in the kernel where my sysfs attribute code is not going to work correctly. Maybe we should adjust the sysfs driver to strip leading and trailing white space before passing the string on. -- Jon Smirl [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On 7/25/05, Dmitry Torokhov [EMAIL PROTECTED] wrote: On Sunday 24 July 2005 23:09, Jon Smirl wrote: I just pulled from GIT to test bind/unbind. I couldn't get it to work; it isn't taking into account the CR on the end of the input value of the sysfs attribute. This patch will fix it but I'm sure there is a cleaner solution. echo -n should take care of this problem I think. That will work around it but I think we should fix it. Changing to strncmp() fixes most cases. - if (strcmp(name, dev-bus_id) == 0) + if (strncmp(name, dev-bus_id, strlen(dev-bus_id)) == 0) I work in this area and I couldn't figure out why it was silently not working. I had to add the printk to the source before I could figure it out. I suspect most people are going to have this trouble. This has also made me realize that I have created other places in the kernel where my sysfs attribute code is not going to work correctly. Maybe we should adjust the sysfs driver to strip leading and trailing white space before passing the string on. -- Jon Smirl [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On 7/25/05, Jon Smirl [EMAIL PROTECTED] wrote: On 7/25/05, Dmitry Torokhov [EMAIL PROTECTED] wrote: On Sunday 24 July 2005 23:09, Jon Smirl wrote: I just pulled from GIT to test bind/unbind. I couldn't get it to work; it isn't taking into account the CR on the end of the input value of the sysfs attribute. This patch will fix it but I'm sure there is a cleaner solution. echo -n should take care of this problem I think. That will work around it but I think we should fix it. Changing to strncmp() fixes most cases. - if (strcmp(name, dev-bus_id) == 0) + if (strncmp(name, dev-bus_id, strlen(dev-bus_id)) == 0) This will produce interesting results if you have both blah-1 and blah-10 devices on the bus. -- Dmitry - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On 7/25/05, Dmitry Torokhov [EMAIL PROTECTED] wrote: On 7/25/05, Jon Smirl [EMAIL PROTECTED] wrote: On 7/25/05, Dmitry Torokhov [EMAIL PROTECTED] wrote: On Sunday 24 July 2005 23:09, Jon Smirl wrote: I just pulled from GIT to test bind/unbind. I couldn't get it to work; it isn't taking into account the CR on the end of the input value of the sysfs attribute. This patch will fix it but I'm sure there is a cleaner solution. echo -n should take care of this problem I think. That will work around it but I think we should fix it. Changing to strncmp() fixes most cases. - if (strcmp(name, dev-bus_id) == 0) + if (strncmp(name, dev-bus_id, strlen(dev-bus_id)) == 0) This will produce interesting results if you have both blah-1 and blah-10 devices on the bus. Then the better solution is to fix the generic attribute set code to strip leading and trailing white space. -- Jon Smirl [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On Mon, Jul 25, 2005 at 12:30:43PM -0400, Jon Smirl wrote: On 7/25/05, Dmitry Torokhov [EMAIL PROTECTED] wrote: On 7/25/05, Jon Smirl [EMAIL PROTECTED] wrote: On 7/25/05, Dmitry Torokhov [EMAIL PROTECTED] wrote: On Sunday 24 July 2005 23:09, Jon Smirl wrote: I just pulled from GIT to test bind/unbind. I couldn't get it to work; it isn't taking into account the CR on the end of the input value of the sysfs attribute. This patch will fix it but I'm sure there is a cleaner solution. echo -n should take care of this problem I think. That will work around it but I think we should fix it. Changing to strncmp() fixes most cases. - if (strcmp(name, dev-bus_id) == 0) + if (strncmp(name, dev-bus_id, strlen(dev-bus_id)) == 0) This will produce interesting results if you have both blah-1 and blah-10 devices on the bus. Yes, not a good thing for USB devices specifically. Then the better solution is to fix the generic attribute set code to strip leading and trailing white space. No, that might break other things as we have not been doing this from day one. I'd rather just change these two places, if it's that big of a deal. It was documented (in a lwn.net article) and the changelog entry, that you should use echo -n. thanks, greg k-h - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On 7/25/05, Greg KH [EMAIL PROTECTED] wrote: On Mon, Jul 25, 2005 at 12:30:43PM -0400, Jon Smirl wrote: On 7/25/05, Dmitry Torokhov [EMAIL PROTECTED] wrote: On 7/25/05, Jon Smirl [EMAIL PROTECTED] wrote: On 7/25/05, Dmitry Torokhov [EMAIL PROTECTED] wrote: On Sunday 24 July 2005 23:09, Jon Smirl wrote: I just pulled from GIT to test bind/unbind. I couldn't get it to work; it isn't taking into account the CR on the end of the input value of the sysfs attribute. This patch will fix it but I'm sure there is a cleaner solution. echo -n should take care of this problem I think. That will work around it but I think we should fix it. Changing to strncmp() fixes most cases. - if (strcmp(name, dev-bus_id) == 0) + if (strncmp(name, dev-bus_id, strlen(dev-bus_id)) == 0) This will produce interesting results if you have both blah-1 and blah-10 devices on the bus. Yes, not a good thing for USB devices specifically. Then the better solution is to fix the generic attribute set code to strip leading and trailing white space. No, that might break other things as we have not been doing this from day one. I'd rather just change these two places, if it's that big of a deal. It was documented (in a lwn.net article) and the changelog entry, that you should use echo -n. I didn't realize that echo was adding the CR, I thought that it always appeared on the end of a sysfs attribute set. So now I have to go add white space stripping to a dozen fbdev/drm sysfs attribute implementations. Given that the param is const I may have to allocate new buffers and copy. I also wonder how many other people have made the same mistake. Are you sure it would break other things? These are supposed to be text attributes, not binary ones. -- Jon Smirl [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On Mon, Jul 25, 2005 at 08:28:10PM -0400, Jon Smirl wrote: On 7/25/05, Greg KH [EMAIL PROTECTED] wrote: On Mon, Jul 25, 2005 at 12:30:43PM -0400, Jon Smirl wrote: On 7/25/05, Dmitry Torokhov [EMAIL PROTECTED] wrote: On 7/25/05, Jon Smirl [EMAIL PROTECTED] wrote: On 7/25/05, Dmitry Torokhov [EMAIL PROTECTED] wrote: On Sunday 24 July 2005 23:09, Jon Smirl wrote: I just pulled from GIT to test bind/unbind. I couldn't get it to work; it isn't taking into account the CR on the end of the input value of the sysfs attribute. This patch will fix it but I'm sure there is a cleaner solution. echo -n should take care of this problem I think. That will work around it but I think we should fix it. Changing to strncmp() fixes most cases. - if (strcmp(name, dev-bus_id) == 0) + if (strncmp(name, dev-bus_id, strlen(dev-bus_id)) == 0) This will produce interesting results if you have both blah-1 and blah-10 devices on the bus. Yes, not a good thing for USB devices specifically. Then the better solution is to fix the generic attribute set code to strip leading and trailing white space. No, that might break other things as we have not been doing this from day one. I'd rather just change these two places, if it's that big of a deal. It was documented (in a lwn.net article) and the changelog entry, that you should use echo -n. I didn't realize that echo was adding the CR, I thought that it always appeared on the end of a sysfs attribute set. So now I have to go add white space stripping to a dozen fbdev/drm sysfs attribute implementations. Given that the param is const I may have to allocate new buffers and copy. I also wonder how many other people have made the same mistake. Nah, just zero out that \n character :) Are you sure it would break other things? These are supposed to be text attributes, not binary ones. I agree, I don't know what would break. Care to make a patch so we could find out? thanks, greg k-h - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On 7/25/05, Greg KH [EMAIL PROTECTED] wrote: On Mon, Jul 25, 2005 at 08:28:10PM -0400, Jon Smirl wrote: I didn't realize that echo was adding the CR, I thought that it always appeared on the end of a sysfs attribute set. So now I have to go add white space stripping to a dozen fbdev/drm sysfs attribute implementations. Given that the param is const I may have to allocate new buffers and copy. I also wonder how many other people have made the same mistake. Nah, just zero out that \n character :) The input buffer is const char * buf. I will have to override the const to zero it out. Are you sure it would break other things? These are supposed to be text attributes, not binary ones. I agree, I don't know what would break. Care to make a patch so we could find out? I'll put one together to trim leading/trailing white space from the buffer before it is passed into the attribute functions. Now that I think about this I believe the attributes should have always had the leading/trailing white space removed. If we don't do it in the sysfs code then every driver has to do it. -- Jon Smirl [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On Mon, Jul 25, 2005 at 08:56:17PM -0400, Jon Smirl wrote: On 7/25/05, Greg KH [EMAIL PROTECTED] wrote: On Mon, Jul 25, 2005 at 08:28:10PM -0400, Jon Smirl wrote: I didn't realize that echo was adding the CR, I thought that it always appeared on the end of a sysfs attribute set. So now I have to go add white space stripping to a dozen fbdev/drm sysfs attribute implementations. Given that the param is const I may have to allocate new buffers and copy. I also wonder how many other people have made the same mistake. Nah, just zero out that \n character :) The input buffer is const char * buf. I will have to override the const to zero it out. Yeah, hence the :) above. Are you sure it would break other things? These are supposed to be text attributes, not binary ones. I agree, I don't know what would break. Care to make a patch so we could find out? I'll put one together to trim leading/trailing white space from the buffer before it is passed into the attribute functions. Now that I think about this I believe the attributes should have always had the leading/trailing white space removed. If we don't do it in the sysfs code then every driver has to do it. Ok, sounds good. thanks, greg k-h - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On 7/25/05, Greg KH [EMAIL PROTECTED] wrote:
I'll put one together to trim leading/trailing white space from the
buffer before it is passed into the attribute functions. Now that I
think about this I believe the attributes should have always had the
leading/trailing white space removed. If we don't do it in the sysfs
code then every driver has to do it.
Ok, sounds good.
How does this look? This is a count based interface but a lot of
attributes don't work unless I add the terminating zero. This
interface should be documented: count or zero terminated, white space
stripped or not, etc. Are these strings ASCII, UTF8, Unicode?
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,6 +208,28 @@ 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;
+ char *x, *y, *z;
+
+ /* locate leading white space */
+ x = buffer-page;
+ while( isspace(*x) (x - buffer-page count))
+ x++;
+
+ /* locate trailng white space */
+ z = y = x;
+ while (y - buffer-page count) {
+ y++;
+ z = y;
+ while( isspace(*y) (y - buffer-page count)) {
+ y++;
+ }
+ }
+ count = z - x;
+
+ /* strip the white space */
+ if (buffer-page != x)
+ memmove(buffer-page, x, count);
+ buffer-page[count] = '\0';
return ops-store(kobj,attr,buffer-page,count);
}
--
Jon Smirl
[EMAIL PROTECTED]
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On Monday 25 July 2005 22:15, Jon Smirl wrote:
+ while( isspace(*x) (x - buffer-page count))
+ x++;
+
+ /* locate trailng white space */
+ z = y = x;
+ while (y - buffer-page count) {
+ y++;
+ z = y;
+ while( isspace(*y) (y - buffer-page count)) {
+ y++;
Can we have consistent space vs. paren placement, pretty please?
--
Dmitry
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On Sunday 24 July 2005 23:09, Jon Smirl wrote: > I just pulled from GIT to test bind/unbind. I couldn't get it to work; > it isn't taking into account the CR on the end of the input value of > the sysfs attribute. This patch will fix it but I'm sure there is a > cleaner solution. > "echo -n" should take care of this problem I think. -- Dmitry - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] driver core: Add the ability to unbind drivers to devices from userspace
I just pulled from GIT to test bind/unbind. I couldn't get it to work;
it isn't taking into account the CR on the end of the input value of
the sysfs attribute. This patch will fix it but I'm sure there is a
cleaner solution.
--
Jon Smirl
[EMAIL PROTECTED]
diff --git a/drivers/base/bus.c b/drivers/base/bus.c
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -137,9 +137,11 @@ decl_subsys(bus, _bus, NULL);
static int driver_helper(struct device *dev, void *data)
{
const char *name = data;
-
- if (strcmp(name, dev->bus_id) == 0)
+printk(KERN_ERR "unbind: %s %s\n", name, dev->bus_id);
+ if (strncmp(name, dev->bus_id, strlen(dev->bus_id)) == 0) {
+printk(KERN_ERR "match\n");
return 1;
+ }
return 0;
}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] driver core: Add the ability to unbind drivers to devices from userspace
I just pulled from GIT to test bind/unbind. I couldn't get it to work;
it isn't taking into account the CR on the end of the input value of
the sysfs attribute. This patch will fix it but I'm sure there is a
cleaner solution.
--
Jon Smirl
[EMAIL PROTECTED]
diff --git a/drivers/base/bus.c b/drivers/base/bus.c
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -137,9 +137,11 @@ decl_subsys(bus, ktype_bus, NULL);
static int driver_helper(struct device *dev, void *data)
{
const char *name = data;
-
- if (strcmp(name, dev-bus_id) == 0)
+printk(KERN_ERR unbind: %s %s\n, name, dev-bus_id);
+ if (strncmp(name, dev-bus_id, strlen(dev-bus_id)) == 0) {
+printk(KERN_ERR match\n);
return 1;
+ }
return 0;
}
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] driver core: Add the ability to unbind drivers to devices from userspace
On Sunday 24 July 2005 23:09, Jon Smirl wrote: I just pulled from GIT to test bind/unbind. I couldn't get it to work; it isn't taking into account the CR on the end of the input value of the sysfs attribute. This patch will fix it but I'm sure there is a cleaner solution. echo -n should take care of this problem I think. -- Dmitry - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
