Re: [Linux-fbdev-devel] [PATCH] fbdev: Wrong test on unsigned in fb_set_user_cmap()?

struct fb_cmap_user member start is unsigned. The same condition
is already checked in fb_set_cmap(), so this should be safe.

Signed-off-by: Roel Kluin <redacted>
---

quoted

Is this required?

Drop the whole if() as exactly the same condition is checked in the
fb_set_cmap() again.

Ok

Anyway, the check of the cmap->start < 0 does
not make any sense as the start is u32 value (most userspace
addresses will be lower then 2GB on 32 bit system so the error cannot
be caught by the check). I vote for removing the (cmap->start < 0) in
the fb_set_cmap as well as most drivers check the start value already
in driver's fb_setcolreg() function.

In fb_set_cmap() this is not `cmap->start' but `start' which has type
int. Therefore I think the test makes some sense there, so I left it.

Best regards,
Krzysztof

Thanks, here:

diff --git a/drivers/video/fbcmap.c b/drivers/video/fbcmap.c
index f53b9f1..f46d617 100644
--- a/drivers/video/fbcmap.c
+++ b/drivers/video/fbcmap.c

@@ -266,11 +266,6 @@ 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 &&
-				!info->fbops->fb_setcmap)) {
-		rc = -EINVAL;
-		goto out1;
-	}
 	rc = fb_set_cmap(&umap, info);
 out1:
 	unlock_fb_info(info);