ext2_dir_entry To ext2_dir_entry_2 Casting

Hello Theodore Ts'o,

Is there a function similar to ext2fs_dir_iterate2() that will call a hook function on an ext2_dir_entry_2 structure and not an ext2_dir_entry structure?

The reason I ask is because btrfs-convert currently tries to do a cast between the two structures as such:

	static int dir_iterate_proc(..., struct ext2_dir_entry *old, ...)
	{
	...
		struct ext2_dir_entry_2 *dirent = (struct ext2_dir_entry_2 *)old;

which works fine on little-endian machines, but breaks on big-endian machines.

If there isn't, would you be interested in a patch that adds a function, say, ext2_dir_entry_upgrade(struct ext2_dir_entry *old, struct ext2_dir_entry_2 *new) that will convert one structure to the other and take into account the endianness of the machine? This would be better than just ad-hoc fixing the code in btrfs.

Quoted 14 lines · Wade Cline

Hello Theodore Ts'o,

Is there a function similar to ext2fs_dir_iterate2() that will call a hook function on an ext2_dir_entry_2 structure and not an ext2_dir_entry structure?

The reason I ask is because btrfs-convert currently tries to do a cast between the two structures as such:

	static int dir_iterate_proc(..., struct ext2_dir_entry *old, ...)
	{
	...
		struct ext2_dir_entry_2 *dirent = (struct ext2_dir_entry_2 *)old;

Quoted 1 line

which works fine on little-endian machines, but breaks on big-endian machines.

The reason why we don't have that function today is because what most programs (especially inside e2fsprogs) have done is to use the ext2_dir_entry structure and just simply used (name_len& 0xff) to get the actual name length, and in the case of e2fsck (which is the only program which in practice has cared about the file type) to get the file type by doing (name_len>> 8).

Quoted 5 lines

If there isn't, would you be interested in a patch that adds a function, say, ext2_dir_entry_upgrade(struct ext2_dir_entry *old, struct ext2_dir_entry_2 *new) that will convert one structure to the other and take into account the endianness of the machine? This would be better than just ad-hoc fixing the code in btrfs.

We could do that, but that would mean needing to copy the data structure somewhere else, which means you would have to allocate memory for the new data structure, and then you'd have to release that memory later, etc.

I would think that using (name_len& 0xFF) is a much simpler solution, and my suggestion is to not depend on the file type in the directory entry (since there might be some very old ext2 file systems that don't set the file type), and to use the inode's mode bits as authoratative for the file type of the inode.

Regards,

- Ted

Quoted 6 lines

I would think that using (name_len& 0xFF) is a much simpler solution, and my suggestion is to not depend on the file type in the directory entry (since there might be some very old ext2 file systems that don't set the file type), and to use the inode's mode bits as authoratative for the file type of the inode.

Interesting compatibility issue. Will keep it in mind.

To clarify, the EXT2_FEATURE_INCOMPAT_FILETYPE flag indicates that there _may_ be file type information in the directory entry (and so only the low 8 bits of name_len should be considered part of the name length), but it does not guarantee that it will be present in the high 8 bits of name_len.

If it is not there, then readdir will simply return DT_UNKNOWN in the d_type field of the directory entry returned by readdir(2). This is something all application programs have to be prepared to deal with --- if they need the file type information, and they get DT_UNKNOWN, then they will need to stat the file to get the information.

- Ted

In this case, dir_iterate_proc() is being passed into ext2_dir_iterate2() and is called from ext2fs_process_dir_block(), not readdir(2). It looks like the main issue would be detecting if the EXT2_FEATURE_INCOMPAT_FILETYPE flag is -unset- and manually setting the file type to EXT2_FT_UNKNOWN (there appears to be a case for handling EXT2_FT_UNKNOWN; so long as the file type is not set to an undefined value it should be okay).