Skip to contentSkip to conversation

fbdev: Wrong test on unsigned in fb_set_user_cmap()?

PATCH In review Roel Kluin 2 participants 2 messages Oct 20, 2009 – Nov 3, 2009 Framebuffer layer

lore
Share & tools
lore

Roel Kluin

author
#0 lore raw

PATCH 1/1 fbdev: Wrong test on unsigned in fb_set_user_cmap()?

struct fb_cmap_user member start is unsigned.

Signed-off-by Roel Kluin <[email protected]>

---
Is this required?

drivers/video/fbcmap.c +1 -1
index f53b9f1..f46f05f 100644
@@ -266,7 +266,7 @@ int fb_set_user_cmap(struct fb_cmap_user *cmap, struct fb_info *info)
 		rc = -ENODEV;
 		goto out;
 	}
-	if (cmap->start < 0 || (!info->fbops->fb_setcolreg &&
+	if ((int)cmap->start < 0 || (!info->fbops->fb_setcolreg &&
 				!info->fbops->fb_setcmap)) {
 		rc = -EINVAL;
 		goto out1;
#1 lore raw
Quoted 11 lines · On Tue, 20 Oct 2009 21:30:38 +0200 Roel Kluin <[email protected]> wrote:

struct fb_cmap_user member start is unsigned.

Signed-off-by Roel Kluin <[email protected]>

---
Is this required?

drivers/video/fbcmap.c +0 -0
index f53b9f1..f46f05f 100644

drivers/video/fbcmap.c int fb_set_user_cmap(struct fb_cmap_user *cmap, struct fb_info *info) · line 272

Quoted 8 lines
drivers/video/fbcmap.c +1 -1
@@ -266,7 +266,7 @@ int fb_set_user_cmap(struct fb_cmap_user *cmap, struct fb_info *info)
 		rc = -ENODEV;
 		goto out;
 	}
-	if (cmap->start < 0 || (!info->fbops->fb_setcolreg &&
+	if ((int)cmap->start < 0 || (!info->fbops->fb_setcolreg &&
 				!info->fbops->fb_setcmap)) {
 		rc = -EINVAL;
 		goto out1;

I think the test _is_ needed, as `start' is passed in direct from userspace, via do_fb_ioctl():FBIOPUTCMAP.

However fb_set_user_cmap() calls fb_set_cmap() which also checks `start', only this time it does it correctly.

So overall the code is OK and the check which you've identified is unneeded.

In fact, all of

	if (cmap->start < 0 || (!info->fbops->fb_setcolreg &&
				!info->fbops->fb_setcmap)) {
		rc = -EINVAL;
		goto out1;
	}

can be removed.