Martin Natano wrote:
> Below the conversion from uiomovei() to uiomove() for bktr. The patch
> also replaces two occurrences of uio->uio_iov->iov_len with
> uio->uio_resid. I don't see a reason why bktr should inspect iov_len
> directly.

Looks good. As for computing count, we have 0 <= bktr->rows <= 0x7fe,
0 <= bktr->cols <= 0x3fe and pixfmt_table[bktr->pixfmt[.public.Bpp is 4
at most. So count is always positive even with an int and changing
it to size_t and calling uiomove keeps the same behavior as today.

Checking uio_resid and using ulmin also makes sense. bktr->vbisize is
bounded by VBI_BUFFER_SIZE and uio_resid is expected to be the sum
of the iov_len when video_read or vbi_read is called.
 
> Index: dev/pci/bktr/bktr_core.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/bktr/bktr_core.c,v
> retrieving revision 1.36
> diff -u -p -u -r1.36 bktr_core.c
> --- dev/pci/bktr/bktr_core.c  14 Mar 2015 03:38:49 -0000      1.36
> +++ dev/pci/bktr/bktr_core.c  24 Jan 2016 11:25:49 -0000
> @@ -993,7 +993,7 @@ int
>  video_read(bktr_ptr_t bktr, int unit, dev_t dev, struct uio *uio)
>  {
>          int             status;
> -        int             count;
> +        size_t          count;
>  
>  
>       if (bktr->bigbuf == 0)  /* no frame buffer allocated (ioctl failed) */
> @@ -1008,7 +1008,7 @@ video_read(bktr_ptr_t bktr, int unit, de
>       count = bktr->rows * bktr->cols *
>               pixfmt_table[ bktr->pixfmt ].public.Bpp;
>  
> -     if ((int) uio->uio_iov->iov_len < count)
> +     if (uio->uio_resid < count)
>               return( EINVAL );
>  
>       bktr->flags &= ~(METEOR_CAP_MASK | METEOR_WANT_MASK);
> @@ -1027,7 +1027,7 @@ video_read(bktr_ptr_t bktr, int unit, de
>  
>       status = tsleep(BKTR_SLEEP, BKTRPRI, "captur", 0);
>       if (!status)            /* successful capture */
> -             status = uiomovei((caddr_t)bktr->bigbuf, count, uio);
> +             status = uiomove((caddr_t)bktr->bigbuf, count, uio);
>       else
>               printf ("%s: read: tsleep error %d\n",
>                       bktr_name(bktr), status);
> @@ -1047,7 +1047,7 @@ video_read(bktr_ptr_t bktr, int unit, de
>  int
>  vbi_read(bktr_ptr_t bktr, struct uio *uio, int ioflag)
>  {
> -     int             readsize, readsize2;
> +     size_t          readsize, readsize2;
>       int             status;
>  
>  
> @@ -1067,22 +1067,20 @@ vbi_read(bktr_ptr_t bktr, struct uio *ui
>       /* We cannot read more bytes than there are in
>        * the circular buffer
>        */
> -     readsize = (int)uio->uio_iov->iov_len;
> -
> -     if (readsize > bktr->vbisize) readsize = bktr->vbisize;
> +     readsize = ulmin(uio->uio_resid, bktr->vbisize);
>  
>       /* Check if we can read this number of bytes without having
>        * to wrap around the circular buffer */
> -     if((bktr->vbistart + readsize) >= VBI_BUFFER_SIZE) {
> +     if (readsize >= VBI_BUFFER_SIZE - bktr->vbistart) {
>               /* We need to wrap around */
>  
>               readsize2 = VBI_BUFFER_SIZE - bktr->vbistart;
> -             status = uiomovei((caddr_t)bktr->vbibuffer + bktr->vbistart, 
> readsize2, uio);
> +             status = uiomove((caddr_t)bktr->vbibuffer + bktr->vbistart, 
> readsize2, uio);
>               if (status == 0)
> -                     status = uiomovei((caddr_t)bktr->vbibuffer, (readsize - 
> readsize2), uio);
> +                     status = uiomove((caddr_t)bktr->vbibuffer, (readsize - 
> readsize2), uio);
>       } else {
>               /* We do not need to wrap around */
> -             status = uiomovei((caddr_t)bktr->vbibuffer + bktr->vbistart, 
> readsize, uio);
> +             status = uiomove((caddr_t)bktr->vbibuffer + bktr->vbistart, 
> readsize, uio);
>       }
>  
>       /* Update the number of bytes left to read */
> 
> cheers,
> natano
> 

Reply via email to