[patch #9754] find: new '-attr' predicate to test inode flags

--8323329-1189641421-1784184757=:2334372
Content-Type: TEXT/plain; CHARSET=utf-8
Content-Transfer-Encoding: QUOTED-PRINTABLE
Content-Disposition: inline

Follow-up Comment #7, patch #9754 (group findutils):=0A=0AI expect you're r=
ight. I'm no expert in find's code structure. But I'd suggest=0Athat rather=
 than HAVE_INODE_FLAGS (which presupposes an implementation), a=0Aname like=
 HAVE_ATTRIBUTES or HAVE_FLAGS might be better, because it's higher=0Alevel=
 (and then each implementation could have its own define such as=0AHAVE_LIN=
UX_FS_H or HAVE_ATTRIBUTES_LINUX). For example, Solaris's file=0Aattributes=
 are stored in the extended attributes, not the inode. But maybe the=0Aname=
 is fine.=0A=0AHowever: The attr_to_iflag (and attr_to_rate and get_iflags)=
 functions are=0Aguarded by #ifdef HAVE_INODE_FLAGS. I don't think that's e=
nough. Another=0Asystem might have HAVE_INODE_FLAGS, but a different implem=
entation, and that=0Afunction refers to symbols like FS_APPEND_FL without b=
eing guarded by #ifdef=0AHAVE_LINUX_FS_H. Either the whole function should =
be guarded by=0AHAVE_LINUX_FS_H, and each implementation has its own entire=
 attr_to_iflag=0Afunction, or the function is only guarded by HAVE_INODE_FL=
AGS, and inside it,=0Aeach system's implementation is guarded by its separa=
te define (like=0AHAVE_LINUX_FS_H). Maybe that can wait until the second im=
plementation is=0Aadded, but setting the scene now wouldn't be a bad idea. =
Maybe the find=0Adevelopers would have a preference.=0A=0ABy the way, in 20=
19 you indicated that linux/fs.h applied to multiple=0Afilesystem types but=
 ext2-style attributes were specific to the ext2 family of=0Afilesystems. T=
hat doesn't seem correct. The seemingly ext2-specific libe2p=0Ainterface (f=
getflags) from e2fsprogs also applies to multiple filesystem=0Atypes. I had=
n't tested it on xfs but it works at least on ext4 and btrfs. I've=0Ajust t=
ested it on xfs, and it works there too. I don't know which is the=0Abetter=
 interface, but berny did mention some flags were missing from the=0Apatch.=
 They are available via the fgetflags() interface. So maybe it would be=0Ab=
etter to switch to using fgetflags()? [No, see below]=0A=0AWarning: If you =
did change to use fgetflags() note that there's currently a=0Abug when it r=
eturns -1. If it did so because ioctl() failed, then it sets the=0Aflags to=
 an uninitialised value, so the flags need to be set to zero when=0Afgetfla=
gs fails. I've submitted a patch so maybe it'll get fixed.=0A=0AActually, I=
 just checked linux/fs.h and it does have CaseFold and Verity and=0Aothers =
that aren't in the patch. Presumably they have been added since 2019.=0ACom=
paring ext2fs/ext2_fs.h and libux/fs.h, there are very few differences.=0AO=
nly linux/fs.h has EOFBLOCKS (ext2fs/ext2_fs.h had it but removed it). Only=
=0Aext2fs/ext2_fs.h has SNAPFILE, SNAPFILE_DELETED, SNAPFILE_SHRUNK. This i=
s=0Awhat's on a debian12 system so it's old. There may be new attributes by=
 now=0A(No it's the same on github). So it sounds like the only advantage t=
o using=0Afgetflags instead would be to search for snapshot inodes. [No, se=
e the end]=0A=0AAnyway, the patch is missing FS_DIRTY_FL, FS_COMPRBLK_FL, F=
S_NOCOMP_FL,=0AFS_BTREE_FL, FS_IMAGIC_FL, FS_HUGE_FILE_FL, FS_VERITY_FL, FS=
_EA_INODE_FL,=0AFS_EOFBLOCKS_FL (maybe), FS_DAX_FL, and FS_CASEFOLD_FL. It =
might as well=0Ainclude them.=0A=0AAlso, I think it's a good idea at least =
add chattr(1) to the SEE ALSO section=0Ain find.1 (and the info equivalent)=
. Although I think it would be better to=0Alist the attribute letters expli=
citly in find's documentation, the way it=0Aincludes some strftime format c=
onversions, rather than only directing the=0Areader to look at strftime's o=
wn manpage. It makes it easier for the reader=0Abecause they don't have to =
look up another manpage. But find's developers=0Amight disagree. Adding a l=
ist of letters and explanations for each=0Aimplementation might become unwi=
eldy with more implementations (but I'd still=0Arecommend it). Although, si=
nce the chattr manpage doesn't mention all of the=0Aattributes, documenting=
 them in find's manpage is necessary. chattr(1) (on=0Adebian12) only mentio=
ns 22 attributes, but linux/fs.h has 29 and=0Aext2fs/ext2_fs.h has 31. So t=
here are extra ones that need to be documented=0A(if find should support al=
l of them even though chattr doesn't).=0A=0AIt looks like fgetflags and thi=
s patch both use ioctl but with a different=0Acommand: EXT2_IOC_GETFLAGS or=
 FS_IOC_GETFLAGS. But looking deeper, both are=0Adefined to the exact same =
thing. So there is no difference, just what gets=0Alisted in headers. It's =
the same ioctl. So snapshots would work via=0AFS_IOC_GETFLAGS, it just isn'=
t mentioned in linux/fs.h. So the only advantage=0Ato fgetflags() is that i=
t's ported to other platforms (but it might not get=0Ainstalled often on ot=
her platforms).=0A=0AIf all of the attributes are to be supported, letters =
would need to be chosen=0Afor the 9 that are not mentioned in chattr(1). He=
re they are along with my=0Asuggestions for letters to go with them:=0A=0A =
 y=3Ddirty (0x00000100)=0A  b=3Dcomprblk (0x00000200)=0A  B=3Dbtree (0x0000=
1000)=0A  M=3Dimagic (0x00002000)=0A  h=3Dhugefile (0x00040000)=0A  n=3Dea_=
inode (0x00200000)=0A  z=3Dsnapfile (0x01000000)=0A  Z=3Dsnapfile_deleted (=
0x04000000)=0A  U=3Dsnapfile_shrunk (0x08000000)=0A=0AOf course, it begs th=
e question: Why doesn't chattr mention these? Maybe that=0Adoesn't matter. =
Presumably they are usable via fsetflags() or=0Aioctl(EXT2_IOC_SETFLAGS) or=
 ioctl(FS_IOC_SETFLAGS).=0A=0A=0A    ______________________________________=
_________________=0A=0AReply to this item at:=0A=0A  <https://savannah.gnu.=
org/patch/?9754>=0A=0A_______________________________________________=0AMes=
sage sent via Savannah=0Ahttps://savannah.gnu.org/=0A
--8323329-1189641421-1784184757=:2334372
Content-Type: APPLICATION/pgp-signature; name=signature.asc

-----BEGIN PGP SIGNATURE-----

iHUEABYIAB0WIQQk97aszIMMAvLLwm6qLAuaBUf3TgUCalh/tQAKCRCqLAuaBUf3
TrlpAP9s2OBBLUfRqfbYOjvMOSI8EcPvdWYjKBUYXwEgSi736gEAtYukfCtFBzs2
dnv4AVRwjM3rz2XjBogQCfQG22/2Fwc=
=5Ald
-----END PGP SIGNATURE-----

--8323329-1189641421-1784184757=:2334372--