You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Michael Clark <mi...@metaparadigm.com> on 2007/12/20 06:02:43 UTC
Extended Attributes Support
Hi Folks,
I have been doing some work on implementing extended attributes
support for apr.
I am doing this as I would like to add a property provider to
mod_dav_fs that uses extended attributes instead of a db per file
(TwistedDAV on Mac OS X also uses extended attributes for this BTW)
and I want to do this portably so it could potentially be included.
Extended attributes can also be used for setting mime-types on files
(mod_mime_xattr uses the native Linux API so this could be ported
to use portable apr xattrs and work on more platforms). Another nice
idea is setting file listing 'description' attributes and getting
mod_autoindex to look at them.
I have looked at a few of the OS extended attributes interface and
have come up with an API proposal (at the end of this mail).
It addresses:
* setting, getting, listing and removing of 'user' extended attributes
on a file specified by its filepath.
It does not address:
* system namespaces on platforms with more than one attribute namespace
(only the user namespace is accessible on platforms with multiple
attr namespaces, new flags could potentially be added to access
platform specific system namespaces)
* setting, getting, listing and removing attributes on a file
specified by a file descriptor (apr_os_file_t)
I have working sample implementations for Linux, Mac OS X, FreeBSD 6
and Solaris 10 (which each have different APIs BTW):
* Linux impl use l?(get|set|list|remove)xattr
* Mac OS X impl use (get|set|list|remove)xattr (different args to linux)
* FreeBSD impl uses extattr_(get|set|list|delete)_(file|link)
* Solaris impl uses subfiles (attropen,unlinkat and friends)
Not implemented:
* Windows - should be able to use named :streams (similar to Solaris)
* OS/2 - it has extended attributes but that's all I know.
* Irix/HPUX/AIX/OS390/netware/... - unknown? do they have them?
Any ideas on feasibility on these and other apr supported platforms?
I'll send some initial patches soon. They still need a little work
and some more test cases.
I have some questions on build structure...
Should I put the declarations in apr_file_info.h or would it be
better to add a apr_file_xattr.h header?
I have all the unix implementations rolled into one file
(file_io/unix/xattr.c) with many #if's. Should I perhaps have
a linux.c, darwin.c, freebsd.c, solaris.c in a subdirectory?
Not sure how I would integrate this into the build (perhaps
having a single #if that create empty compilation units for
the inactive platforms?).
Test cases should go into a new unit - not linked into testall?
Also what is the procedure for proposing and adding a new API like
this. I guess my patch should have it disabled by default and enabled
with a configure flag - say --enable-xattrs (or autodetected and enabled
on supported platforms)?
Proposed API:
/**
* @defgroup apr_file_xattr File Extended Attribute Functions
* @{
*/
/** Don't follow symbolic links flag for apr_file_xattr functions */
#define APR_XATTR_NOFOLLOW 1
/** When setting values, fail if the attribute already exists */
#define APR_XATTR_CREATE 2
/** When setting values, fail if the attribute does not already exist */
#define APR_XATTR_REPLACE 4
/**
* Set an extended attribute on the file specificed by filepath
* @param filepath the full path to the file
* @param name the attribute name to set
* @param value the attribute value
* @param size the size in bytes of the attribute value
* @param flags to control how the attribute is set
* <PRE>
* APR_XATTR_NOFOLLOW if the file is a symbolic link then the
* attribute will be set on the link.
* APR_XATTR_CREATE return an error if the attribute name
* already exists.
* APR_XATTR_REPLACE return an error if the attribute name
* does not already exist.
* </PRE>
* @param p the pool to allocate any memory from if required
* @remark if neither flag APR_XATTR_CREATE or APR_XATTR_REPLACE are
* given then the attribute will either be created if it does not
* already exist or replaced if it does exist.
*/
APR_DECLARE(apr_status_t) apr_file_xattr_set(const char *filepath,
const char *name,
const void *value,
apr_size_t size,
apr_int32_t flags,
apr_pool_t *p);
/**
* Get an extended attribute from the file specificed by filepath
* @param filepath the full path to the file
* @param name the attribute name to get
* @param value the returned attribute value, if value is NULL then only
* the size of the attribute value is returned
* @param size the returned size of the attribute value
* @param flags to control how the attribute is got
* <PRE>
* APR_XATTR_NOFOLLOW if the file is a symbolic link then the
* attribute will be gotten from the link.
* </PRE>
* @param p the pool to allocate any memory from if required
*/
APR_DECLARE(apr_status_t) apr_file_xattr_get(const char *filepath,
const char *name,
void **value,
apr_size_t *size,
apr_int32_t flags,
apr_pool_t *p);
/**
* List the extended attributes for the file specificed by filepath
* @param filepath the full path to the file
* @param list the returned array of attributes names
* @param flags to control how the file is listed
* <PRE>
* APR_XATTR_NOFOLLOW if the file is a symbolic link then the
* attributes of the link will be listed.
* </PRE>
* @param p the pool to allocate any memory from if required
* @remark list is an array containing simple null terminated strings.
*/
APR_DECLARE(apr_status_t) apr_file_xattr_list(const char *filepath,
apr_array_header_t **list,
apr_int32_t flags,
apr_pool_t *p);
/**
* Remove an extended attribute from the file specificed by filepath
* @param filepath the full path to the file
* @param name the attribute name to remove
* @param flags to control how the attribute is removed
* <PRE>
* APR_XATTR_NOFOLLOW if the file is a symbolic link then the
* attribute will be removed from the link.
* </PRE>
* @param p the pool to allocate any memory from if required
*/
APR_DECLARE(apr_status_t) apr_file_xattr_remove(const char *filepath,
const char *name,
apr_int32_t flags,
apr_pool_t *p);
/** @} */
Re: Extended Attributes Support
Posted by Michael Clark <mi...@metaparadigm.com>.
William A. Rowe, Jr. wrote:
> Michael Clark wrote:
>> William A. Rowe, Jr. wrote:
>>> I'd go for a subdir option (xattrs/solaris.c, xattrs/freebsd.c, etc)
>>> where the parent apr_xattrs.c in the unix directory just sucks in
>>> the correct implementation by ifdef tests...
>>
>> I went for this approach (as it was Davi's initial suggestion for
>> this directory structure also).
>>
>> It works. Only (minor) issue is the build-outputs.mk sources
>> dependencies for the files in the file_io/unix/xattr subdirectory
>> don't get generated.
>
> I'm confused, if file_io/unix/apr_xattr.c does
>
> #ifdef SOLARIS
> #include "xattr/solaris.c"
> #endif
>
> for example, how didn't it pick up the dependencies? Are the defines
> broken at that phase?
gen-build.py only appears to generate source dependencies for
MODULE/ARCH/*.c and I have them in file_io/unix/xattr/SUBARCH.c
eg contents of build-output.mk:
file_io/unix/xattr.lo: file_io/unix/xattr.c .make.dirs include/apr.h
include/apr_general.h include/apr_errno.h include/apr_inherit.h
include/apr_file_info.h include/apr_user.h include/apr_file_io.h
include/apr_allocator.h include/apr_want.h include/apr_thread_mutex.h
include/apr_pools.h include/apr_time.h include/apr_file_xattr.h
include/apr_tables.h
OBJECTS_file_io_unix = file_io/unix/filepath.lo file_io/unix/seek.lo
file_io/unix/buffer.lo file_io/unix/dir.lo file_io/unix/filepath_util.lo
file_io/unix/copy.lo file_io/unix/tempdir.lo file_io/unix/mktemp.lo
file_io/unix/filedup.lo file_io/unix/pipe.lo file_io/unix/flock.lo
file_io/unix/filestat.lo file_io/unix/xattr.lo file_io/unix/readwrite.lo
file_io/unix/fileacc.lo file_io/unix/fullrw.lo file_io/unix/open.lo
is missing file_io/unix/xattr/solaris.c, file_io/unix/xattr/linux.c, etc
So modifying an implementation doesn't trigger a rebuild of xattr.lo
Re: Extended Attributes Support
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Michael Clark wrote:
> William A. Rowe, Jr. wrote:
>> I'd go for a subdir option (xattrs/solaris.c, xattrs/freebsd.c, etc)
>> where the parent apr_xattrs.c in the unix directory just sucks in
>> the correct implementation by ifdef tests...
>
> I went for this approach (as it was Davi's initial suggestion for this
> directory structure also).
>
> It works. Only (minor) issue is the build-outputs.mk sources
> dependencies for the files in the file_io/unix/xattr subdirectory don't
> get generated.
I'm confused, if file_io/unix/apr_xattr.c does
#ifdef SOLARIS
#include "xattr/solaris.c"
#endif
for example, how didn't it pick up the dependencies? Are the defines
broken at that phase?
Re: Extended Attributes Support
Posted by Michael Clark <mi...@metaparadigm.com>.
William A. Rowe, Jr. wrote:
> I'd go for a subdir option (xattrs/solaris.c, xattrs/freebsd.c, etc)
> where the parent apr_xattrs.c in the unix directory just sucks in
> the correct implementation by ifdef tests...
I went for this approach (as it was Davi's initial suggestion for this
directory structure also).
It works. Only (minor) issue is the build-outputs.mk sources
dependencies for the files in the file_io/unix/xattr subdirectory don't
get generated.
>> Not sure how I would integrate this into the build (perhaps
>> having a single #if that create empty compilation units for
>> the inactive platforms?).
>
> the last #else case in apr_xattrs.c falls into APR_ENOTIMPL stubs.
>
>> Test cases should go into a new unit - not linked into testall?
>
> yes linked in. APR_ENOTIMPL just isn't a fail-case.
OK. I have it linked in and followed the threads approach of using a
conditional to define an unimplemented test and have it succeed with the
unimplemented message.
>> Also what is the procedure for proposing and adding a new API like
>> this. I guess my patch should have it disabled by default and enabled
>> with a configure flag - say --enable-xattrs (or autodetected and enabled
>> on supported platforms)?
>
> Nope - just enable it; this is on trunk so we have some time to get
> things right. When folks deploy conditional features it makes it
> a total nightmare for the next package which shares the libs.
OK. I have it enabled by default and symbols always will be linked in
plus there is a --disable-xattr option which will pull in the ENOTIMPL
versions, useful if someone has broken headers.
Thanks
Michael.
Re: Extended Attributes Support
Posted by Michael Clark <mi...@metaparadigm.com>.
OK. I think I've incorporated most if not all of the feedback
and I've raised an issue here:
http://issues.apache.org/bugzilla/show_bug.cgi?id=44127
Here is a second round of patches based on feedback from the list.
Changes:
* Moved headers out of apr_file_info.h into apr_file_xattr.h
* Change interface from const char* filename to use apr_file_t *file
* Removed APR_XATTR_NOFOLLOW from flags
(in line with change to apr_file_t* based interface)
* Changed flags apr_int32_t from to apr_uint32_t
* Updated tests and sample client apxattr to API revisions
* Clarified apr_get_xattr docs on pool allocation of void **value
* Moved from file_io/unix/xattr_(darwin|linux|freebsd|solaris).c to
file_io/unix/xattr/(darwin|linux|freebsd|solaris).c
* Added code to escape and unescape '/' characters in Solaris
subfile (attribute) names. Also added special char test cases.
* Added APR_ENOTIMPL stubs for netware, os2, win32 and unsupported unix
* Added --disable-xattr configure flag (enabled by default if detected)
Headers, build infrastructure and test cases:
http://oss.metaparadigm.com/apache-privsep/2.3.0-dev/xattr-patches/apr-xattr-headers.patch
http://oss.metaparadigm.com/apache-privsep/2.3.0-dev/xattr-patches/apr-xattr-build.patch
http://oss.metaparadigm.com/apache-privsep/2.3.0-dev/xattr-patches/apr-xattr-tests.patch
Mac OS X, FreeBSD, Linux and Solaris implementations plus ENOTIMPL stubs
for unsupported platforms
http://oss.metaparadigm.com/apache-privsep/2.3.0-dev/xattr-patches/apr-xattr-impl-stubs.patch
http://oss.metaparadigm.com/apache-privsep/2.3.0-dev/xattr-patches/apr-xattr-impl-darwin.patch
http://oss.metaparadigm.com/apache-privsep/2.3.0-dev/xattr-patches/apr-xattr-impl-freebsd.patch
http://oss.metaparadigm.com/apache-privsep/2.3.0-dev/xattr-patches/apr-xattr-impl-linux.patch
http://oss.metaparadigm.com/apache-privsep/2.3.0-dev/xattr-patches/apr-xattr-impl-solaris.patch
All-in-one patch (applies cleanly to trunk)
http://oss.metaparadigm.com/apache-privsep/2.3.0-dev/xattr-patches/apr-xattr-all.patch
apxattr - Utility to view/modify extended attributes on files
http://oss.metaparadigm.com/apache-privsep/2.3.0-dev/xattr-patches/apxattr.c
Re: Extended Attributes Support
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Michael Clark wrote:
>
> I have been doing some work on implementing extended attributes
> support for apr.
Cool beans.
> Not implemented:
>
> * Windows - should be able to use named :streams (similar to Solaris)
Actually you have streams, and you have ntfs EA's. They are actually
different beasts, AIUI.
> I have some questions on build structure...
>
> Should I put the declarations in apr_file_info.h or would it be
> better to add a apr_file_xattr.h header?
I'd suggest a new header (it can include apr_file_info for itself)
to keep things simple, if this api proves to grow.
> I have all the unix implementations rolled into one file
> (file_io/unix/xattr.c) with many #if's. Should I perhaps have
> a linux.c, darwin.c, freebsd.c, solaris.c in a subdirectory?
Well, I've been in the sendfile implementation. It's not pretty.
I'd go for a subdir option (xattrs/solaris.c, xattrs/freebsd.c, etc)
where the parent apr_xattrs.c in the unix directory just sucks in
the correct implementation by ifdef tests...
> Not sure how I would integrate this into the build (perhaps
> having a single #if that create empty compilation units for
> the inactive platforms?).
the last #else case in apr_xattrs.c falls into APR_ENOTIMPL stubs.
> Test cases should go into a new unit - not linked into testall?
yes linked in. APR_ENOTIMPL just isn't a fail-case.
> Also what is the procedure for proposing and adding a new API like
> this. I guess my patch should have it disabled by default and enabled
> with a configure flag - say --enable-xattrs (or autodetected and enabled
> on supported platforms)?
Nope - just enable it; this is on trunk so we have some time to get
things right. When folks deploy conditional features it makes it
a total nightmare for the next package which shares the libs.
Re: [Patch] Proposed Extended Attributes Implementation
Posted by Michael Clark <mi...@metaparadigm.com>.
Davi Arnaut wrote:
>> Would an apr_file_t based implementation be portable i.e. do you have a
>> call like Solaris' openat on windows that will open the named stream
>> relative to a file handle?
>>
>
> See above, in cases where there is no file descriptor based api, use the
> filename in the apr_file_t structure.
>
OK. Great. I'll go ahead with this approach.
Re: [Patch] Proposed Extended Attributes Implementation
Posted by Davi Arnaut <da...@apache.org>.
Michael Clark wrote:
> Davi Arnaut wrote:
>> Hi Michael,
>>
>> Michael Clark wrote:
>>
>>> Michael Clark wrote:
>>>
>>>> I have been doing some work on implementing extended attributes
>>>> support for apr.
>>>>
>>>> ...
>>>>
>>>> I'll send some initial patches soon. They still need a little work
>>>> and some more test cases.
>>>>
>>> I've come up with an initial set of patches and a sample client application.
>>>
>>>
>> [..]
>>
>>
>>> It may be a week or so before I get time to get a mod_dav_fs_props
>>> implementation that use these interfaces (which i'll post to httpd-dev).
>>> Perhaps also a nice patch for mod_autoindex xattr descriptions.
>>>
>>> Any committers out there who would like to sponsor this?
>>>
>> From a quick review, it looks good, but I would prefer a apr_file_t
>> based implementation. Other than this, please open a entry on bugzilla
>> so we can track more easily the patches/progress.
>>
>
> Thanks. I'll open a bugzilla entry for it.
>
> Okay, so you recommend a change from const char *filepath to apr_file_t
> *thefile and fetch the apr_os_file_t (file descriptors) and use the f*
> variants.
The file descriptor is accessible through apr_file_t::filedes.
> I had used filepath as it seemed consistent with the existing
> interfaces. i.e. apr_stat, apr_file_perms_set, etc do not have fstat,
> fchown equivalents.
Hum, we have apr_file_info_get for fstat equivalent. For file
permissions, the path make much more sense..
> I am okay with using file descriptors. Would it be good to have both?
> e.g. apr_file_xattr_get, apr_file_xattr_get_path
You can access the file name in apr_file_t::fname. Only the apr_file_t
one will suffice.
> Would an apr_file_t based implementation be portable i.e. do you have a
> call like Solaris' openat on windows that will open the named stream
> relative to a file handle?
See above, in cases where there is no file descriptor based api, use the
filename in the apr_file_t structure.
Regards,
Davi Arnaut
Re: [Patch] Proposed Extended Attributes Implementation
Posted by Michael Clark <mi...@metaparadigm.com>.
Davi Arnaut wrote:
> Hi Michael,
>
> Michael Clark wrote:
>
>> Michael Clark wrote:
>>
>>> I have been doing some work on implementing extended attributes
>>> support for apr.
>>>
>>> ...
>>>
>>> I'll send some initial patches soon. They still need a little work
>>> and some more test cases.
>>>
>> I've come up with an initial set of patches and a sample client application.
>>
>>
>
> [..]
>
>
>> It may be a week or so before I get time to get a mod_dav_fs_props
>> implementation that use these interfaces (which i'll post to httpd-dev).
>> Perhaps also a nice patch for mod_autoindex xattr descriptions.
>>
>> Any committers out there who would like to sponsor this?
>>
>
> From a quick review, it looks good, but I would prefer a apr_file_t
> based implementation. Other than this, please open a entry on bugzilla
> so we can track more easily the patches/progress.
>
Thanks. I'll open a bugzilla entry for it.
Okay, so you recommend a change from const char *filepath to apr_file_t
*thefile and fetch the apr_os_file_t (file descriptors) and use the f*
variants.
I had used filepath as it seemed consistent with the existing
interfaces. i.e. apr_stat, apr_file_perms_set, etc do not have fstat,
fchown equivalents.
I am okay with using file descriptors. Would it be good to have both?
e.g. apr_file_xattr_get, apr_file_xattr_get_path
Would an apr_file_t based implementation be portable i.e. do you have a
call like Solaris' openat on windows that will open the named stream
relative to a file handle?
Cheers,
Michael
Re: [Patch] Proposed Extended Attributes Implementation
Posted by Davi Arnaut <da...@apache.org>.
Hi Michael,
Michael Clark wrote:
> Michael Clark wrote:
>> I have been doing some work on implementing extended attributes
>> support for apr.
>>
>> ...
>>
>> I'll send some initial patches soon. They still need a little work
>> and some more test cases.
>
> I've come up with an initial set of patches and a sample client application.
>
[..]
>
> It may be a week or so before I get time to get a mod_dav_fs_props
> implementation that use these interfaces (which i'll post to httpd-dev).
> Perhaps also a nice patch for mod_autoindex xattr descriptions.
>
> Any committers out there who would like to sponsor this?
>From a quick review, it looks good, but I would prefer a apr_file_t
based implementation. Other than this, please open a entry on bugzilla
so we can track more easily the patches/progress.
Regards,
Davi Arnaut
Re: [Patch] Proposed Extended Attributes Implementation
Posted by Michael Clark <mi...@metaparadigm.com>.
Michael Clark wrote:
> http://oss.metaparadigm.com/apache-privsep/2.3.0-dev/xattr-patches/apr-xattr-impl-solaris.patch
>
Just a note - the solaris implementation has a bug as the '/' character
is not allowed in subfile names and this current implementation doesn't
escape the attribute names - this is a potential security issue - it is
not something I had overlooked - I just I had not added the
escaping/unescaping code for this yet. The other implementations are not
effected in this way. I will redo this patch.
I will in due course add some test cases for attributes with special
characters ('/', ':", '\', etc). A windows implementation that uses
:named streams will have the same issue and attribute names with special
characters will need to be escaped.
Michael.
[Patch] Proposed Extended Attributes Implementation
Posted by Michael Clark <mi...@metaparadigm.com>.
Michael Clark wrote:
> I have been doing some work on implementing extended attributes
> support for apr.
>
> ...
>
> I'll send some initial patches soon. They still need a little work
> and some more test cases.
I've come up with an initial set of patches and a sample client application.
* Headers, build infrastructure and test cases:
autoconf auto-detects the xattr flavour and should compile out and not
impact platforms without support. If found, adds APR_HAS_XATTR to apr.h
http://oss.metaparadigm.com/apache-privsep/2.3.0-dev/xattr-patches/apr-xattr-headers.patch
http://oss.metaparadigm.com/apache-privsep/2.3.0-dev/xattr-patches/apr-xattr-build.patch
http://oss.metaparadigm.com/apache-privsep/2.3.0-dev/xattr-patches/apr-xattr-tests.patch
* Mac OS X, FreeBSD, Linux and Solaris implementations
Test cases passing on all my installs of the above platforms (although
they don't yet test APR_XATTR_NOFOLLOW or attributes on directories)
http://oss.metaparadigm.com/apache-privsep/2.3.0-dev/xattr-patches/apr-xattr-impl-darwin.patch
http://oss.metaparadigm.com/apache-privsep/2.3.0-dev/xattr-patches/apr-xattr-impl-freebsd.patch
http://oss.metaparadigm.com/apache-privsep/2.3.0-dev/xattr-patches/apr-xattr-impl-linux.patch
http://oss.metaparadigm.com/apache-privsep/2.3.0-dev/xattr-patches/apr-xattr-impl-solaris.patch
* All-in-one patch (applies cleanly to trunk)
http://oss.metaparadigm.com/apache-privsep/2.3.0-dev/xattr-patches/apr-xattr-all.patch
* apxattr - Utility to view/modify extended attributes on files
http://oss.metaparadigm.com/apache-privsep/2.3.0-dev/xattr-patches/apxattr.c
It may be a week or so before I get time to get a mod_dav_fs_props
implementation that use these interfaces (which i'll post to httpd-dev).
Perhaps also a nice patch for mod_autoindex xattr descriptions.
Any committers out there who would like to sponsor this?
Michael.
Re: Extended Attributes Support
Posted by Michael Clark <mi...@metaparadigm.com>.
Michael Clark wrote:
> Davi Arnaut wrote:
>> Hum, I tend to prefer separate separate files having the #if to empty,
>> maybe in file_io/unix/xattr/ subdirectory. You can also put internal
>> headers in include/arch/...
>>
> OK. That is what I thought and had done this in the patches (although
> it is not yet in a subdirectory).
>
> I had them in file_io/unix/xattr/ they were not picked up by
> buildconfig and I wasn't sure where to tune this?
I've been looking at build.conf and gen-build.py
"...
# directories that have platform-specific code in them. the resulting
# pattern will be: SUBDIR/PLATFORM/*.c
platform_dirs =
dso file_io locks memory misc mmap network_io poll random
..."
It doesn't appear to pick up source from subdirs of the platform - only
directly in the platform dir itself.
So I don't think I can easily do file_io/unix/xattr/*.c
It would probably need to be file_io/unix/xattr/unix/*.c according to
the rules above (as gen-build.py doesn't break out any of the unices
into platforms except for aix).
So without complicating this too much I think I can either have (what I
have now):
file_io/unix/xattr_darwin.c
file_io/unix/xattr_linux.c
file_io/unix/xattr_freebsd.c
file_io/unix/xattr_solaris.c
...
or perhaps make it a top level (which I think might be the way to go to
keep it tidy):
xattr/unix/darwin.c
xattr/unix/linux.c
xattr/unix/freebsd.c
xattr/unix/solaris.c
Re: Extended Attributes Support
Posted by Michael Clark <mi...@metaparadigm.com>.
Davi Arnaut wrote:
> I prefer a new apr_file_xattr.h header.
>
OK. I'll do this.
>> I have all the unix implementations rolled into one file
>> (file_io/unix/xattr.c) with many #if's. Should I perhaps have
>> a linux.c, darwin.c, freebsd.c, solaris.c in a subdirectory?
>> Not sure how I would integrate this into the build (perhaps
>> having a single #if that create empty compilation units for
>> the inactive platforms?).
>>
>
> Hum, I tend to prefer separate separate files having the #if to empty,
> maybe in file_io/unix/xattr/ subdirectory. You can also put internal
> headers in include/arch/...
>
OK. That is what I thought and had done this in the patches (although it
is not yet in a subdirectory).
I had them in file_io/unix/xattr/ they were not picked up by buildconfig
and I wasn't sure where to tune this?
> Auto detected, with a configure option to disable (--disable-xattrs).
>
OK. I will add a disable option.
> s/specificed/specified/
>
Picked this one up since. thanks.
> s/filepath/file path/
>
OK.
> apr_uint32_t flags
>
This was a case of following what was in apr_file_info.h for the other
interfaces with a flags argument.
>> * @param filepath the full path to the file
>> * @param name the attribute name to get
>> * @param value the returned attribute value, if value is NULL then only
>> * the size of the attribute value is returned
>>
>
> Would be nice to have some kind of option to allocate from the pool,
> otherwise it doesn't make much sense to pass void **.
>
It does allocate. I will make this more clear in the docs.
Perhaps I can ditch the size only mode and not allow NULL? (it'll allow
removing a couple of lines)
I'll go through all of this feedback, add an issue to bugzilla and run
up another patch set.
Thanks much.
Re: Extended Attributes Support
Posted by Davi Arnaut <da...@apache.org>.
Hi Michael,
Michael Clark wrote:
> Hi Folks,
>
> I have been doing some work on implementing extended attributes
> support for apr.
Great. This is one of the features that I always miss in the APR.
> I am doing this as I would like to add a property provider to
> mod_dav_fs that uses extended attributes instead of a db per file
> (TwistedDAV on Mac OS X also uses extended attributes for this BTW)
> and I want to do this portably so it could potentially be included.
Good, this can also be useful to a lot of apache modules out there (in
particular, I would like to use it to store per cache file headers).
> Extended attributes can also be used for setting mime-types on files
> (mod_mime_xattr uses the native Linux API so this could be ported
> to use portable apr xattrs and work on more platforms). Another nice
> idea is setting file listing 'description' attributes and getting
> mod_autoindex to look at them.
Also good.
> I have looked at a few of the OS extended attributes interface and
> have come up with an API proposal (at the end of this mail).
>
> It addresses:
> * setting, getting, listing and removing of 'user' extended attributes
> on a file specified by its filepath.
>
> It does not address:
> * system namespaces on platforms with more than one attribute namespace
> (only the user namespace is accessible on platforms with multiple
> attr namespaces, new flags could potentially be added to access
> platform specific system namespaces)
> * setting, getting, listing and removing attributes on a file
> specified by a file descriptor (apr_os_file_t)
>
> I have working sample implementations for Linux, Mac OS X, FreeBSD 6
> and Solaris 10 (which each have different APIs BTW):
>
> * Linux impl use l?(get|set|list|remove)xattr
> * Mac OS X impl use (get|set|list|remove)xattr (different args to linux)
> * FreeBSD impl uses extattr_(get|set|list|delete)_(file|link)
> * Solaris impl uses subfiles (attropen,unlinkat and friends)
>
> Not implemented:
>
> * Windows - should be able to use named :streams (similar to Solaris)
> * OS/2 - it has extended attributes but that's all I know.
> * Irix/HPUX/AIX/OS390/netware/... - unknown? do they have them?
I may look into implementing it on Windows/HPUX later.
> Any ideas on feasibility on these and other apr supported platforms?
>
> I'll send some initial patches soon. They still need a little work
> and some more test cases.
>
> I have some questions on build structure...
>
> Should I put the declarations in apr_file_info.h or would it be
> better to add a apr_file_xattr.h header?
I prefer a new apr_file_xattr.h header.
> I have all the unix implementations rolled into one file
> (file_io/unix/xattr.c) with many #if's. Should I perhaps have
> a linux.c, darwin.c, freebsd.c, solaris.c in a subdirectory?
> Not sure how I would integrate this into the build (perhaps
> having a single #if that create empty compilation units for
> the inactive platforms?).
Hum, I tend to prefer separate separate files having the #if to empty,
maybe in file_io/unix/xattr/ subdirectory. You can also put internal
headers in include/arch/...
> Test cases should go into a new unit - not linked into testall?
>
> Also what is the procedure for proposing and adding a new API like
> this. I guess my patch should have it disabled by default and enabled
> with a configure flag - say --enable-xattrs (or autodetected and enabled
> on supported platforms)?
Auto detected, with a configure option to disable (--disable-xattrs).
> Proposed API:
>
> /**
> * @defgroup apr_file_xattr File Extended Attribute Functions
> * @{
> */
>
> /** Don't follow symbolic links flag for apr_file_xattr functions */
> #define APR_XATTR_NOFOLLOW 1
>
> /** When setting values, fail if the attribute already exists */
> #define APR_XATTR_CREATE 2
>
> /** When setting values, fail if the attribute does not already exist */
> #define APR_XATTR_REPLACE 4
>
> /**
> * Set an extended attribute on the file specificed by filepath
s/specificed/specified/
s/filepath/file path/
> * @param filepath the full path to the file
> * @param name the attribute name to set
> * @param value the attribute value
> * @param size the size in bytes of the attribute value
> * @param flags to control how the attribute is set
> * <PRE>
> * APR_XATTR_NOFOLLOW if the file is a symbolic link then the
> * attribute will be set on the link.
> * APR_XATTR_CREATE return an error if the attribute name
> * already exists.
> * APR_XATTR_REPLACE return an error if the attribute name
> * does not already exist.
> * </PRE>
> * @param p the pool to allocate any memory from if required
> * @remark if neither flag APR_XATTR_CREATE or APR_XATTR_REPLACE are
> * given then the attribute will either be created if it does not
> * already exist or replaced if it does exist.
> */
> APR_DECLARE(apr_status_t) apr_file_xattr_set(const char *filepath,
> const char *name,
> const void *value,
> apr_size_t size,
> apr_int32_t flags,
apr_uint32_t
> apr_pool_t *p);
What was the reasoning for not using a apr_file_t based implementation
and using fsetxattr and likes?
> /**
> * Get an extended attribute from the file specificed by filepath
Same as above.
> * @param filepath the full path to the file
> * @param name the attribute name to get
> * @param value the returned attribute value, if value is NULL then only
> * the size of the attribute value is returned
Would be nice to have some kind of option to allocate from the pool,
otherwise it doesn't make much sense to pass void **.
> * @param size the returned size of the attribute value
> * @param flags to control how the attribute is got
> * <PRE>
> * APR_XATTR_NOFOLLOW if the file is a symbolic link then the
> * attribute will be gotten from the link.
> * </PRE>
> * @param p the pool to allocate any memory from if required
> */
> APR_DECLARE(apr_status_t) apr_file_xattr_get(const char *filepath,
> const char *name,
> void **value,
> apr_size_t *size,
> apr_int32_t flags,
> apr_pool_t *p);
>
> /**
> * List the extended attributes for the file specificed by filepath
Same as above.
> * @param filepath the full path to the file
> * @param list the returned array of attributes names
> * @param flags to control how the file is listed
> * <PRE>
> * APR_XATTR_NOFOLLOW if the file is a symbolic link then the
> * attributes of the link will be listed.
> * </PRE>
> * @param p the pool to allocate any memory from if required
> * @remark list is an array containing simple null terminated strings.
> */
> APR_DECLARE(apr_status_t) apr_file_xattr_list(const char *filepath,
> apr_array_header_t **list,
> apr_int32_t flags,
> apr_pool_t *p);
>
> /**
> * Remove an extended attribute from the file specificed by filepath
Same as above.
> * @param filepath the full path to the file
> * @param name the attribute name to remove
> * @param flags to control how the attribute is removed
> * <PRE>
> * APR_XATTR_NOFOLLOW if the file is a symbolic link then the
> * attribute will be removed from the link.
> * </PRE>
> * @param p the pool to allocate any memory from if required
> */
> APR_DECLARE(apr_status_t) apr_file_xattr_remove(const char *filepath,
> const char *name,
> apr_int32_t flags,
apr_uint32_t
> apr_pool_t *p);
>
> /** @} */
>
Looks go so far.
Regards,
Davi Arnaut