Page MenuHomeFreeBSD

find: add -xattr and -xttrname
AcceptedPublic

Authored by kevans on Sat, Feb 14, 7:57 PM.
Tags
None
Referenced Files
F145430028: D55286.diff
Thu, Feb 19, 6:34 PM
Unknown Object (File)
Tue, Feb 17, 6:58 PM
Unknown Object (File)
Tue, Feb 17, 12:07 PM
Unknown Object (File)
Tue, Feb 17, 6:42 AM
Unknown Object (File)
Tue, Feb 17, 3:22 AM
Unknown Object (File)
Mon, Feb 16, 10:33 PM
Subscribers

Details

Reviewers
kib
rmacklem
Group Reviewers
Klara
Summary

We use -xattr in our openrsync tests for convenience, and it seems like
a good addition to FreeBSD. -xattr and -xattrname will both consult all
available namespaces by default, but -xattrname allows filtering by
namespace using a "user:" or "system:" prefix.

Inspired by: https://github.com/apple-oss-distributions/shell_cmds
Sponsored by: Klara, Inc.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 70722
Build 67605: arc lint + arc unit

Event Timeline

It looks ok to me, although I do not know
anything about the atf testing part.

The limitation of it is that it does not
handle named attributes (which some,
like Solaris, call extended attributes).
I've suggested something be added to
the man page patch.

Alternately, you could also support named
attributes for these options, but that's a little
more work.
--> For ZFS, both can work, so this patch works

for ZFS. (For ZFS, both syscall interfaces finds
the attributes.)
For NFSv4, some servers support Linux extended
attributes and others support Solaris style, which
Solaris call extended attributes, but NFSv4 calls
named attributes.

I don't know how to separate these clearly, but at
least using extended attributes vs named attributes is a start.

Just to make it more confusing, Windows calls named attributes
alternate data streams, while MacOS calls them
fork files (and might also have Linux style extended
attributes, although their docs are confusing).

usr.bin/find/find.1
991

Maybe you could add a sentence noting that
this does not handle named attributes, but only
extended attributes. (I know, you say extended
attributes, but Solaris calls named attributes extended
attributes and people get confused.)

1408

You should probably add extattr(2) to the
See Also..

usr.bin/find/function.c
1775

This works but is a little obscure.
It assumes that the "empty" string is the
first element of EXTATTR_NAMESPACE_NAMES
and that EXTATTR_NAMESPACE_EMPTY is defined
as 0 (which it is).

You might consider changing this to a
strcmp(xattr_ns[ns], EXTATTR_NAMESPACE_EMPTY_STRING)
but I'll leave it up to you.

This revision is now accepted and ready to land.Sat, Feb 14, 10:50 PM

Do we want to accept e.g. 'random_garbage:aaa' as the xattr name? I.e., if user put non-existing namespace before ':' should we silently do not find anything?

usr.bin/find/function.c
62

Would it compile?

1834

IMO it would be clearer if you put the return find_has_xattr() statement there instead of break.

In D55286#1264421, @kib wrote:

Do we want to accept e.g. 'random_garbage:aaa' as the xattr name? I.e., if user put non-existing namespace before ':' should we silently do not find anything?

That was my intention, yeah- I don't think we impose restrictions on the name anywhere else, so it seemed reasonable to leave it alone besides the special filtering I wanted to do.

usr.bin/find/function.c
1775

This was actually trying to avoid making exactly that assumption- I started iteration at 0 in case EMPTY isn't 0 for whatever reason, and chose to skip it here. I think the indices have to match the namespace definitions in order for EXTATTR_NAMESPACE_NAMES to be useful at all- the only other use uses it to map arbitrary namespace id to a string.

kevans marked 4 inline comments as done.

Address review feedback

This revision now requires review to proceed.Tue, Feb 17, 5:00 AM
This revision is now accepted and ready to land.Tue, Feb 17, 5:09 AM