Martin Natano wrote:
> On Tue, Feb 02, 2016 at 07:30:08PM +0100, Stefan Kempf wrote:
> > Looks good. I agree with changing left to size_t. One small remark
> > though: size_t is defined as unsigned long. Do the DPRINTFs that print
> > the value of left have to be changed to use %zu in the format string?
> Said DDPRINTFs are in functions not touched by the diff, where 'left' is
> a cn_t. However, this got me thinking: The current code is a wild mix of
> off_t, cn_t and size_t variables for 'left' and the chunk size values
> ('tocopy', 'toread' and 'towrite').

Oh, I missed that, sorry.
 
> Please see the diff below. In addition to the uiomove() change, it also
> unifies the code in ntfs_subr.c to consistently make use of the size_t
> type for aforementioned variables. Note, that the upper bound is a
> variable called 'rsize' (a size_t) in all those cases.

Using the same type for those variables consistently sounds like a
good idea to me. Thanks for updating the diff.
 
> natano
> 
> 
> Index: ntfs/ntfs_subr.c
> ===================================================================
> RCS file: /cvs/src/sys/ntfs/ntfs_subr.c,v
> retrieving revision 1.44
> diff -u -p -u -r1.44 ntfs_subr.c
> --- ntfs/ntfs_subr.c  14 Mar 2015 03:38:52 -0000      1.44
> +++ ntfs/ntfs_subr.c  2 Feb 2016 20:09:53 -0000
> @@ -1340,7 +1340,8 @@ ntfs_writeattr_plain(struct ntfsmount *n
>  {
>       size_t          init;
>       int             error = 0;
> -     off_t           off = roff, left = rsize, towrite;
> +     off_t           off = roff;
> +     size_t          left = rsize, towrite;
>       caddr_t         data = rdata;
>       struct ntvattr *vap;
>       *initp = 0;
> @@ -1351,7 +1352,7 @@ ntfs_writeattr_plain(struct ntfsmount *n
>               if (error)
>                       return (error);
>               towrite = MIN(left, ntfs_cntob(vap->va_vcnend + 1) - off);
> -             DDPRINTF("ntfs_writeattr_plain: o: %lld, s: %lld "
> +             DDPRINTF("ntfs_writeattr_plain: o: %lld, s: %zu "
>                   "(%llu - %llu)\n", off, towrite,
>                   vap->va_vcnstart, vap->va_vcnend);
>               error = ntfs_writentvattr_plain(ntmp, ip, vap,
> @@ -1359,11 +1360,9 @@ ntfs_writeattr_plain(struct ntfsmount *n
>                                        towrite, data, &init, uio);
>               if (error) {
>                       DPRINTF("ntfs_writeattr_plain: ntfs_writentvattr_plain "
> -                         "failed: o: %d, s: %d\n",
> -                         (u_int32_t)off, (u_int32_t)towrite);
> -                     DPRINTF("ntfs_writeattr_plain: attrib: %d - %d\n",
> -                         (u_int32_t)vap->va_vcnstart,
> -                         (u_int32_t)vap->va_vcnend);
> +                         "failed: o: %lld, s: %zu\n", off, towrite);
> +                     DPRINTF("ntfs_writeattr_plain: attrib: %llu - %llu\n",
> +                         vap->va_vcnstart, vap->va_vcnend);
>                       ntfs_ntvattrrele(vap);
>                       break;
>               }
> @@ -1390,10 +1389,10 @@ ntfs_writentvattr_plain(struct ntfsmount
>       int             error = 0;
>       off_t           off;
>       int             cnt;
> -     cn_t            ccn, ccl, cn, left, cl;
> +     cn_t            ccn, ccl, cn, cl;
>       caddr_t         data = rdata;
>       struct buf     *bp;
> -     size_t          tocopy;
> +     size_t          left, tocopy;
>  
>       *initp = 0;
>  
> @@ -1414,7 +1413,7 @@ ntfs_writentvattr_plain(struct ntfsmount
>               ccn = vap->va_vruncn[cnt];
>               ccl = vap->va_vruncl[cnt];
>  
> -             DDPRINTF("ntfs_writentvattr_plain: left %llu, cn: 0x%llx, "
> +             DDPRINTF("ntfs_writentvattr_plain: left %zu, cn: 0x%llx, "
>                   "cl: %llu, off: %lld\n", left, ccn, ccl, off);
>  
>               if (ntfs_cntob(ccl) < off) {
> @@ -1440,7 +1439,7 @@ ntfs_writentvattr_plain(struct ntfsmount
>                       cl = ntfs_btocl(tocopy + off);
>                       KASSERT(cl == 1 && tocopy <= ntfs_cntob(1));
>                       DDPRINTF("ntfs_writentvattr_plain: write: cn: 0x%llx "
> -                         "cl: %llu, off: %lld len: %llu, left: %llu\n",
> +                         "cl: %llu, off: %lld len: %zu, left: %zu\n",
>                           cn, cl, off, tocopy, left);
>                       if ((off == 0) && (tocopy == ntfs_cntob(cl)))
>                       {
> @@ -1456,7 +1455,7 @@ ntfs_writentvattr_plain(struct ntfsmount
>                               }
>                       }
>                       if (uio) {
> -                             error = uiomovei(bp->b_data + off, tocopy, uio);
> +                             error = uiomove(bp->b_data + off, tocopy, uio);
>                               if (error != 0)
>                                       break;
>                       } else
> @@ -1495,10 +1494,10 @@ ntfs_readntvattr_plain(struct ntfsmount 
>       *initp = 0;
>       if (vap->va_flag & NTFS_AF_INRUN) {
>               int             cnt;
> -             cn_t            ccn, ccl, cn, left, cl;
> +             cn_t            ccn, ccl, cn, cl;
>               caddr_t         data = rdata;
>               struct buf     *bp;
> -             size_t          tocopy;
> +             size_t          left, tocopy;
>  
>               DDPRINTF("ntfs_readntvattr_plain: data in run: %lu chains\n",
>                   vap->va_vruncnt);
> @@ -1512,7 +1511,7 @@ ntfs_readntvattr_plain(struct ntfsmount 
>                       ccn = vap->va_vruncn[cnt];
>                       ccl = vap->va_vruncl[cnt];
>  
> -                     DDPRINTF("ntfs_readntvattr_plain: left %llu, "
> +                     DDPRINTF("ntfs_readntvattr_plain: left %zu, "
>                           "cn: 0x%llx, cl: %llu, off: %lld\n",
>                           left, ccn, ccl, off);
>  
> @@ -1542,8 +1541,8 @@ ntfs_readntvattr_plain(struct ntfsmount 
>  
>                                       DDPRINTF("ntfs_readntvattr_plain: "
>                                           "read: cn: 0x%llx cl: %llu, "
> -                                         "off: %lld, len: %llu, "
> -                                         "left: %llu\n",
> +                                         "off: %lld, len: %zu, "
> +                                         "left: %zu\n",
>                                           cn, cl, off, tocopy, left);
>                                       error = bread(ntmp->ntm_devvp,
>                                                     ntfs_cntobn(cn),
> @@ -1554,7 +1553,7 @@ ntfs_readntvattr_plain(struct ntfsmount 
>                                               return (error);
>                                       }
>                                       if (uio) {
> -                                             error = uiomovei(bp->b_data + 
> off,
> +                                             error = uiomove(bp->b_data + 
> off,
>                                                       tocopy, uio);
>                                               if (error != 0)
>                                                       break;
> @@ -1574,7 +1573,7 @@ ntfs_readntvattr_plain(struct ntfsmount 
>                               tocopy = MIN(left, ntfs_cntob(ccl) - off);
>                               DDPRINTF("ntfs_readntvattr_plain: hole: "
>                                   "ccn: 0x%llx ccl: %llu, off: %lld, "
> -                                 "len: %llu, left: %llu\n",
> +                                 "len: %zu, left: %zu\n",
>                                   ccn, ccl, off, tocopy, left);
>                               left -= tocopy;
>                               off = 0;
> @@ -1600,7 +1599,7 @@ ntfs_readntvattr_plain(struct ntfsmount 
>       } else {
>               DDPRINTF("ntfs_readnvattr_plain: data is in mft record\n");
>               if (uio) 
> -                     error = uiomovei(vap->va_datap + roff, rsize, uio);
> +                     error = uiomove(vap->va_datap + roff, rsize, uio);
>               else
>                       memcpy(rdata, vap->va_datap + roff, rsize);
>               *initp += rsize;
> @@ -1619,7 +1618,8 @@ ntfs_readattr_plain(struct ntfsmount *nt
>  {
>       size_t          init;
>       int             error = 0;
> -     off_t           off = roff, left = rsize, toread;
> +     off_t           off = roff;
> +     size_t          left = rsize, toread;
>       caddr_t         data = rdata;
>       struct ntvattr *vap;
>       *initp = 0;
> @@ -1630,19 +1630,17 @@ ntfs_readattr_plain(struct ntfsmount *nt
>               if (error)
>                       return (error);
>               toread = MIN(left, ntfs_cntob(vap->va_vcnend + 1) - off);
> -             DDPRINTF("ntfs_readattr_plain: o: %lld, s: %lld "
> +             DDPRINTF("ntfs_readattr_plain: o: %lld, s: %zu "
>                   "(%llu - %llu)\n", off, toread,
>                   vap->va_vcnstart, vap->va_vcnend);
>               error = ntfs_readntvattr_plain(ntmp, ip, vap,
>                                        off - ntfs_cntob(vap->va_vcnstart),
>                                        toread, data, &init, uio);
>               if (error) {
> -                     printf("ntfs_readattr_plain: " \
> -                            "ntfs_readntvattr_plain failed: o: %d, s: %d\n",
> -                            (u_int32_t) off, (u_int32_t) toread);
> -                     printf("ntfs_readattr_plain: attrib: %d - %d\n",
> -                            (u_int32_t) vap->va_vcnstart, 
> -                            (u_int32_t) vap->va_vcnend);
> +                     printf("ntfs_readattr_plain: ntfs_readntvattr_plain "
> +                         "failed: o: %lld, s: %zu\n", off, toread);
> +                     printf("ntfs_readattr_plain: attrib: %llu - %llu\n",
> +                            vap->va_vcnstart, vap->va_vcnend);
>                       ntfs_ntvattrrele(vap);
>                       break;
>               }
> @@ -1684,9 +1682,10 @@ ntfs_readattr(struct ntfsmount *ntmp, st
>       if (vap->va_compression && vap->va_compressalg) {
>               u_int8_t       *cup;
>               u_int8_t       *uup;
> -             off_t           off = roff, left = rsize, tocopy;
> +             off_t           off = roff;
>               caddr_t         data = rdata;
>               cn_t            cn;
> +             size_t          left = rsize, tocopy;
>  
>               DDPRINTF("ntfs_ntreadattr: compression: %u\n",
>                   vap->va_compressalg);
> @@ -1711,7 +1710,7 @@ ntfs_readattr(struct ntfsmount *ntmp, st
>  
>                       if (init == ntfs_cntob(NTFS_COMPUNIT_CL)) {
>                               if (uio)
> -                                     error = uiomovei(cup + off, tocopy, 
> uio);
> +                                     error = uiomove(cup + off, tocopy, uio);
>                               else
>                                       memcpy(data, cup + off, tocopy);
>                       } else if (init == 0) {
> @@ -1730,7 +1729,7 @@ ntfs_readattr(struct ntfsmount *ntmp, st
>                               if (error)
>                                       break;
>                               if (uio)
> -                                     error = uiomovei(uup + off, tocopy, 
> uio);
> +                                     error = uiomove(uup + off, tocopy, uio);
>                               else
>                                       memcpy(data, uup + off, tocopy);
>                       }

Reply via email to