apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael Clark <mich...@metaparadigm.com>
Subject Re: Extended Attributes Support
Date Fri, 21 Dec 2007 09:52:25 GMT
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/


> 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.

View raw message