httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Stein <gst...@lyra.org>
Subject Re: [PATCH] add Expat as an option
Date Sat, 17 Apr 1999 08:17:29 GMT
Ralf S. Engelschall wrote:
>...
> Sorry to complain, but I think your suck_in_expat() function doesn't work as
> expected on all platforms. Actually it's not only the problem that the stuff
> is linked in. You also have to make sure the symbols are actually _exported_.
> Look inside my EAPI patch I posted and there the src/ap/ap_mm.c and
> src/include/ap_mm.h files. There provide a complete(!) wrapper for the MM
> library and really solves the DSO problems.  I'm +1 for including expat, but
> only this safe way.

This was discussed before, and it appeared to me that people did not
concur with your idea to wrap every Expat function.

Second, the functions *are* exported on all platforms but Windows and
AIX (where the .def and .exp files must be updated). (I note that your
EAPI patch does not place ap_MM_* into those files(!)). I don't believe
that we can add those functions to the list unless we actually link them
in, which is problematic for optional functionality such as this.

I'll write the necessary gunk to dynamically append stuff to the .exp
and .def files, but might be willing to state that Expat is not usable
by DSOs on those platforms right now (yah, that's suboptimial, but
hey...).

Side note: we should probably torch the .def file since API_EXPORT uses
__declspec().

> Hmmm.. with Expat and MM and the existing regex stuff we've already three
> third-party libs. I would appreciate that we open a new src/lib/ or
> src/contrib/ directory under which regex/, mm/ and expat/ subdirs can exists.
> This way it's clear that it's third-party stuff. Additionally I wouldn't merge
> the headers into src/include/. Instead the appropriate -I$(SRC)/lib/expat/
> (added by src/Configure to CFLAGS) should be fine, shouldn't it?

I was following the regex methodology, which placed the header into
src/include. I was also seeking to prevent huge compilation lines. You'd
end up with something like this:

gcc foo.c -DUSE_REGEX -I$(SRC)/lib/regex/ -DUSE_EXPAT -I$(SRC)/lib/expat
-DUSE_MM -I$(SRC)/lib/mm ...

However, it is more modular to do the above.

I would not call the directory "contrib" as I believe that our shipping
it implies that we must support it. src/lib/foo seems more appropriate.

I'm not very comfortable with the Configure process, so would prefer to
commit the Expat stuff, then have somebody (you? :-) handle the shift to
src/lib/. I'm also willing to look into it and develop the patches, as
long as somebody says they'll look at them.

> > => Q: what should we do if a module says RULE_EXPAT=yes, but the
> > directory doesn't exist? This test would be at line 1552 (of the patched
> > Configure); what is the "approved" way to punt from Configure with an
> > error?
> 
> echo 1>&2 plus an exit 1?

No problem. I'll update the patch.

> > Presuming this goes in, then I'll follow up with an appropriate
> > src/expat/ subdirectory patch. That's a bit different decision: do we do
> > nothing in the core for Expat, do we add a framework but not ship Expat
> > itself, or do we add the framework and Expat?
> 
> I personally would say we should always provide the framework for Expat (even
> it's a dummy API like in ap_mm()). Whether we then actually ship with Expat is
> a different question.  Although I would appreciate a
> src/lib/{regex,mm,expat}/ area.

Right. I'll ask that question later. (I'm out the next two weeks, so it
will be a little while to get to that).

I'll do the src/lib/ stuff if: 1) people think it is required before
committing this patch, and 2) if nobody else can/will do the src/lib/
thingy.

Thx,
-g

--
Greg Stein, http://www.lyra.org/

Mime
View raw message