Ranslam, Robert E wrote:
> Changing the Subject to something more relevant
>
> Just sent the patch directly to you. Also saw Vinesh post it
>
The old code looks correct to me, the comments are confusing:
size = XFS_DIR2_SF_HDR_SIZE(i8count) + /* header */
count + /* namelen */
count * (uint)sizeof(xfs_dir2_sf_off_t) + /* offset */
namelen + /* name */
(i8count ? /* inumber */
(uint)sizeof(xfs_dir2_ino8_t) * count :
(uint)sizeof(xfs_dir2_ino4_t) * count);
i8count is set based on their being a 64 bit inode in the directory, a
short form dir will attempt to pack inode numbers into 4 bytes if they
will all fit.
So we have:
// the size of the header record
XFS_DIR2_SF_HDR_SIZE(i8count) +
// 1 byte for the namelen of each entry
count +
// one off_t thing for each entry
count * (uint)sizeof(xfs_dir2_sf_off_t) +
// space for the names
namelen +
// space for count 4 or 8 byte inode numbers
(i8count ? /* inumber */
(uint)sizeof(xfs_dir2_ino8_t) * count :
(uint)sizeof(xfs_dir2_ino4_t) * count);
So the use of count where it says namelen is correct, we have 1 byte for
each string length.
You could probably restructure this into something a little less complex:
XFS_DIR2_SF_HDR_SIZE(i8count) +
namelen +
count * (sizeof(xfs_dir2_sf_off_t) + 1 +
(i8count ? sizeof(xfs_dir2_ino8_t) : sizeof(xfs_dir2_ino4_t)))
I would actually break the conditional expression out of there and see if that
makes a difference.
inode_size = i8count ? sizeof(xfs_dir2_ino8_t) :
sizeof(xfs_dir2_ino4_t);
size = XFS_DIR2_SF_HDR_SIZE(i8count) +
count * (sizeof(xfs_dir2_sf_off_t) + 1 + inode_size);
Steve
|