On Mon, May 04, 2020 at 04:26:35PM -0600, Andreas Dilger wrote:
>
> > On May 3, 2020, at 6:52 AM, Jonny Grant <jg@xxxxxxxx> wrote:
> >
> > Hello
> >
> > Could a comment be added to clarify 'file_type' ?
> >
> > struct ext4_dir_entry_2 {
> > __le32 inode; /* Inode number */
> > __le16 rec_len; /* Directory entry length */
> > __u8 name_len; /* Name length */
> > __u8 file_type;
> > char name[EXT4_NAME_LEN]; /* File name */
> > };
> >
> >
> >
> > This what I am proposing to add:
> >
> > __u8 file_type; /* See directory file type macros below */
>
> For this kind of structure field, it makes sense to reference the macro
> names directly, like:
>
> __u8 file_type; /* See EXT4_FT_* type macros below */
>
> since "macros below" may be ambiguous as the header changes over time.
>
>
> Even better (IMHO) is to use a named enum for this, like:
>
> enum ext4_file_type file_type:8; /* See EXT4_FT_ types below */
>
> /*
> * Ext4 directory file types. Only the low 3 bits are used. The
> * other bits are reserved for now.
> */
> enum ext4_file_type {
> EXT4_FT_UNKNOWN = 0,
> EXT4_FT_REG_FILE = 1,
> EXT4_FT_DIR = 2,
> EXT4_FT_CHRDEV = 3,
> EXT4_FT_BLKDEV = 4,
> EXT4_FT_FIFO = 5,
> EXT4_FT_SOCK = 6,
> EXT4_FT_SYMLINK = 7,
> EXT4_FT_MAX,
> EXT4_FT_DIR_CSUM = 0xDE
> };
>
> so that the allowed values for this field are clear from the definition.
> However, the use of a fixed-with bitfield (enum :8) is a GCC-ism and Ted
> may be against that for portability reasons, since the kernel and
> userspace headers should be as similar as possible.
This is an on-disk structure. Do /not/ make this an enum because that
would replace a __u8 with an int, which will break directories.
--D
> Cheers, Andreas
>
>
>
>
>
