Discussion:
bktr uiomove() conversion
Martin Natano
2016-01-24 11:31:58 UTC
Permalink
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.

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
Stefan Kempf
2016-02-08 05:38:20 UTC
Permalink
Post by Martin Natano
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.
Post by Martin Natano
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
Loading...