2005-09-26 02:27:40

by [email protected]

[permalink] [raw]
Subject: usb-snd-audio breakage

usb-snd-audio broke in the last 24hrs in Linus' git tree. modprobe of
the module fails. I have a PSC805 USB audio device.

modprobe snd-usb-audio
err -> snd_usb_audio: Unknown parameter `'

--
Jon Smirl
[email protected]


2005-09-26 02:43:15

by [email protected]

[permalink] [raw]
Subject: Re: usb-snd-audio breakage

The Redhat FC4 installer is adds index=0 in modprobe.conf. The index
parameter appears to have been removed fron snd-usb-audio.

There are two issues:
1) should index have been left as a non-functioning param so that
existing installs won't break.
2) Why didn't I get a decent error message about index being the
problem instead of the message about `'

On 9/25/05, Jon Smirl <[email protected]> wrote:
> usb-snd-audio broke in the last 24hrs in Linus' git tree. modprobe of
> the module fails. I have a PSC805 USB audio device.
>
> modprobe snd-usb-audio
> err -> snd_usb_audio: Unknown parameter `'
>
> --
> Jon Smirl
> [email protected]
>


--
Jon Smirl
[email protected]

2005-09-26 03:38:10

by Dave Jones

[permalink] [raw]
Subject: Re: usb-snd-audio breakage

On Sun, Sep 25, 2005 at 10:43:11PM -0400, Jon Smirl wrote:
> The Redhat FC4 installer is adds index=0 in modprobe.conf. The index
> parameter appears to have been removed fron snd-usb-audio.
>
> There are two issues:
> 1) should index have been left as a non-functioning param so that
> existing installs won't break.
> 2) Why didn't I get a decent error message about index being the
> problem instead of the message about `'

This patch really should have been merged for 2.6.13
but somehow fell through the cracks. I don't think it
even landed in -mm

Dave



Name: Ignore trailing whitespace on kernel parameters correctly: Fixed version
Signed-off-by: Rusty Russell <[email protected]>

Dave Jones says:

... if the modprobe.conf has trailing whitespace, modules fail to load
with the following helpful message..

snd_intel8x0: Unknown parameter `'

Previous version truncated last argument.

Index: linux-2.6.13-rc6-git7-Module/kernel/params.c
===================================================================
--- linux-2.6.13-rc6-git7-Module.orig/kernel/params.c 2005-08-10 16:12:45.000000000 +1000
+++ linux-2.6.13-rc6-git7-Module/kernel/params.c 2005-08-16 14:31:16.000000000 +1000
@@ -80,8 +80,6 @@
int in_quote = 0, quoted = 0;
char *next;

- /* Chew any extra spaces */
- while (*args == ' ') args++;
if (*args == '"') {
args++;
in_quote = 1;
@@ -121,6 +119,9 @@
next = args + i + 1;
} else
next = args + i;
+
+ /* Chew up trailing spaces. */
+ while (*next == ' ') next++;
return next;
}

@@ -134,6 +135,9 @@
char *param, *val;

DEBUGP("Parsing ARGS: %s\n", args);
+
+ /* Chew leading spaces */
+ while (*args == ' ') args++;

while (*args) {
int ret;


2005-09-26 13:32:47

by [email protected]

[permalink] [raw]
Subject: Re: usb-snd-audio breakage

So module and proc code will strip white space, but sysfs won't strip
white space. Where is the consistency?

On 9/25/05, Dave Jones <[email protected]> wrote:
> On Sun, Sep 25, 2005 at 10:43:11PM -0400, Jon Smirl wrote:
> > The Redhat FC4 installer is adds index=0 in modprobe.conf. The index
> > parameter appears to have been removed fron snd-usb-audio.
> >
> > There are two issues:
> > 1) should index have been left as a non-functioning param so that
> > existing installs won't break.
> > 2) Why didn't I get a decent error message about index being the
> > problem instead of the message about `'
>
> This patch really should have been merged for 2.6.13
> but somehow fell through the cracks. I don't think it
> even landed in -mm
>
> Dave
>
>
>
> Name: Ignore trailing whitespace on kernel parameters correctly: Fixed version
> Signed-off-by: Rusty Russell <[email protected]>
>
> Dave Jones says:
>
> ... if the modprobe.conf has trailing whitespace, modules fail to load
> with the following helpful message..
>
> snd_intel8x0: Unknown parameter `'
>
> Previous version truncated last argument.
>
> Index: linux-2.6.13-rc6-git7-Module/kernel/params.c
> ===================================================================
> --- linux-2.6.13-rc6-git7-Module.orig/kernel/params.c 2005-08-10 16:12:45.000000000 +1000
> +++ linux-2.6.13-rc6-git7-Module/kernel/params.c 2005-08-16 14:31:16.000000000 +1000
> @@ -80,8 +80,6 @@
> int in_quote = 0, quoted = 0;
> char *next;
>
> - /* Chew any extra spaces */
> - while (*args == ' ') args++;
> if (*args == '"') {
> args++;
> in_quote = 1;
> @@ -121,6 +119,9 @@
> next = args + i + 1;
> } else
> next = args + i;
> +
> + /* Chew up trailing spaces. */
> + while (*next == ' ') next++;
> return next;
> }
>
> @@ -134,6 +135,9 @@
> char *param, *val;
>
> DEBUGP("Parsing ARGS: %s\n", args);
> +
> + /* Chew leading spaces */
> + while (*args == ' ') args++;
>
> while (*args) {
> int ret;
>
>
>


--
Jon Smirl
[email protected]

2005-09-26 15:07:42

by Greg KH

[permalink] [raw]
Subject: Re: usb-snd-audio breakage

On Mon, Sep 26, 2005 at 09:32:43AM -0400, Jon Smirl wrote:
> So module

That's up to that maintainer.

> and proc code

That is not true at all.

> will strip white space, but sysfs won't strip white space. Where is
> the consistency?

If you want to strip whitespace for all of your subsystem's sysfs files,
a single function call will do this.

After thinking about this for a while, and seeing all of the different
iterations that the sysfs-whitespace-cleanup patch went through, I do
not want to add this to sysfs. It is very easy to add this to a
subsystem, or even provide a generic function to do this if you want to
(I'd be glad to add that to the sysfs core) but it's not for the core of
sysfs to do for all files.

thanks,

greg k-h

2005-09-26 15:54:35

by [email protected]

[permalink] [raw]
Subject: Re: usb-snd-audio breakage

On 9/26/05, Greg KH <[email protected]> wrote:
> On Mon, Sep 26, 2005 at 09:32:43AM -0400, Jon Smirl wrote:
> > So module
>
> That's up to that maintainer.
>
> > and proc code
>
> That is not true at all.

In one of the older threads about this someone pointed out two places
in /proc where it is done too. I tried searching for the message but I
can't find it.

> > will strip white space, but sysfs won't strip white space. Where is
> > the consistency?
>
> If you want to strip whitespace for all of your subsystem's sysfs files,
> a single function call will do this.
>
> After thinking about this for a while, and seeing all of the different
> iterations that the sysfs-whitespace-cleanup patch went through, I do
> not want to add this to sysfs. It is very easy to add this to a
> subsystem, or even provide a generic function to do this if you want to
> (I'd be glad to add that to the sysfs core) but it's not for the core of
> sysfs to do for all files.

I went through the iterations because I hadn't thought about the case
where people were assigning multi-line CR terminated values to sysfs
attributes and then using multiple reads to process the assignments
one line at a time. I had thought that sysfs was only supposed to
allow the assignment of single values.

--
Jon Smirl
[email protected]

2005-09-27 07:19:33

by Greg KH

[permalink] [raw]
Subject: Re: usb-snd-audio breakage

On Mon, Sep 26, 2005 at 11:54:31AM -0400, Jon Smirl wrote:
> On 9/26/05, Greg KH <[email protected]> wrote:
> > After thinking about this for a while, and seeing all of the different
> > iterations that the sysfs-whitespace-cleanup patch went through, I do
> > not want to add this to sysfs. It is very easy to add this to a
> > subsystem, or even provide a generic function to do this if you want to
> > (I'd be glad to add that to the sysfs core) but it's not for the core of
> > sysfs to do for all files.
>
> I went through the iterations because I hadn't thought about the case
> where people were assigning multi-line CR terminated values to sysfs
> attributes and then using multiple reads to process the assignments
> one line at a time. I had thought that sysfs was only supposed to
> allow the assignment of single values.

Yes, that is "heavily encouraged" to be the case. But as you found
out...

thanks,

greg k-h