strtok --> strsep in framebuffer drivers (part 2)

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

drivers/video/amifb.c Tue Oct 23 23:04:03 2001 renamed from linux-2.4.13-pre6/drivers/video/amifb.c Tue Oct 23 22:13:43 2001 +2 -0

@@ -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

drivers/video/atafb.c Tue Oct 23 23:00:59 2001 renamed from linux-2.4.13-pre6/drivers/video/atafb.c Fri Sep 14 01:04:43 2001 +1 -1

@@ -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

drivers/video/aty/atyfb_base.c Tue Oct 23 23:05:24 2001 renamed from linux-2.4.13-pre6/drivers/video/aty/atyfb_base.c Tue Oct 23 22:13:43 2001 +2 -0

@@ -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

drivers/video/aty128fb.c Tue Oct 23 23:06:17 2001 renamed from linux-2.4.13-pre6/drivers/video/aty128fb.c Tue Oct 23 22:13:43 2001 +2 -0

@@ -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

drivers/video/clgenfb.c Tue Oct 23 22:59:38 2001 renamed from linux-2.4.13-pre6/drivers/video/clgenfb.c Wed Oct 10 00:13:02 2001 +1 -2

@@ -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

drivers/video/cyberfb.c Tue Oct 23 23:07:42 2001 renamed from linux-2.4.13-pre6/drivers/video/cyberfb.c Tue Oct 23 22:13:43 2001 +2 -0

@@ -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

drivers/video/radeonfb.c Tue Oct 23 23:09:58 2001 renamed from linux-2.4.13-pre6/drivers/video/radeonfb.c Tue Oct 23 22:13:43 2001 +2 -0

@@ -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

drivers/video/retz3fb.c Tue Oct 23 23:30:56 2001 renamed from linux-2.4.13-pre6/drivers/video/retz3fb.c Tue Oct 23 22:13:43 2001 +2 -0

@@ -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

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;

Quoted 9 lines

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.

Quoted 5 lines

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.

manual page IS incorrect (at least if it describes glibc2 function.)

Quoted 2 lines

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.

Well, same error is on man page on this system
(Linux Mandrake release 7.1 (helium)).