Btrfs devel - Oct 2012 - [e2fsprogs] 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.

Thank you,
Wade Cline

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Tue, Oct 02, 2012 at 12:02:22PM -0700, Wade Cline
wrote:
> 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.
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).
> 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
    	
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On 10/02/2012 02:08 PM, Theodore Ts''o wrote:
> On Tue, Oct 02, 2012 at 12:02:22PM -0700, Wade Cline wrote:
>> 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.
> 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).
>
I see. That should work fine, too.
>> 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
Interesting compatibility issue. Will keep it in mind.

Thank you for the help.

Sincerely,
Wade

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Wed, Oct 03, 2012 at 10:39:55AM -0700, Wade Cline
wrote:
> >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
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On 10/03/2012 10:58 AM, Theodore Ts''o wrote:
> 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).

Thank you for the clarification.

Regards,
Wade

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Wed, Oct 03, 2012 at 11:29:58AM -0700, Wade Cline
wrote:
> 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).
EXT2_FT_UNKNOWN == DT_UNKNOWN == 0

Since we''ve always enforced that name_len < 256, the high 8 bits
of name_len will always be zero, even if !EXT2_FEATURE_INCOMPAT_FILETYPE.

							- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html