On Mon, 02 Mar 2009 18:12:06 +0300 Vitaly Wool <vital@xxxxxxxxxxxxxxxxx> wrote: > Hello Mauro, > > below is the patch that removes the subaddr check against ARRAY_SIZE(chip->shadow.bytes). > Regardless of anything, the 'bytes' array is 64 bytes large so this check disables > easy standard programming (TDA9874A_ESP) which has a number of 255 which is hardly the > intended behavior. > > As a matter of fact, we can think of separate check for this case like > if (subaddr + 1 >= ARRAY_SIZE(chip->shadow.bytes) || > subaddr != 0xFF) { > ... /* weird register, refuse */ > } > but I'm not sure if there are no other special cases so for now I suggest to just disable > it. This patch is wrong, since it will allow the access of an inexistent position at the shadow array: chip->shadow.bytes[subaddr+1] = val; The proper fix is to increase the size of the shadow.bytes array to properly handle the subaddr = 0xff. Something like: -#define MAXREGS 64 +#define MAXREGS 256 Except for allocating a few more bytes, such patch won't have any other drawback. > > drivers/media/video/tvaudio.c | 17 +---------------- > 1 file changed, 1 insertion(+), 16 deletions(-) > > Signed-off-by: Vitaly Wool <vital@xxxxxxxxxxxxxxxxx> > > Index: linux-next/drivers/media/video/tvaudio.c > =================================================================== > --- linux-next.orig/drivers/media/video/tvaudio.c 2009-03-02 17:50:40.000000000 +0300 > +++ linux-next/drivers/media/video/tvaudio.c 2009-03-02 18:08:08.000000000 +0300 > @@ -169,13 +169,6 @@ > return -1; > } > } else { > - if (subaddr + 1 >= ARRAY_SIZE(chip->shadow.bytes)) { > - v4l2_info(sd, > - "Tried to access a non-existent register: %d\n", > - subaddr); > - return -EINVAL; > - } > - > v4l2_dbg(1, debug, sd, "chip_write: reg%d=0x%x\n", > subaddr, val); > chip->shadow.bytes[subaddr+1] = val; > @@ -198,16 +191,8 @@ > if (mask != 0) { > if (subaddr < 0) { > val = (chip->shadow.bytes[1] & ~mask) | (val & mask); > - } else { > - if (subaddr + 1 >= ARRAY_SIZE(chip->shadow.bytes)) { > - v4l2_info(sd, > - "Tried to access a non-existent register: %d\n", > - subaddr); > - return -EINVAL; > - } > - > + } else > val = (chip->shadow.bytes[subaddr+1] & ~mask) | (val & mask); > - } > } > return chip_write(chip, subaddr, val); > } > > Cheers, Mauro -- video4linux-list mailing list Unsubscribe mailto:video4linux-list-request@xxxxxxxxxx?subject=unsubscribe https://www.redhat.com/mailman/listinfo/video4linux-list