Hello,
I just noticed two framebuffer drivers with strtok calls that somehow
passed below my radar (cscope). Patch below converts them, too. And it
re-adds "ignore empty tokens" functionalty, which I forgot about the
last time. Please apply.
Ren?
diff -Nur ../linux-2.4.13-pre6/drivers/video/amifb.c ./drivers/video/amifb.c
--- ../linux-2.4.13-pre6/drivers/video/amifb.c Tue Oct 23 22:13:43 2001
+++ ./drivers/video/amifb.c Tue Oct 23 23:04:03 2001
@@ -1193,6 +1193,8 @@
return 0;
while (this_opt = strsep(&options, ",")) {
+ if (!*this_opt)
+ continue;
if (!strcmp(this_opt, "inverse")) {
amifb_inverse = 1;
fb_invert_cmaps();
diff -Nur ../linux-2.4.13-pre6/drivers/video/atafb.c ./drivers/video/atafb.c
--- ../linux-2.4.13-pre6/drivers/video/atafb.c Fri Sep 14 01:04:43 2001
+++ ./drivers/video/atafb.c Tue Oct 23 23:00:59 2001
@@ -2899,7 +2899,7 @@
if (!options || !*options)
return 0;
- for(this_opt=strtok(options,","); this_opt; this_opt=strtok(NULL,",")) {
+ while (this_opt = strsep(&options, ",")) {
if (!*this_opt) continue;
if ((temp=get_video_mode(this_opt)))
default_par=temp;
diff -Nur ../linux-2.4.13-pre6/drivers/video/aty/atyfb_base.c ./drivers/video/aty/atyfb_base.c
--- ../linux-2.4.13-pre6/drivers/video/aty/atyfb_base.c Tue Oct 23 22:13:43 2001
+++ ./drivers/video/aty/atyfb_base.c Tue Oct 23 23:05:24 2001
@@ -2522,6 +2522,8 @@
return 0;
while (this_opt = strsep(&options, ",")) {
+ if (!*this_opt)
+ continue;
if (!strncmp(this_opt, "font:", 5)) {
char *p;
int i;
diff -Nur ../linux-2.4.13-pre6/drivers/video/aty128fb.c ./drivers/video/aty128fb.c
--- ../linux-2.4.13-pre6/drivers/video/aty128fb.c Tue Oct 23 22:13:43 2001
+++ ./drivers/video/aty128fb.c Tue Oct 23 23:06:17 2001
@@ -1614,6 +1614,8 @@
return 0;
while (this_opt = strsep(&options, ",")) {
+ if (!*this_opt)
+ continue;
if (!strncmp(this_opt, "font:", 5)) {
char *p;
int i;
diff -Nur ../linux-2.4.13-pre6/drivers/video/clgenfb.c ./drivers/video/clgenfb.c
--- ../linux-2.4.13-pre6/drivers/video/clgenfb.c Wed Oct 10 00:13:02 2001
+++ ./drivers/video/clgenfb.c Tue Oct 23 22:59:38 2001
@@ -2817,8 +2817,7 @@
if (!options || !*options)
return 0;
- for (this_opt = strtok (options, ","); this_opt != NULL;
- this_opt = strtok (NULL, ",")) {
+ while (this_opt = strsep (&options, ",")) {
if (!*this_opt) continue;
DPRINTK("clgenfb_setup: option '%s'\n", this_opt);
diff -Nur ../linux-2.4.13-pre6/drivers/video/cyberfb.c ./drivers/video/cyberfb.c
--- ../linux-2.4.13-pre6/drivers/video/cyberfb.c Tue Oct 23 22:13:43 2001
+++ ./drivers/video/cyberfb.c Tue Oct 23 23:07:42 2001
@@ -1023,6 +1023,8 @@
}
while (this_opt = strsep(&options, ",")) {
+ if (!*this_opt)
+ continue;
if (!strcmp(this_opt, "inverse")) {
Cyberfb_inverse = 1;
fb_invert_cmaps();
diff -Nur ../linux-2.4.13-pre6/drivers/video/radeonfb.c ./drivers/video/radeonfb.c
--- ../linux-2.4.13-pre6/drivers/video/radeonfb.c Tue Oct 23 22:13:43 2001
+++ ./drivers/video/radeonfb.c Tue Oct 23 23:09:58 2001
@@ -538,6 +538,8 @@
return 0;
while (this_opt = strsep (&options, ",")) {
+ if (!*this_opt)
+ continue;
if (!strncmp (this_opt, "font:", 5)) {
char *p;
int i;
diff -Nur ../linux-2.4.13-pre6/drivers/video/retz3fb.c ./drivers/video/retz3fb.c
--- ../linux-2.4.13-pre6/drivers/video/retz3fb.c Tue Oct 23 22:13:43 2001
+++ ./drivers/video/retz3fb.c Tue Oct 23 23:30:56 2001
@@ -1349,6 +1349,8 @@
return 0;
while (this_opt = strsep(&options, ",")) {
+ if (!*this_opt)
+ continue;
if (!strcmp(this_opt, "inverse")) {
z3fb_inverse = 1;
fb_invert_cmaps();
diff -Nur ../linux-2.4.13-pre6/drivers/video/riva/fbdev.c ./drivers/video/riva/fbdev.c
--- ../linux-2.4.13-pre6/drivers/video/riva/fbdev.c Tue Oct 23 22:13:43 2001
+++ ./drivers/video/riva/fbdev.c Tue Oct 23 23:31:33 2001
@@ -2046,6 +2046,8 @@
return 0;
while (this_opt = strsep(&options, ",")) {
+ if (!*this_opt)
+ continue;
if (!strncmp(this_opt, "font:", 5)) {
char *p;
int i;
diff -Nur ../linux-2.4.13-pre6/drivers/video/tdfxfb.c ./drivers/video/tdfxfb.c
--- ../linux-2.4.13-pre6/drivers/video/tdfxfb.c Tue Oct 23 22:13:43 2001
+++ ./drivers/video/tdfxfb.c Tue Oct 23 23:32:35 2001
@@ -2087,6 +2087,8 @@
return;
while(this_opt = strsep(&options, ",")) {
+ if(!*this_opt)
+ continue;
if(!strcmp(this_opt, "inverse")) {
inverse = 1;
fb_invert_cmaps();
diff -Nur ../linux-2.4.13-pre6/drivers/video/virgefb.c ./drivers/video/virgefb.c
--- ../linux-2.4.13-pre6/drivers/video/virgefb.c Tue Oct 23 22:13:43 2001
+++ ./drivers/video/virgefb.c Tue Oct 23 23:33:40 2001
@@ -1086,6 +1086,8 @@
return 0;
while (this_opt = strsep(&options, ",")) {
+ if (!*this_opt)
+ continue;
if (!strcmp(this_opt, "inverse")) {
Cyberfb_inverse = 1;
fb_invert_cmaps();
Ren? Scharfe wrote:
>
> Hello,
>
> I just noticed two framebuffer drivers with strtok calls that somehow
> passed below my radar (cscope). Patch below converts them, too. And it
> re-adds "ignore empty tokens" functionalty, which I forgot about the
> last time. Please apply.
>
> Ren?
>
> diff -Nur ../linux-2.4.13-pre6/drivers/video/amifb.c ./drivers/video/amifb.c
> --- ../linux-2.4.13-pre6/drivers/video/amifb.c Tue Oct 23 22:13:43 2001
> +++ ./drivers/video/amifb.c Tue Oct 23 23:04:03 2001
> @@ -1193,6 +1193,8 @@
> return 0;
>
> while (this_opt = strsep(&options, ",")) {
> + if (!*this_opt)
> + continue;
> if (!strcmp(this_opt, "inverse")) {
> amifb_inverse = 1;
> fb_invert_cmaps();
> diff -Nur ../linux-2.4.13-pre6/drivers/video/atafb.c ./drivers/video/atafb.c
> --- ../linux-2.4.13-pre6/drivers/video/atafb.c Fri Sep 14 01:04:43 2001
> +++ ./drivers/video/atafb.c Tue Oct 23 23:00:59 2001
> @@ -2899,7 +2899,7 @@
> if (!options || !*options)
> return 0;
>
> - for(this_opt=strtok(options,","); this_opt; this_opt=strtok(NULL,",")) {
> + while (this_opt = strsep(&options, ",")) {
> if (!*this_opt) continue;
> if ((temp=get_video_mode(this_opt)))
> default_par=temp;
> diff -Nur ../linux-2.4.13-pre6/drivers/video/aty/atyfb_base.c ./drivers/video/aty/atyfb_base.c
> --- ../linux-2.4.13-pre6/drivers/video/aty/atyfb_base.c Tue Oct 23 22:13:43 2001
> +++ ./drivers/video/aty/atyfb_base.c Tue Oct 23 23:05:24 2001
> @@ -2522,6 +2522,8 @@
> return 0;
>
> while (this_opt = strsep(&options, ",")) {
> + if (!*this_opt)
> + continue;
> if (!strncmp(this_opt, "font:", 5)) {
> char *p;
> int i;
> diff -Nur ../linux-2.4.13-pre6/drivers/video/aty128fb.c ./drivers/video/aty128fb.c
> --- ../linux-2.4.13-pre6/drivers/video/aty128fb.c Tue Oct 23 22:13:43 2001
> +++ ./drivers/video/aty128fb.c Tue Oct 23 23:06:17 2001
> @@ -1614,6 +1614,8 @@
> return 0;
>
> while (this_opt = strsep(&options, ",")) {
> + if (!*this_opt)
> + continue;
> if (!strncmp(this_opt, "font:", 5)) {
> char *p;
> int i;
> diff -Nur ../linux-2.4.13-pre6/drivers/video/clgenfb.c ./drivers/video/clgenfb.c
> --- ../linux-2.4.13-pre6/drivers/video/clgenfb.c Wed Oct 10 00:13:02 2001
> +++ ./drivers/video/clgenfb.c Tue Oct 23 22:59:38 2001
> @@ -2817,8 +2817,7 @@
> if (!options || !*options)
> return 0;
>
> - for (this_opt = strtok (options, ","); this_opt != NULL;
> - this_opt = strtok (NULL, ",")) {
> + while (this_opt = strsep (&options, ",")) {
> if (!*this_opt) continue;
>
> DPRINTK("clgenfb_setup: option '%s'\n", this_opt);
> diff -Nur ../linux-2.4.13-pre6/drivers/video/cyberfb.c ./drivers/video/cyberfb.c
> --- ../linux-2.4.13-pre6/drivers/video/cyberfb.c Tue Oct 23 22:13:43 2001
> +++ ./drivers/video/cyberfb.c Tue Oct 23 23:07:42 2001
> @@ -1023,6 +1023,8 @@
> }
>
> while (this_opt = strsep(&options, ",")) {
> + if (!*this_opt)
> + continue;
> if (!strcmp(this_opt, "inverse")) {
> Cyberfb_inverse = 1;
> fb_invert_cmaps();
> diff -Nur ../linux-2.4.13-pre6/drivers/video/radeonfb.c ./drivers/video/radeonfb.c
> --- ../linux-2.4.13-pre6/drivers/video/radeonfb.c Tue Oct 23 22:13:43 2001
> +++ ./drivers/video/radeonfb.c Tue Oct 23 23:09:58 2001
> @@ -538,6 +538,8 @@
> return 0;
>
> while (this_opt = strsep (&options, ",")) {
> + if (!*this_opt)
> + continue;
> if (!strncmp (this_opt, "font:", 5)) {
> char *p;
> int i;
> diff -Nur ../linux-2.4.13-pre6/drivers/video/retz3fb.c ./drivers/video/retz3fb.c
> --- ../linux-2.4.13-pre6/drivers/video/retz3fb.c Tue Oct 23 22:13:43 2001
> +++ ./drivers/video/retz3fb.c Tue Oct 23 23:30:56 2001
> @@ -1349,6 +1349,8 @@
> return 0;
>
> while (this_opt = strsep(&options, ",")) {
> + if (!*this_opt)
> + continue;
> if (!strcmp(this_opt, "inverse")) {
> z3fb_inverse = 1;
> fb_invert_cmaps();
> diff -Nur ../linux-2.4.13-pre6/drivers/video/riva/fbdev.c ./drivers/video/riva/fbdev.c
> --- ../linux-2.4.13-pre6/drivers/video/riva/fbdev.c Tue Oct 23 22:13:43 2001
> +++ ./drivers/video/riva/fbdev.c Tue Oct 23 23:31:33 2001
> @@ -2046,6 +2046,8 @@
> return 0;
>
> while (this_opt = strsep(&options, ",")) {
> + if (!*this_opt)
> + continue;
NAME
strsep - extract token from string
[...]
RETURN VALUE
The strsep() function returns a pointer to the token, or
NULL if delim is not found in stringp.
If strsep returns NULL, and you dereference it -> Oops.
! if (!this_opt)
continue;
> -----Original Message-----
> From: Peter W?chtler [mailto:[email protected]]
> Sent: 25 October 2001 15:36
> To: Ren? Scharfe
> Cc: Linus Torvalds; Linux Kernel Mailing List
> Subject: Re: [PATCH] strtok --> strsep in framebuffer drivers (part 2)
>
> Ren? Scharfe wrote:
> >
> > Hello,
> >
> > I just noticed two framebuffer drivers with strtok calls
> > that somehow passed below my radar (cscope). Patch below
> > converts them, too. And it re-adds "ignore empty tokens"
> > functionalty, which I forgot about the last time. Please apply.
> >
> > Ren?
[snip]
> > diff -Nur ../linux-2.4.13-pre6/drivers/video/riva/fbdev.c
> ./drivers/video/riva/fbdev.c
> > --- ../linux-2.4.13-pre6/drivers/video/riva/fbdev.c Tue
> Oct 23 22:13:43 2001
> > +++ ./drivers/video/riva/fbdev.c Tue Oct 23 23:31:33 2001
> > @@ -2046,6 +2046,8 @@
> > return 0;
> >
> > while (this_opt = strsep(&options, ",")) {
> > + if (!*this_opt)
> > + continue;
>
> NAME
> strsep - extract token from string
> [...]
> RETURN VALUE
> The strsep() function returns a pointer to the token, or
> NULL if delim is not found in stringp.
>
> If strsep returns NULL, and you dereference it -> Oops.
>
>
> ! if (!this_opt)
> continue;
If strsep() returns NULL, then the while-loop will terminate.
Thus this is already taken care of.
//Peter
> Ren? Scharfe wrote:
> NAME
> strsep - extract token from string
> [...]
> RETURN VALUE
> The strsep() function returns a pointer to the token, or
> NULL if delim is not found in stringp.
>
> If strsep returns NULL, and you dereference it -> Oops.
>
>
> ! if (!this_opt)
> continue;
Is that manual page incorrect?
I would think that
strsep() returns NULL when *stringp points to '\0' character
and that if delim is not found in stringp then stringp
is just advanced to '\0' character of string (and original
*stringp value is returned)
If that is not true, then these all patches are incorrect.
Namely last token will be skipped.
--
/"\ | Kari
\ / ASCII Ribbon Campaign | Hurtta
X Against HTML Mail |
/ \ |
> > Ren? Scharfe wrote:
>
> > NAME
> > strsep - extract token from string
> > [...]
> > RETURN VALUE
> > The strsep() function returns a pointer to the token, or
> > NULL if delim is not found in stringp.
> >
> > If strsep returns NULL, and you dereference it -> Oops.
> >
> >
> > ! if (!this_opt)
> > continue;
>
> Is that manual page incorrect?
>
> I would think that
>
> strsep() returns NULL when *stringp points to '\0' character
>
> and that if delim is not found in stringp then stringp
> is just advanced to '\0' character of string (and original
> *stringp value is returned)
>
> If that is not true, then these all patches are incorrect.
> Namely last token will be skipped.
Seems that I have also good to talk to mayself....
lib/string.c tells truth.
strsep() returns NULL when *stringp is NULL.
if delim is not found *stringp is set to NULL (and original
*stringp value is returned).
Because kernel code claims that
* It returns empty tokens, too, behaving exactly like the libc function
* of that name. In fact, it was stolen from glibc2 and de-fancy-fied.
* Same semantics, slimmer shape. ;)
quoted manual page IS incorrect (at least if it describes glibc2 function.)
--
/"\ | Kari
\ / ASCII Ribbon Campaign | Hurtta
X Against HTML Mail |
/ \ |
On Sat, Oct 27, 2001 at 10:55:09AM +0300, Kari Hurtta wrote:
> manual page IS incorrect (at least if it describes glibc2 function.)
If you think anything is wrong, write to the manpage maintainer
([email protected]) what you think is wrong and why.
> On Sat, Oct 27, 2001 at 10:55:09AM +0300, Kari Hurtta wrote:
>
> > manual page IS incorrect (at least if it describes glibc2 function.)
>
> If you think anything is wrong, write to the manpage maintainer
> ([email protected]) what you think is wrong and why.
I do not know from where that manual page text was quoted.
According of Ren_ Scharfe, man page text is correct on Mandrake 8.
Well, same error is on man page on this system
(Linux Mandrake release 7.1 (helium)).
So is that fixed by distributor (of Mandrake) or is it fixed by
manpage maintainer ?
--
/"\ | Kari
\ / ASCII Ribbon Campaign | Hurtta
X Against HTML Mail |
/ \ |
On Mon, Oct 29, 2001 at 09:12:10AM +0200, Kari Hurtta wrote:
> Well, same error is on man page on this system
> (Linux Mandrake release 7.1 (helium)).
I have no idea what you mean by "same error", but
let me repeat: if you think that anything is wrong,
then write to the manpage maintainer ([email protected])
and tell him precisely what you think is wrong and why.
Vague mutterings on a list like linux-kernel are unlikely
to improve anything, if, indeed, improvement is needed.