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, );
> > 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(_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, );
> 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(_pool, symname);
> return (error);
> }
>
