Martin Natano wrote:
> Below is a diff to convert videoread() to use uiomove() instead of
> uiomovei(). Note, that passing sc_fsize to ulmin() is not problematic,
> even though it is an int, because the value is always positive.
Yes, sc_fsize *should* always be positive. I had to convince myself
though :-) uvideo(4) sets sc_fsize. There are checks that make
sure that it's not larger than an allocated buffer. And sc_fsize
is used as length argument in a bcopy() in uvideo(4) also. If it was
somehow negative, the bcopy() would cause problems before we got to
the uiomove call below.
So I think this diff is ok.
> cheers,
> natano
>
> Index: dev/video.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/video.c,v
> retrieving revision 1.37
> diff -u -p -u -r1.37 video.c
> --- dev/video.c 29 Aug 2015 20:51:46 -0000 1.37
> +++ dev/video.c 1 Jan 2016 14:31:24 -0000
> @@ -136,7 +136,8 @@ int
> videoread(dev_t dev, struct uio *uio, int ioflag)
> {
> struct video_softc *sc;
> - int unit, error, size;
> + int unit, error;
> + size_t size;
>
> unit = VIDEOUNIT(dev);
> if (unit >= video_cd.cd_ndevs ||
> @@ -169,16 +170,13 @@ videoread(dev_t dev, struct uio *uio, in
> }
>
> /* move no more than 1 frame to userland, as per specification */
> - if (sc->sc_fsize < uio->uio_resid)
> - size = sc->sc_fsize;
> - else
> - size = uio->uio_resid;
> - error = uiomovei(sc->sc_fbuffer, size, uio);
> + size = ulmin(uio->uio_resid, sc->sc_fsize);
> + error = uiomove(sc->sc_fbuffer, size, uio);
> sc->sc_frames_ready--;
> if (error)
> return (error);
>
> - DPRINTF(("uiomove successfully done (%d bytes)\n", size));
> + DPRINTF(("uiomove successfully done (%zu bytes)\n", size));
>
> return (0);
> }
>
