Re: cd9660 uiomove() conversion
> > Diff reads good. ok? ok kettenis@ > Some thoughts (mostly for myself) inline. > > Martin Natano wrote: > > Below the uiomove conversion for isofs/cd9660/cd9660_vnops.c. > > > > cheers, > > natano > > > > Index: isofs/cd9660/cd9660_vnops.c > > === > > RCS file: /cvs/src/sys/isofs/cd9660/cd9660_vnops.c,v > > retrieving revision 1.73 > > diff -u -p -r1.73 cd9660_vnops.c > > --- isofs/cd9660/cd9660_vnops.c 11 Dec 2015 11:25:55 - 1.73 > > +++ isofs/cd9660/cd9660_vnops.c 1 Jan 2016 17:45:50 - > > @@ -227,7 +227,8 @@ cd9660_read(void *v) > > daddr_t lbn, rablock; > > off_t diff; > > int error = 0; > > - long size, n, on; > > + long size, on; > > + size_t n; > > > > if (uio->uio_resid == 0) > > return (0); > > @@ -240,8 +241,7 @@ cd9660_read(void *v) > > > > lbn = lblkno(imp, uio->uio_offset); > > on = blkoff(imp, uio->uio_offset); > > - n = min((u_int)(imp->logical_block_size - on), > > - uio->uio_resid); > > + n = ulmin(imp->logical_block_size - on, uio->uio_resid); > > The subtraction can't overflow, because blkoff is basically a > uio->uio_offset % imp->logical_block_size. ulmin() protects against > truncation. The result is always positive and making n size_t is > straightforward. > > > diff = (off_t)ip->i_size - uio->uio_offset; > > if (diff <= 0) > > return (0); > > @@ -270,13 +270,13 @@ cd9660_read(void *v) > > } else > > error = bread(vp, lbn, size, &bp); > > ci->ci_lastr = lbn; > > - n = min(n, size - bp->b_resid); > > + n = ulmin(n, size - bp->b_resid); > > bp->b_resid is supposed to be <= size because it's the > remaining I/O to get the whole buf filled with size bytes. > So bp->b_resid should be initialized with size initially > and then only decrease. > > > if (error) { > > brelse(bp); > > return (error); > > } > > > > - error = uiomovei(bp->b_data + on, (int)n, uio); > > + error = uiomove(bp->b_data + on, n, uio); > > Straightforward with n being a size_t. > > > brelse(bp); > > } while (error == 0 && uio->uio_resid > 0 && n != 0); > > @@ -344,7 +344,7 @@ iso_uiodir(idp,dp,off) > > } > > > > dp->d_off = off; > > - if ((error = uiomovei((caddr_t)dp, dp->d_reclen, idp->uio)) != 0) > > + if ((error = uiomove(dp, dp->d_reclen, idp->uio)) != 0) > > Straightforward. dp->d_reclen is unsigned. > > > return (error); > > idp->uio_off = off; > > return (0); > > @@ -657,7 +657,7 @@ cd9660_readlink(void *v) > > */ > > if (uio->uio_segflg != UIO_SYSSPACE || > > uio->uio_iov->iov_len < MAXPATHLEN) { > > - error = uiomovei(symname, symlen, uio); > > + error = uiomove(symname, symlen, uio); > > Straightforward, symlen is unsigned. Also did some checking that computation > of symlen does not wrap around also. > > > pool_put(&namei_pool, symname); > > return (error); > > } > > > >
Re: cd9660 uiomove() conversion
Diff reads good. ok? Some thoughts (mostly for myself) inline. Martin Natano wrote: > Below the uiomove conversion for isofs/cd9660/cd9660_vnops.c. > > cheers, > natano > > Index: isofs/cd9660/cd9660_vnops.c > === > RCS file: /cvs/src/sys/isofs/cd9660/cd9660_vnops.c,v > retrieving revision 1.73 > diff -u -p -r1.73 cd9660_vnops.c > --- isofs/cd9660/cd9660_vnops.c 11 Dec 2015 11:25:55 - 1.73 > +++ isofs/cd9660/cd9660_vnops.c 1 Jan 2016 17:45:50 - > @@ -227,7 +227,8 @@ cd9660_read(void *v) > daddr_t lbn, rablock; > off_t diff; > int error = 0; > - long size, n, on; > + long size, on; > + size_t n; > > if (uio->uio_resid == 0) > return (0); > @@ -240,8 +241,7 @@ cd9660_read(void *v) > > lbn = lblkno(imp, uio->uio_offset); > on = blkoff(imp, uio->uio_offset); > - n = min((u_int)(imp->logical_block_size - on), > - uio->uio_resid); > + n = ulmin(imp->logical_block_size - on, uio->uio_resid); The subtraction can't overflow, because blkoff is basically a uio->uio_offset % imp->logical_block_size. ulmin() protects against truncation. The result is always positive and making n size_t is straightforward. > diff = (off_t)ip->i_size - uio->uio_offset; > if (diff <= 0) > return (0); > @@ -270,13 +270,13 @@ cd9660_read(void *v) > } else > error = bread(vp, lbn, size, &bp); > ci->ci_lastr = lbn; > - n = min(n, size - bp->b_resid); > + n = ulmin(n, size - bp->b_resid); bp->b_resid is supposed to be <= size because it's the remaining I/O to get the whole buf filled with size bytes. So bp->b_resid should be initialized with size initially and then only decrease. > if (error) { > brelse(bp); > return (error); > } > > - error = uiomovei(bp->b_data + on, (int)n, uio); > + error = uiomove(bp->b_data + on, n, uio); Straightforward with n being a size_t. > brelse(bp); > } while (error == 0 && uio->uio_resid > 0 && n != 0); > @@ -344,7 +344,7 @@ iso_uiodir(idp,dp,off) > } > > dp->d_off = off; > - if ((error = uiomovei((caddr_t)dp, dp->d_reclen, idp->uio)) != 0) > + if ((error = uiomove(dp, dp->d_reclen, idp->uio)) != 0) Straightforward. dp->d_reclen is unsigned. > return (error); > idp->uio_off = off; > return (0); > @@ -657,7 +657,7 @@ cd9660_readlink(void *v) >*/ > if (uio->uio_segflg != UIO_SYSSPACE || > uio->uio_iov->iov_len < MAXPATHLEN) { > - error = uiomovei(symname, symlen, uio); > + error = uiomove(symname, symlen, uio); Straightforward, symlen is unsigned. Also did some checking that computation of symlen does not wrap around also. > pool_put(&namei_pool, symname); > return (error); > } >
cd9660 uiomove() conversion
Below the uiomove conversion for isofs/cd9660/cd9660_vnops.c. cheers, natano Index: isofs/cd9660/cd9660_vnops.c === RCS file: /cvs/src/sys/isofs/cd9660/cd9660_vnops.c,v retrieving revision 1.73 diff -u -p -r1.73 cd9660_vnops.c --- isofs/cd9660/cd9660_vnops.c 11 Dec 2015 11:25:55 - 1.73 +++ isofs/cd9660/cd9660_vnops.c 1 Jan 2016 17:45:50 - @@ -227,7 +227,8 @@ cd9660_read(void *v) daddr_t lbn, rablock; off_t diff; int error = 0; - long size, n, on; + long size, on; + size_t n; if (uio->uio_resid == 0) return (0); @@ -240,8 +241,7 @@ cd9660_read(void *v) lbn = lblkno(imp, uio->uio_offset); on = blkoff(imp, uio->uio_offset); - n = min((u_int)(imp->logical_block_size - on), - uio->uio_resid); + n = ulmin(imp->logical_block_size - on, uio->uio_resid); diff = (off_t)ip->i_size - uio->uio_offset; if (diff <= 0) return (0); @@ -270,13 +270,13 @@ cd9660_read(void *v) } else error = bread(vp, lbn, size, &bp); ci->ci_lastr = lbn; - n = min(n, size - bp->b_resid); + n = ulmin(n, size - bp->b_resid); if (error) { brelse(bp); return (error); } - error = uiomovei(bp->b_data + on, (int)n, uio); + error = uiomove(bp->b_data + on, n, uio); brelse(bp); } while (error == 0 && uio->uio_resid > 0 && n != 0); @@ -344,7 +344,7 @@ iso_uiodir(idp,dp,off) } dp->d_off = off; - if ((error = uiomovei((caddr_t)dp, dp->d_reclen, idp->uio)) != 0) + if ((error = uiomove(dp, dp->d_reclen, idp->uio)) != 0) return (error); idp->uio_off = off; return (0); @@ -657,7 +657,7 @@ cd9660_readlink(void *v) */ if (uio->uio_segflg != UIO_SYSSPACE || uio->uio_iov->iov_len < MAXPATHLEN) { - error = uiomovei(symname, symlen, uio); + error = uiomove(symname, symlen, uio); pool_put(&namei_pool, symname); return (error); }