> 
> 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 -0000      1.73
> > +++ isofs/cd9660/cd9660_vnops.c     1 Jan 2016 17:45:50 -0000
> > @@ -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);
> >     }
> > 
> 
> 

Reply via email to