Discussion:
nfs uiomove() conversion
Martin Natano
2016-01-27 21:01:29 UTC
Permalink
Below the uiomove() conversion for nfs. I didn't change the type of 'n'
to be size_t, because it never exceeds the maximum rpc size (nm_rsize),
which is an integer too. (Also, to avoid unnecessary code churn.)

Index: nfs/nfs_bio.c
===================================================================
RCS file: /cvs/src/sys/nfs/nfs_bio.c,v
retrieving revision 1.80
diff -u -p -u -r1.80 nfs_bio.c
--- nfs/nfs_bio.c 14 Mar 2015 03:38:52 -0000 1.80
+++ nfs/nfs_bio.c 27 Jan 2016 20:46:53 -0000
@@ -177,7 +177,7 @@ again:
return (error);
}
}
- n = min((unsigned)(biosize - on), uio->uio_resid);
+ n = ulmin(biosize - on, uio->uio_resid);
offdiff = np->n_size - uio->uio_offset;
if (offdiff < (off_t)n)
n = (int)offdiff;
@@ -211,7 +211,7 @@ again:
return (error);
}
}
- n = min(uio->uio_resid, NFS_MAXPATHLEN - bp->b_resid);
+ n = ulmin(uio->uio_resid, NFS_MAXPATHLEN - bp->b_resid);
got_buf = 1;
on = 0;
break;
@@ -223,7 +223,7 @@ again:
if (n > 0) {
if (!baddr)
baddr = bp->b_data;
- error = uiomovei(baddr + on, (int)n, uio);
+ error = uiomove(baddr + on, n, uio);
}

if (vp->v_type == VLNK)
@@ -318,7 +318,7 @@ nfs_write(void *v)
nfsstats.biocache_writes++;
lbn = uio->uio_offset / biosize;
on = uio->uio_offset & (biosize-1);
- n = min((unsigned)(biosize - on), uio->uio_resid);
+ n = ulmin(biosize - on, uio->uio_resid);
bn = lbn * (biosize / DEV_BSIZE);
again:
bp = nfs_getcacheblk(vp, bn, biosize, p);
@@ -349,7 +349,7 @@ again:
goto again;
}

- error = uiomovei((char *)bp->b_data + on, n, uio);
+ error = uiomove((char *)bp->b_data + on, n, uio);
if (error) {
bp->b_flags |= B_ERROR;
brelse(bp);
@@ -590,7 +590,7 @@ nfs_doio(struct buf *bp, struct proc *p)
len = np->n_size - ((((off_t)bp->b_blkno) << DEV_BSHIFT)
+ diff);
if (len > 0) {
- len = min(len, uiop->uio_resid);
+ len = ulmin(len, uiop->uio_resid);
memset((char *)bp->b_data + diff, 0, len);
bp->b_validend = diff + len;
} else
Index: nfs/nfs_subs.c
===================================================================
RCS file: /cvs/src/sys/nfs/nfs_subs.c,v
retrieving revision 1.128
diff -u -p -u -r1.128 nfs_subs.c
--- nfs/nfs_subs.c 16 Jun 2015 11:09:40 -0000 1.128
+++ nfs/nfs_subs.c 27 Jan 2016 20:46:58 -0000
@@ -712,8 +712,8 @@ nfsm_uiotombuf(struct mbuf **mp, struct
uiop->uio_rw = UIO_WRITE;

while (len) {
- xfer = min(len, M_TRAILINGSPACE(mb));
- uiomovei(mb_offset(mb), xfer, uiop);
+ xfer = ulmin(len, M_TRAILINGSPACE(mb));
+ uiomove(mb_offset(mb), xfer, uiop);
mb->m_len += xfer;
len -= xfer;
if (len > 0) {
Index: nfs/nfs_vnops.c
===================================================================
RCS file: /cvs/src/sys/nfs/nfs_vnops.c,v
retrieving revision 1.166
diff -u -p -u -r1.166 nfs_vnops.c
--- nfs/nfs_vnops.c 22 Dec 2015 21:36:57 -0000 1.166
+++ nfs/nfs_vnops.c 27 Jan 2016 20:47:05 -0000
@@ -2032,7 +2032,7 @@ nfs_readdir(void *v)
break;
}

- if ((error = uiomovei(dp, dp->d_reclen, uio)))
+ if ((error = uiomove(dp, dp->d_reclen, uio)))
break;

newoff = fxdr_hyper(&ndp->cookie[0]);

cheers,
natano
Stefan Kempf
2016-02-06 16:41:05 UTC
Permalink
Post by Martin Natano
Below the uiomove() conversion for nfs. I didn't change the type of 'n'
to be size_t, because it never exceeds the maximum rpc size (nm_rsize),
which is an integer too. (Also, to avoid unnecessary code churn.)
Looks good. It's easy to see in the code that (0 < on < biosize), so we
have (biosize - on > 0). Since these variables are ints, the result of
ulmin will fit into an int also.

Remaining comments see inline.
Post by Martin Natano
Index: nfs/nfs_bio.c
===================================================================
RCS file: /cvs/src/sys/nfs/nfs_bio.c,v
retrieving revision 1.80
diff -u -p -u -r1.80 nfs_bio.c
--- nfs/nfs_bio.c 14 Mar 2015 03:38:52 -0000 1.80
+++ nfs/nfs_bio.c 27 Jan 2016 20:46:53 -0000
return (error);
}
}
- n = min((unsigned)(biosize - on), uio->uio_resid);
+ n = ulmin(biosize - on, uio->uio_resid);
offdiff = np->n_size - uio->uio_offset;
if (offdiff < (off_t)n)
n = (int)offdiff;
return (error);
}
}
- n = min(uio->uio_resid, NFS_MAXPATHLEN - bp->b_resid);
+ n = ulmin(uio->uio_resid, NFS_MAXPATHLEN - bp->b_resid);
got_buf = 1;
on = 0;
break;
if (n > 0) {
if (!baddr)
baddr = bp->b_data;
- error = uiomovei(baddr + on, (int)n, uio);
+ error = uiomove(baddr + on, n, uio);
}
if (vp->v_type == VLNK)
@@ -318,7 +318,7 @@ nfs_write(void *v)
nfsstats.biocache_writes++;
lbn = uio->uio_offset / biosize;
on = uio->uio_offset & (biosize-1);
- n = min((unsigned)(biosize - on), uio->uio_resid);
+ n = ulmin(biosize - on, uio->uio_resid);
bn = lbn * (biosize / DEV_BSIZE);
bp = nfs_getcacheblk(vp, bn, biosize, p);
goto again;
}
- error = uiomovei((char *)bp->b_data + on, n, uio);
+ error = uiomove((char *)bp->b_data + on, n, uio);
if (error) {
bp->b_flags |= B_ERROR;
brelse(bp);
@@ -590,7 +590,7 @@ nfs_doio(struct buf *bp, struct proc *p)
len = np->n_size - ((((off_t)bp->b_blkno) << DEV_BSHIFT)
+ diff);
if (len > 0) {
- len = min(len, uiop->uio_resid);
+ len = ulmin(len, uiop->uio_resid);
memset((char *)bp->b_data + diff, 0, len);
bp->b_validend = diff + len;
} else
Index: nfs/nfs_subs.c
===================================================================
RCS file: /cvs/src/sys/nfs/nfs_subs.c,v
retrieving revision 1.128
diff -u -p -u -r1.128 nfs_subs.c
--- nfs/nfs_subs.c 16 Jun 2015 11:09:40 -0000 1.128
+++ nfs/nfs_subs.c 27 Jan 2016 20:46:58 -0000
@@ -712,8 +712,8 @@ nfsm_uiotombuf(struct mbuf **mp, struct
uiop->uio_rw = UIO_WRITE;
while (len) {
- xfer = min(len, M_TRAILINGSPACE(mb));
- uiomovei(mb_offset(mb), xfer, uiop);
+ xfer = ulmin(len, M_TRAILINGSPACE(mb));
+ uiomove(mb_offset(mb), xfer, uiop);
mb->m_len += xfer;
len -= xfer;
if (len > 0) {

Ok also. len is a size_t and we already had the M_TRAILINGSPACE
returns a positive value.
Post by Martin Natano
Index: nfs/nfs_vnops.c
===================================================================
RCS file: /cvs/src/sys/nfs/nfs_vnops.c,v
retrieving revision 1.166
diff -u -p -u -r1.166 nfs_vnops.c
--- nfs/nfs_vnops.c 22 Dec 2015 21:36:57 -0000 1.166
+++ nfs/nfs_vnops.c 27 Jan 2016 20:47:05 -0000
@@ -2032,7 +2032,7 @@ nfs_readdir(void *v)
break;
}
- if ((error = uiomovei(dp, dp->d_reclen, uio)))
+ if ((error = uiomove(dp, dp->d_reclen, uio)))
Simple because d_reclen is unsigned.

Loading...