Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement binding for extended file attributes handling on Linux. (master) #1021

Closed
wants to merge 3 commits into from

Conversation

wilx
Copy link
Contributor

@wilx wilx commented Oct 6, 2018

This is like #1018 but for master branch and with most of review comments addressed.

@wilx wilx changed the title Implement binding for extended file attributes handling on Linux. Implement binding for extended file attributes handling on Linux. (master) Oct 6, 2018
@wilx
Copy link
Contributor Author

wilx commented Oct 7, 2018

While I was adding the JavaDocs I have realized that the attribute name string is of unspecified encoding, which, I guess, means some native encoding will be used. But the attribute name can be non-ASCII as well which requires known encoding to be used. I can use attribute name with Japanese:

~/jna/jna> setfattr -n 'user.日本語' -v test TODO 
~/jna/jna> getfattr -d TODO                      
# file: TODO
user.日本語="test"

So it seems to me the utility functions should take name encoding parameter as well and all other variants should forward to it. What do you think?

@matthiasblaesing
Copy link
Member

According to the manpage the signature for setxattr is:

int setxattr (const char *path, const char *name, const void *value, size_t size, int flags);

For the datatypes it is said:

The VFS imposes limitations that an attribute names is limited to 255 bytes and an attribute value is limited to 64 kB. The list of attribute names that can be returned is also limited to 64 kB (see BUGS in listxattr(2)).

There are multiple mappings possible. The existing mappings map C char[] to java String. This can result in unaccessible files (where the encoded filename can't be represented in a String)/attributes, but I think for normal bindings this is a valid tradeoff. If you need to be able to access all files and attributes, a C char[] should be mapped as java byte[].

So I would bind the above as:

int setxattr (String path, String name, byte[] value, size_t size, int flags);

@matthiasblaesing
Copy link
Member

@wilx could you please have a look at this branch:
https://github.com/matthiasblaesing/jna/commits/upstream-pr-1021

I squashed your commits and added two new ones:

matthiasblaesing@62cd54e - adds a CHANGES.md entry for the new function
matthiasblaesing@bd184d5 - adds bindings based on byte[]

The latter commit is intended to circumvent Memory usage. Memory uses finalize to clean up native resources and a ByteBuffer created from a Memory object retains another reference to the Memory object, so adds another step in GC. Using a byte[] reduces this to two memcpy (to and from native).

@wilx
Copy link
Contributor Author

wilx commented Nov 10, 2018

@wilx I am OK with that.

@matthiasblaesing
Copy link
Member

Merged via 4e88709

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants