Page MenuHomeFreeBSD

Fix inode extra time fields overflow.
ClosedPublic

Authored by fsu on Apr 22 2021, 5:25 PM.

Details

Summary

Make inode extra time fields updating logic more closer to linux.

Test Plan

Found using pjdfstest:
pjdfstest/tests/utimensat/09.t

Testcase passed successfully.

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

fsu requested review of this revision.Apr 22 2021, 5:25 PM
fsu created this revision.
pfg requested changes to this revision.Apr 22 2021, 8:49 PM

Do not touch inode.h.

You can use inline functions to replace the macros and perhaps with some casting you can get over the issues.

sys/fs/ext2fs/inode.h
103–106

These changes are wrong. Long discussion with bde@ showed we should not diverge from UFS here and the kernel won't handle anything different different than int32_t.

This revision now requires changes to proceed.Apr 22 2021, 8:49 PM
sys/fs/ext2fs/inode.h
103–106

I think we not diverge from ufs so mach, you can check struct ufs2_dinode and ufs_time_t typedef, where ufs_time_t is int64_t.

While the ext_time_t approach is valid and it also consistent with UFS2, it is usually faster to use 32bit values so you *could* use casts. Either way is valid so I won't object.

sys/fs/ext2fs/ext2_inode_cnv.c
96–97

You are replacing a macro so use inline here

104

use inline here, and move it nearer to the place where it will be used.: basically where the macro was.

sys/fs/ext2fs/inode.h
103–106

I see: I was looking at ufs1_dinode while you are using ufs2_dinode.
last time I looked at linux these were uint32_t so casting should be enough.

This revision is now accepted and ready to land.Apr 23 2021, 4:23 PM