Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Andreas Dilger <adilger@dilger.ca>
Message-Id: <0FC238E1-59D7-448B-BD72-C2794A3BD99E@dilger.ca>
Content-Type: multipart/signed;
 boundary="Apple-Mail=_276E3F35-881D-4FCA-A735-A8DEA28D9F23";
 protocol="application/pgp-signature"; micalg=pgp-sha256
Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\))
Subject: Re: /fs/ext4/ext4.h add a comment to ext4_dir_entry_2
Date:   Mon, 4 May 2020 17:00:03 -0600
In-Reply-To: <20200504223900.GA5691@magnolia>
Cc:     Jonny Grant <jg@jguk.org>,
        Ext4 Developers List <linux-ext4@vger.kernel.org>
To:     "Darrick J. Wong" <darrick.wong@oracle.com>
References: <bf50e54f-2a0c-17a4-89c3-4afcc298daeb@jguk.org>
 <1B91A6E6-7F4A-4C58-93E7-394217C1631C@dilger.ca>
 <20200504223900.GA5691@magnolia>
Sender: linux-ext4-owner@vger.kernel.org
Precedence: bulk


--Apple-Mail=_276E3F35-881D-4FCA-A735-A8DEA28D9F23
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=us-ascii


> On May 4, 2020, at 4:39 PM, Darrick J. Wong <darrick.wong@oracle.com> =
wrote:
>=20
> On Mon, May 04, 2020 at 04:26:35PM -0600, Andreas Dilger wrote:
>>=20
>>> On May 3, 2020, at 6:52 AM, Jonny Grant <jg@jguk.org> wrote:
>>>=20
>>> Hello
>>>=20
>>> Could a comment be added to clarify 'file_type' ?
>>>=20
>>> 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 */
>>> };
>>>=20
>>>=20
>>>=20
>>> This what I am proposing to add:
>>>=20
>>>   __u8    file_type;        /* See directory file type macros below =
*/
>>=20
>> For this kind of structure field, it makes sense to reference the =
macro
>> names directly, like:
>>=20
>> 	__u8	file_type;	/* See EXT4_FT_* type macros below */
>>=20
>> since "macros below" may be ambiguous as the header changes over =
time.
>>=20
>>=20
>> Even better (IMHO) is to use a named enum for this, like:
>>=20
>>        enum ext4_file_type file_type:8; /* See EXT4_FT_ types below =
*/
>>=20
>> /*
>> * 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		=3D 0,
>> 	EXT4_FT_REG_FILE	=3D 1,
>> 	EXT4_FT_DIR		=3D 2,
>> 	EXT4_FT_CHRDEV		=3D 3,
>> 	EXT4_FT_BLKDEV		=3D 4,
>> 	EXT4_FT_FIFO		=3D 5,
>> 	EXT4_FT_SOCK		=3D 6,
>> 	EXT4_FT_SYMLINK		=3D 7,
>> 	EXT4_FT_MAX,
>> 	EXT4_FT_DIR_CSUM	=3D 0xDE
>> };
>>=20
>> 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.
>=20
> 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.

No, that is what the fixed bitfield declaration "enum ... :8" would do -
declare this enum to be an 8-bit integer.  I've verified that this works
as expected with GCC, to allow an enum with a specific size, like :8 or
:32 or :64.  Obviously, if you specify a bitfield size that doesn't =
align
with the start of the next structure field, there would be padding added
so that the next field is properly aligned, but that isn't the case =
here.

Since e2fsprogs needs to be portable to other compilers/OS, I'm not sure
if Ted would want the kernel header declaration to be different than the
e2fsprogs header.  I've grown to like using enum for these kind of =
"flags"
definitions, since they are much more concrete than a bare "int flags"
declaration, and still better than "int flags; /* see EXT4_FT_* below =
*/"
since the enum is a hard compiler linkage vs. just a comment, for the
same reasons that static inline functions are better than CPP macros.

Cheers, Andreas


--Apple-Mail=_276E3F35-881D-4FCA-A735-A8DEA28D9F23
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
	filename=signature.asc
Content-Type: application/pgp-signature;
	name=signature.asc
Content-Description: Message signed with OpenPGP

-----BEGIN PGP SIGNATURE-----
Comment: GPGTools - http://gpgtools.org

iQIzBAEBCAAdFiEEDb73u6ZejP5ZMprvcqXauRfMH+AFAl6wnnMACgkQcqXauRfM
H+CpsA//bLfIXPj0SYQT9WujSn1aXkCf5/mTbA4PMrGohtSEZ2ehxjcnA27Losip
KFSV1Fpx+C40XbvLFfPTX7o3Y5VKsYxanu1JG3B6ZA2ku6JHvqzuoePvrEP8L6HV
K9301YhfOsv5oSRkG2aHFiSC8fSo+n/F6a1Cn8FoHcF0QIKLJQK3v7PaS9UH5QBd
kr66Esc1rdFKDhe8rtVtlL+5DO7CfXeClEPGwgNmaEztmEUiTbmgQaxE1Vz2pPC5
rHX7PyvxZJJ2Qi63abamDxWoAGspcBIfcubSJFHj8t/w1aPcUixKMpHZ2pJyeV/1
mnkOXZ5Zy0l8T3axsSocANiucTjIvsCKIuv1Nt1FvIZ7nFceVoBmwwJ5kPM05gH6
ftNwYvUi00HH8lNnOB8rKfyNYIMUOjoBjkkVaKcs+cdcoBZws1VFf+M7R8xO4Uh9
tCWvAlkzwrQspWkQi3wixjjXLAhj1gxxKxFwZn6nNR4QmXg/NGiSARLyPvtvSxIg
tl5H8UyKKDWE9AYTviHqZYLYvUlLVHpTRfBkZPqcqjUBMTsp7gY7lWRW2jdlzq1+
HPA4PKcendc7y73BlKHCRayFWAN0BG3fBcQEaJGfysSu4+UCOsT6aXeBgm6zXjjp
32FEy+CLk3kfYvWCtBjg+OHA3Jy1lMQO6wxqAmOVlairSCNQl1Q=
=O1hy
-----END PGP SIGNATURE-----

--Apple-Mail=_276E3F35-881D-4FCA-A735-A8DEA28D9F23--