patchfindutils - Patches: patch #9754, find: new '-attr' predicate to...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

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

Submitter:  Matt Whitlock <mwhitlock>
Submitted:  Thu 31 Jan 2019 02:03:43 AM UTC
   
 
Category:  find enhancement Priority:  5 - Normal
Status:  None Privacy:  Public
Assigned to:  None Originator Name: 
Open/Closed:  Open Release:  None
Fixed Release:  None

Tue 23 Apr 2019 04:01:50 AM UTC, comment #2: 

Inode flags are not specific to Linux's Extended File System, but they are specific to Linux. They were lifted from Ext to Linux's (filesystem-agnostic) VFS layer because they're used by multiple file system implementations. The following note from <linux/fs.h> may be instructive:


 * Note: for historical reasons, these flags were originally used and
 * defined for use by ext2/ext3, and then other file systems started
 * using these flags so they wouldn't need to write their own version
 * of chattr/lsattr (which was shipped as part of e2fsprogs).


As it happens, I mostly use these flags on XFS volumes.

Regarding the "Casefold" and "Verity" flags you mentioned, I do not see these in the <linux/fs.h> header. I would guess that they are specific to Ext and are accessed by filesystem-specific APIs. If they are ever promoted to the VFS layer, then it might make sense to add them to Find.

I have added an Autoconf check and preprocessor guard directives so that -attr is implemented only on systems with a working <linux/fs.h> header.

I do not see the code style warnings that you quoted, but I have incorporated fixes that should eliminate them.

I am indeed willing to assign copyrights for my contribution to the Free Software Foundation.

(file #46811)

Matt Whitlock <mwhitlock>
Sat 09 Feb 2019 04:43:53 PM UTC, comment #1: 

Thanks for the patch.

For this size of contributions, we need to have the GNU copyright
assignment in place.  Are you willing to do that?

Re. attr in general: this is quite EXT2/3/4 && GNU Linux specific, no?
So such an extension would need to be 'configure'-able on platforms
where it is not available.

The #ifdef's in [1] are already quite complex, and doesn't seem to cover
further platforms the GNU findutils have to support.

[1] https://git.kernel.org/pub/scm/fs/ext2/e2fsprogs.git/tree/lib/e2p/fgetflags.c

E.g. on FreeBSD, I'm getting:


parser.c:27:10: fatal error: 'linux/fs.h' file not found
#include <linux/fs.h>
         ^~~~~~~~~~~~
1 error generated.


BTW: on GCC-8.2.1 on GNU/Linux (openSUSE:Tumbleweed), I'm getting
the following warnings:


make[3]: Entering directory '/media/big_data/berny/git/findutils/find'
  CC       parser.o
parser.c: In function 'parse_attr':
parser.c:923:7: error: suggest explicit braces to avoid ambiguous 'else' [-Werror=dangling-else]
    if (!(iflag = attr_to_iflag (*p)))
       ^
parser.c:944:3: error: this 'else' clause does not guard... [-Werror=misleading-indentation]
   else
   ^~~~
parser.c:947:5: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'else'
     our_pred = insert_primary (entry, expr);
     ^~~~~~~~
cc1: all warnings being treated as errors
make[3]: *** [Makefile:1936: parser.o] Error 1
  CC       pred.o
pred.c: In function 'pred_attr':
pred.c:254:23: error: suggest parentheses around arithmetic in operand of '^' [-Werror=parentheses]
   ret = (state.iflags & pred_ptr->args.intmask.and_mask ^ pred_ptr->args.intmask.xor_mask) != 0;
          ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[3]: *** [Makefile:1936: pred.o] Error 1
  CC       util.o
util.c:281:1: error: function declaration isn't a prototype [-Werror=strict-prototypes]
 get_iflags ()
 ^~~~~~~~~~
util.c: In function 'get_iflags':
util.c:281:1: error: old-style function definition [-Werror=old-style-definition]
util.c:287:20: error: declaration of 'ret' shadows a previous local [-Werror=shadow]
       int fd = -1, ret;
                    ^~~
util.c:283:7: note: shadowed declaration is here
   int ret = 0;
       ^~~
util.c:296:7: error: this statement may fall through [-Werror=implicit-fallthrough=]
    if (state.curdepth == 0)
       ^
util.c:302:2: note: here
  case SYMLINK_NEVER_DEREF:
  ^~~~
cc1: all warnings being treated as errors
make[3]: *** [Makefile:1936: util.o] Error 1


I didn't check the code exactly yet (especially the parsing), mainly to avoid
tainting myself in order to have a chance to re-implement -attr in the case
we want to add it and you are not willing to undergo the above mentioned
GNU copyright assignment.

A quick look unveils that your code misses the 'F' (Casefold) and the 'V' (Verity)
flag, and I'm unsure how we could keep this in sync with e2fsprogs [2] in the long run.

[2] https://git.kernel.org/pub/scm/fs/ext2/e2fsprogs.git/tree/lib/e2p/pf.c

I don't know if this has been discussed before - maybe the complexity
was the reason not to add -attr to find.

Bernhard Voelker <berny>
Group administrator
Thu 31 Jan 2019 02:03:43 AM UTC, original submission:  

On my system, I frequently need to find files having (or lacking) a particular file attribute. To date, I have implemented this with relatively gross pipelines like the following:


$ find -type f -exec lsattr -- {} + |
  grep -a '^......d' |
  cut -d' ' -f2- |
  xargs -d'\n' -r -- <somecmd>


I have long wanted 'find' to have native support for testing file attributes, a.k.a. inode flags, as the above pipeline could be simplified to:


$ find -type f -attr +d -exec <somecmd> {} +


To that end, I have implemented support in 'find' for exactly this. Please consider merging the attached patch. It includes additions to the documentation (man and texinfo) describing the new predicate.

I do not know how to update the translations files for the localizable strings I altered, so I would appreciate some guidance in that area.

Matt Whitlock <mwhitlock>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by berny (Posted a comment)
  • -email is unavailable- added by mwhitlock (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 2 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-04-23 mwhitlock Attached File- Added 0002-find-support-attr-only-if-OS-supports-inode-flags.patch, #46811
    2019-01-31 mwhitlock Attached File- Added 0001-find-introduce-attr-predicate-to-test-inode-flags.patch, #46138

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code