Date: Fri, 2 Jun 95 20:08:14 MDT From: terry@cs.weber.edu (Terry Lambert) To: freebsd-bugs@FreeBSD.org Cc: terry@cs.weber.edu (Terry Lambert) Subject: Some notes on UFS Message-ID: <9506030208.AA04665@cs.weber.edu>
next in thread | raw e-mail | index | archive | help
OK, I have some notes on UFS; sorry they are not formatted as diffs,
but I'm still partially machine-less on this.
I've been looking arounf for 64 bit clean/unclean, and here's what Iive
found (so far):
ufs_bmap.c:
line 152: is_sequential(ump, ip->i_db[ ...
because the is_sequential macro forces evaluation, there should be
an explicit cast in the macro when doing address aritmatic to ensure
the sign of the values being compared (address cast), since addresses
can be compared as signed values. This is definitely a nit. The
is_sequential macro is in ufs_mount.h.
line 201: for (bn = xap->in_off + 1; ...
using bn ( a daddr_t: types.h: long) this way is potentially wrong;
I don't know if it should be passed into bmaparray as unsigned in
the first place or not (comparing against ump->um_nindir: MNINDIR,
an unsigned long value: ufs_mount.h is definitely bad).
line 201: is_sequential(ump, ((daddr_t ...
another nit.
ufs_disksubr.c:
There's some PREDISKSLOCE stuff I'll ignore.
in the routine diskerr():
the bp->b_bcount vs. lp->d_secsize is signed vs. unsigned.
This points up a potential problem in struct buf, with b_bcount
being a long (else why have d_secsize unsigned?).
ufs_lookup.c:
line 158: int vpid;
This should be: u_long vpid;
line 244: endsearch = roundup(dp->i_size, DIRBLKSIZ);
endsearch is a doff_t (inode.h: unsigned long); the conversion from
a quad (dp->i_size) should be explicit (via a macro from inode.h
where long is used instead of quad for doff_t typedef?).
line 387: dp->i_offset = roundup(dp->i_size, DIRBLKSIZ);
same conversion as @ 244.
line 665: newdir.d_namlen = cnp->cn_namelen;
cn_namelen (namei.h: long) seems to be a long for no good reason?
otherwise, this is a potential loss of precision.
line 763: if (spacefree + dsize < newentrysize)
dsize is an unsigned int, while the other two variables are int.
According to ANSI, the lvalue of the addition is unsigned, so the
compare for an unsigned less than a signed is probably buggy at
the boundry conditions.
line 825: ep->d_reclen += dp->i_reclen;
d_reclen (dirent.h: unsigned short) is potentially smaller than
i_reclen (inode.h: unsigned long), so this is a potential overflow.
line 878: for (off = 0; off < ip->i_size; ...
off is an off_t (signed quad) and i_size is an alias for i_din.di_size
which is (dinode.h: u_quad_t) an unsigned quad. This is probably
a nit.
line 941: if (target->i_number == rootino)
If this isn't going to be referenced as a manifest constant (ROOTINO)
for no good reason, at least declare it as an ino_t instead of as
an int (ino_t: types.h: unsigned long).
line 972: if (dirbuf.dotdot_ino == rootino)
same problem here.
ufs_quota.c:
line 169: ncurblocks should be declared unsigned.
line 285: ncurinodes should be declared unsigned.
ufs_vnops.c:
line 769: oldparent, newparent should be ino_t, not int.
line 1474: ...VTOI(ap->a_vp)->i_size <= uio->uio_offset;
i_size is unsigned, uio_offset is signed (off_t).
line 1494: isize = ip->i_size;
since a path can not exceed MAXPATHLEN, this should be an explicit
cast. The int = quad here is an intentional loss of precision.
Well, that's all for right now. 8-).
Terry Lambert
terry@cs.weber.edu
---
Any opinions in this posting are my own and not those of my present
or previous employers.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?9506030208.AA04665>