Mailing-List: contact dev-help@apr.apache.org; run by ezmlm
Precedence: bulk
Received-SPF: neutral (nike.apache.org: local policy)
Message-ID: <476C2F0F.703@rowe-clan.net>
Date: Fri, 21 Dec 2007 15:24:31 -0600
From: "William A. Rowe, Jr." <wrowe@rowe-clan.net>
User-Agent: Thunderbird 2.0.0.9 (X11/20071115)
MIME-Version: 1.0
To: Michael Clark <michael@metaparadigm.com>
CC: dev@apr.apache.org
Subject: Re: Extended Attributes Support
References: <4769F773.10503@metaparadigm.com>
In-Reply-To: <4769F773.10503@metaparadigm.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

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.