zoran: invalid test on unsigned

fmt->index is unsigned. test doesn't work

---
Is there another test required?

index 092333b..0db5d0f 100644
@@ -1871,7 +1871,7 @@ static int zoran_enum_fmt(struct zoran *zr, struct v4l2_fmtdesc *fmt, int flag)
 		if (num == fmt->index)
 			break;
 	}
-	if (fmt->index < 0 /* late, but not too late */  || i == NUM_FORMATS)
+	if (i == NUM_FORMATS)
 		return -EINVAL;
 
 	strncpy(fmt->description, zoran_formats[i].name, sizeof(fmt->description)-1);

This is an old driver and I think back in v4l1 the indexes weren't all unsigned. There were a number of tests like this in it. Patch is fine.

That part of the patch is fine. It turns out there is another problem that already existed in this function. It's clearer with a few more lines of context.

        int num = -1, i;
        for (i = 0; i < NUM_FORMATS; i++) {
                if (zoran_formats[i].flags & flag)
                        num++;
                if (num == fmt->index)
                        break;
        }
      	if (i == NUM_FORMATS)
                return -EINVAL;
	/* stuff to return format i */

The v4l2 api enumerates formats separately for each buffer type, e.g. there is one list of formats for video capture and a different list for video output. The driver just has one list of formats and each format is flagged with the type(s) it can be used with. So when someone requests the capture format at index 2 we search the list and return the 3rd capture format we find. Since we don't know how many capture formats there are (NUM_FORMATS is the number of formats in the driver's single list, i.e. the union of all format types) we can't reject an index that is too large until after searching the whole list.

The problem with this code is if someone requests a format at fmt->index == (unsigned)-1. If the first format in the array doesn't have the requested type then num will still be -1 when it's compared to fmt->index and there will appear to be a match.

Here's a patch to redo the function that should fix everything:

zoran: fix bug when enumerating format -1

If someone requests a format at fmt->index == (unsigned)-1 and the first format in the array doesn't have the requested type then num will still be -1 when it's compared to fmt->index and there will appear to be a match.

Restructure the loop so this can't happen. It's simpler this way too. The unnecessary check for (unsigned)fmt->index < 0 found by Roel Kluin <[email protected]> is removed this way too.

diff -r 63eba6df4b8a -r c247021eb11c linux/drivers/media/video/zoran/zoran_driver.c
@@ -1871,22 +1871,20 @@ static int zoran_querycap(struct file *f
 
 static int zoran_enum_fmt(struct zoran *zr, struct v4l2_fmtdesc *fmt, int flag)
 {
-       int num = -1, i;
-
-       for (i = 0; i < NUM_FORMATS; i++) {
-               if (zoran_formats[i].flags & flag)
-                       num++;
-               if (num == fmt->index)
-                       break;
-       }
-       if (fmt->index < 0 /* late, but not too late */  || i == NUM_FORMATS)
-               return -EINVAL;
-
-       strncpy(fmt->description, zoran_formats[i].name, sizeof(fmt->description)-1);
-       fmt->pixelformat = zoran_formats[i].fourcc;
-       if (zoran_formats[i].flags & ZORAN_FORMAT_COMPRESSED)
-               fmt->flags |= V4L2_FMT_FLAG_COMPRESSED;
-       return 0;
+       unsigned int num, i;
+
+       for (num = i = 0; i < NUM_FORMATS; i++) {
+               if (zoran_formats[i].flags & flag && num++ == fmt->index) {
+                       strncpy(fmt->description, zoran_formats[i].name,
+                               sizeof(fmt->description) - 1);
+                       /* fmt struct pre-zeroed, so adding '\0' not neeed */
+                       fmt->pixelformat = zoran_formats[i].fourcc;
+                       if (zoran_formats[i].flags & ZORAN_FORMAT_COMPRESSED)
+                               fmt->flags |= V4L2_FMT_FLAG_COMPRESSED;
+                       return 0;
+               }
+       }
+       return -EINVAL;
 }
 
 static int zoran_enum_fmt_vid_cap(struct file *file, void *__fh,