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);
> }