Hello,
Thanks for working on this!
Milos Nikic, le ven. 23 janv. 2026 22:55:23 -0800, a ecrit:
> diff --git a/ext2fs/ext2fs.h b/ext2fs/ext2fs.h
> index 62ee9f77..eea85d1d 100644
> --- a/ext2fs/ext2fs.h
> +++ b/ext2fs/ext2fs.h
> @@ -427,9 +427,14 @@ dino_ref (ino_t inum)
> unsigned long bg_num = (inum - 1) / inodes_per_group;
> unsigned long group_inum = (inum - 1) % inodes_per_group;
> struct ext2_group_desc *bg = group_desc (bg_num);
> + uint16_t inode_size = le16toh (sblock->s_inode_size);
> + if (inode_size == 0)
> + inode_size = EXT2_GOOD_OLD_INODE_SIZE; /* Fallback for old volumes */
> + unsigned long inodes_per_blk = block_size / inode_size;
> - block_t block = le32toh (bg->bg_inode_table) + (group_inum /
> inodes_per_block);
> + block_t block = le32toh (bg->bg_inode_table) + (group_inum /
> inodes_per_blk);
Mmm, is EXT2_INODE_SIZE not already putting the right value in
inodes_per_block?
> - struct ext2_inode *inode = disk_cache_block_ref (block);
> + void *block_ptr = disk_cache_block_ref (block);
> - inode += group_inum % inodes_per_block;
> + size_t offset = (group_inum % inodes_per_blk) * inode_size;
We'd probably want to make inode_size a global variable so we don't have
to recompute it all the time while it is constant for the opened FS.
> + struct ext2_inode *inode = (struct ext2_inode *)((char *)block_ptr +
> offset);
> ext2_debug ("(%llu) = %p", inum, inode);
> return inode;
> }
> diff --git a/ext2fs/inode.c b/ext2fs/inode.c
> index dc309ac8..0fe07748 100644
> --- a/ext2fs/inode.c
> +++ b/ext2fs/inode.c
> @@ -106,6 +106,36 @@ diskfs_new_hardrefs (struct node *np)
> {
> allow_pager_softrefs (np);
> }
> +
> +static inline void
> +ext2_decode_extra_time (uint32_t legacy_sec, uint32_t extra,
> + time_t *sec, long *nsec)
> +{
> + /* Epoch extension (bits 32 and 33) */
> + *sec = (time_t)legacy_sec + (((time_t)extra & 0x3) << 32);
Urgl, so they just multiplied by 4 the lifetime of the ext2 format... :/
> + /* Nanoseconds (bits 2 through 31) */
> + *nsec = (long)(extra >> 2);
> +}
> +
> +static inline uint32_t
> +ext2_encode_extra_time (time_t sec, long nsec)
> +{
> + uint32_t extra;
> + /* Pack nanoseconds into the upper 30 bits */
> + extra = (uint32_t)(nsec << 2);
> + /* Pack bits 32 and 33 of seconds into the lower 2 bits */
> + extra |= (uint32_t)((sec >> 32) & 0x3);
> + return extra;
> +}
> +
> +/* Helper to check if the current filesystem supports extended inodes */
> +static inline int
> +ext2_has_extra_inodes (struct ext2_super_block *sb)
> +{
> + return (le32toh (sb->s_rev_level) > EXT2_GOOD_OLD_REV
> + && le16toh (sb->s_inode_size) > EXT2_GOOD_OLD_INODE_SIZE);
> +}
> +
>
> /* The user must define this function if she wants to use the node
> cache. Read stat information out of the on-disk node. */
> @@ -136,23 +166,31 @@ diskfs_user_read_node (struct node *np, struct
> lookup_context *ctx)
> st->st_gen = le32toh (di->i_generation);
>
> st->st_atim.tv_sec = le32toh (di->i_atime);
> -#ifdef not_yet
> - /* ``struct ext2_inode'' doesn't do better than sec. precision yet. */
> -#else
> - st->st_atim.tv_nsec = 0;
> -#endif
> st->st_mtim.tv_sec = le32toh (di->i_mtime);
> -#ifdef not_yet
> - /* ``struct ext2_inode'' doesn't do better than sec. precision yet. */
> -#else
> - st->st_mtim.tv_nsec = 0;
> -#endif
> st->st_ctim.tv_sec = le32toh (di->i_ctime);
> -#ifdef not_yet
> - /* ``struct ext2_inode'' doesn't do better than sec. precision yet. */
> -#else
> - st->st_ctim.tv_nsec = 0;
> -#endif
> + st->st_atim.tv_nsec = st->st_mtim.tv_nsec = st->st_ctim.tv_nsec = 0;
> + if (ext2_has_extra_inodes (sblock))
> + {
> + struct ext2_inode_extra *di_extra =
> + (struct ext2_inode_extra *) ((char *) di + EXT2_GOOD_OLD_INODE_SIZE);
> +
> + /* Only decode if the inode actually uses the extra space
> (i_extra_isize)
> + The i_extra_isize tells us how many extra bytes are used in THIS
> inode. */
> + if (le16toh (di_extra->i_extra_isize) >= 32) /* Enough room for all 3
> extra timestamps */
Better use offsetof() + sizeof() to make it really obvious that we are
checking for the last required field, rather than a magic number 32.
> + {
> + ext2_decode_extra_time (le32toh (di->i_atime), le32toh
> (di_extra->i_atime_extra),
> + &st->st_atim.tv_sec, &st->st_atim.tv_nsec);
> + ext2_decode_extra_time (le32toh (di->i_ctime), le32toh
> (di_extra->i_ctime_extra),
> + &st->st_ctim.tv_sec, &st->st_ctim.tv_nsec);
> + ext2_decode_extra_time (le32toh (di->i_mtime), le32toh
> (di_extra->i_mtime_extra),
> + &st->st_mtim.tv_sec, &st->st_mtim.tv_nsec);
> + info->i_atime_extra = le32toh (di_extra->i_atime_extra);
> + info->i_ctime_extra = le32toh (di_extra->i_ctime_extra);
> + info->i_mtime_extra = le32toh (di_extra->i_mtime_extra);
Do we really need to remember these three?
> + }
> + else
> + info->i_atime_extra = info->i_ctime_extra = info->i_mtime_extra = 0;
> + }
>
> st->st_blocks = le32toh (di->i_blocks);
>
> @@ -416,19 +454,25 @@ write_node (struct node *np)
> di->i_links_count = htole16 (st->st_nlink);
>
> di->i_atime = htole32(st->st_atim.tv_sec);
> -#ifdef not_yet
> - /* ``struct ext2_inode'' doesn't do better than sec. precision yet. */
> - di->i_atime.tv_nsec = htole32 (st->st_atim.tv_nsec);
> -#endif
> di->i_mtime = htole32 (st->st_mtim.tv_sec);
> -#ifdef not_yet
> - di->i_mtime.tv_nsec = htole32 (st->st_mtim.tv_nsec);
> -#endif
> di->i_ctime = htole32 (st->st_ctim.tv_sec);
> -#ifdef not_yet
> - di->i_ctime.tv_nsec = htole32 (st->st_ctim.tv_nsec);
> -#endif
> -
> + if (ext2_has_extra_inodes (sblock))
You also need to check that the superblock-recorded inode size is large
enough for the required time fields.
> + {
> + struct ext2_inode_extra *di_extra =
> + (struct ext2_inode_extra *) ((char *) di +
> EXT2_GOOD_OLD_INODE_SIZE);
> + if (le16toh (di_extra->i_extra_isize) < 32)
> + di_extra->i_extra_isize = htole16 (32);
Again, use offsetof+sizeof. You can define a macro to use it in the
various places.
> +
> + di_extra->i_atime_extra = htole32 (ext2_encode_extra_time
> (st->st_atim.tv_sec,
> +
> st->st_atim.tv_nsec));
> + di_extra->i_mtime_extra = htole32 (ext2_encode_extra_time
> (st->st_mtim.tv_sec,
> +
> st->st_mtim.tv_nsec));
> + di_extra->i_ctime_extra = htole32 (ext2_encode_extra_time
> (st->st_ctim.tv_sec,
> +
> st->st_ctim.tv_nsec));
> + info->i_atime_extra = le32toh (di_extra->i_atime_extra);
> + info->i_mtime_extra = le32toh (di_extra->i_mtime_extra);
> + info->i_ctime_extra = le32toh (di_extra->i_ctime_extra);
Again, these three don't seem to be used?
And i_extra_isize in info is unused too.
Samuel
