Differential D29933

Fix inode extra time fields overflow.
ClosedPublic
Actions

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

Details

Reviewers

pfg

Commits

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

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

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

fsu requested review of this revision.Apr 22 2021, 5:25 PM

fsu created this revision.

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	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

fsu added inline comments.Apr 23 2021, 2:46 PM

sys/fs/ext2fs/inode.h
103	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
97	You are replacing a macro so use inline here
106	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	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.