Re: Breaking something? Now is the time?
Justin Erenkrantz wrote: IIRC, I did macroize it during my test runs (at one point at least - I may not have committed it) and found no performance improvement. The problems seemed to be with the function pointer itself. It's all a little fuzzy though, so it's possible I didn't macroize. A better solution, IMHO, would be just to code a drop-in replacement for memory/unix/apr_pools.c. -- justin Perhaps it would be useful to know exactly what benchmarks people are using to identify the impact of these changes. -- Emery
Re: Breaking something? Now is the time?
On Fri, 28 Jun 2002, Justin Erenkrantz wrote:
> p->alloc
> and
> #define apr_palloc(...) p->alloc
None, but that's not what we were doing with SMS.
> to the fact that we used to have a function like this:
>
> apr_palloc()
> {
> return p->alloc();
> }
Yeah, but it did even more. It was more like:
apr_sms_alloc(sms, size)
{
/* ... do a bunch of stuff here ... */
rv = sms->type->alloc(sms, size);
/* ... do some more stuff ... */
return rv;
}
Re: Breaking something? Now is the time?
On Fri, Jun 28, 2002 at 12:22:01PM -0700, Brian Pane wrote:
> I think SMS's use of a wrapper function to do the indirect method
> call was the main problem, which is why we'd have to use a macro
> instead if we reintroduced a function pointer model.
Count me confused, but what is the difference between:
p->alloc
and
#define apr_palloc(...) p->alloc
Aren't they going to resolve to the same thing? Or are you referring
to the fact that we used to have a function like this:
apr_palloc()
{
return p->alloc();
}
IIRC, I did macroize it during my test runs (at one point at least -
I may not have committed it) and found no performance improvement.
The problems seemed to be with the function pointer itself. It's all
a little fuzzy though, so it's possible I didn't macroize.
A better solution, IMHO, would be just to code a drop-in replacement
for memory/unix/apr_pools.c. -- justin
Re: Breaking something? Now is the time?
Justin Erenkrantz wrote: On Fri, Jun 28, 2002 at 12:11:09PM -0700, Brian Pane wrote: I want to break something: binary compatibility for the pool API. This has been on my list for a long time, but I haven't yet had time to implement it. What I'm thinking of is the following: * Preface the apr_pool_t structure with a set of function pointers for the pool's "methods": alloc, free, destroy, create subpool, etc. Sounds like SMS. We could never overcome speed limitations and we always seemed to place blame on the function pointers as the reason why the SMS performance wasn't as good as pools. I think SMS's use of a wrapper function to do the indirect method call was the main problem, which is why we'd have to use a macro instead if we reintroduced a function pointer model. I'd want to see performance metrics saying that we aren't going to see a massive performance decrease with this. -- justin Definitely. --Brian
Re: Breaking something? Now is the time?
At 02:11 PM 6/28/2002, Brian Pane wrote: I want to break something: binary compatibility for the pool API. This has been on my list for a long time, but I haven't yet had time to implement it. What you are describing is [was] SMS. Even with the opaque structure, we are still facing derefs that will significantly alter performance. Definately look in CVS's attic for the original implementation to start with. We don't have the time to implement this in the current Apache release cycle [2.0.40] given the issues people raised with the original SMS implementation. I suggest that -if- we prove this is viable, we introduce it with the rollout of version 1.0. Bill
Re: APR_STATUS_* semantics [Re: Breaking something? Now is the time?]
On Fri, 28 Jun 2002, William A. Rowe, Jr. wrote: > >What I'd like to propose is that we document that, for any given status > >code, _more_ than one APR_STATUS_IS* macro can match, and it's the > >programmer's responsibility to decide in what order to make the tests. +1 --Cliff
Re: APR_STATUS_* semantics [Re: Breaking something? Now is the time?]
At 02:43 PM 6/28/2002, =?UTF-8?B?QnJhbmtvIMSMaWJlag==?= wrote:
Since we're talking about semantics, breakage, etc, I'll take the
opportunity to bore everybody with an issue I'd like resolved, too;
Namely, the semantics of the APR_STATUS_IS_* macros.
I've said several times before that APR_STATUS_IS_ENOENT and
APR_STATUS_IS_ENOTDIR don't have the same meaning on Windows and Unix.
That's because Windows doesn't have an error code that would mean exactly
the same as the Posix ENOTDIR. Simulating it would be a huge cost, though.
Here's an example of the differing behaviour: If "foo" does not exist,
doint an apr_stat("foo/bar") will trigger APR_STATUS_IS_ENOENT on Unix,
but APR_STATUS_IS_ENOTDIR on Windows. That makes it very hard to write a
porable "mkdir -p" implementation; and, indeed, apr_dir_make_recursive
can't work correctly on Windows because of that.
What I'd like to propose is that we document that, for any given status
code, _more_ than one APR_STATUS_IS* macro can match, and it's the
programmer's responsibility to decide in what order to make the tests.
Brane, thank you for a terrific explanation of the issue. +1 here.
Bill
APR_STATUS_* semantics [Re: Breaking something? Now is the time?]
Since we're talking about semantics, breakage, etc, I'll take the
opportunity to bore everybody with an issue I'd like resolved, too;
Namely, the semantics of the APR_STATUS_IS_* macros.
I've said several times before that APR_STATUS_IS_ENOENT and
APR_STATUS_IS_ENOTDIR don't have the same meaning on Windows and Unix.
That's because Windows doesn't have an error code that would mean
exactly the same as the Posix ENOTDIR. Simulating it would be a huge
cost, though.
Here's an example of the differing behaviour: If "foo" does not exist,
doint an apr_stat("foo/bar") will trigger APR_STATUS_IS_ENOENT on Unix,
but APR_STATUS_IS_ENOTDIR on Windows. That makes it very hard to write a
porable "mkdir -p" implementation; and, indeed, apr_dir_make_recursive
can't work correctly on Windows because of that.
What I'd like to propose is that we document that, for any given status
code, _more_ than one APR_STATUS_IS* macro can match, and it's the
programmer's responsibility to decide in what order to make the tests.
My proposed patch for the ENOENT issue would then be:
Index: apr_errno.h
===
RCS file: /home/cvs/apr/include/apr_errno.h,v
retrieving revision 1.91
diff -u -r1.91 apr_errno.h
--- apr_errno.h 20 May 2002 13:22:36 - 1.91
+++ apr_errno.h 28 Jun 2002 19:40:58 -
@@ -923,6 +923,7 @@
|| (s) == APR_OS_START_SYSERR + WSAENAMETOOLONG)
#define APR_STATUS_IS_ENOENT(s) ((s) == APR_ENOENT \
|| (s) == APR_OS_START_SYSERR + ERROR_FILE_NOT_FOUND \
+|| (s) == APR_OS_START_SYSERR + ERROR_PATH_NOT_FOUND \
|| (s) == APR_OS_START_SYSERR + ERROR_OPEN_FAILED \
|| (s) == APR_OS_START_SYSERR + ERROR_NO_MORE_FILES)
#define APR_STATUS_IS_ENOTDIR(s)((s) == APR_ENOTDIR \
Wrowe and I discussed this quite a bit, but haven't come to a final
decision yet. I'm bringing it up again because this is definitely
something that has to be fixed before we hit 1.0.
Thanks,
--
Brane Äibej <[EMAIL PROTECTED]> http://www.xbc.nu/brane/
Re: Breaking something? Now is the time?
On Fri, 28 Jun 2002, William A. Rowe, Jr. wrote: > If it is used by -anybody- they trust the existing implementation. > That said, it should behave sensibly. The fact that you've asked three > times says you want to change it. Hehehehe You noticed? :) Sorry to be a pest, I'm just getting sick of changing things only to have someone come behind a week later and say "you can't do that." I just wanted to be sure. Thanks, Cliff
RE: Breaking something? Now is the time?
> On Fri, Jun 28, 2002 at 12:11:09PM -0700, Brian Pane wrote: > > I want to break something: binary compatibility for the pool API. > > > > This has been on my list for a long time, but I haven't yet had > > time to implement it. > > > > What I'm thinking of is the following: > > > > * Preface the apr_pool_t structure with a set of function > > pointers for the pool's "methods": alloc, free, destroy, > > create subpool, etc. > > Sounds like SMS. We could never overcome speed limitations and we > always seemed to place blame on the function pointers as the reason > why the SMS performance wasn't as good as pools. > > I'd want to see performance metrics saying that we aren't going to > see a massive performance decrease with this. -- justin > Yes, please, we need some performance measurements. I've been doing some profiling of Apache 2.0 on AIX and even with mod_mem_cache, we still serve static files with keep-alive at about half the rate of iPlanet. The sad thing is I don't see any single smoking guns. Just lots of little stuff everywhere. Bill
Re: Breaking something? Now is the time?
If it is used by -anybody- they trust the existing implementation. That said, it should behave sensibly. The fact that you've asked three times says you want to change it. Make it so ;-) Bill At 01:38 PM 6/28/2002, Cliff Woolley wrote: On Fri, 28 Jun 2002, William A. Rowe, Jr. wrote: > IMHO, the implementation is what people have tested, not the documented > behavior. Use the source, luke :-) But what I'm saying is that I don't think anybody *has* tested it. I couldn't find a single use case in Apache where the called function would ever return anything other than 1, meaning that this "early-termination" functionality is not used by Apache AFAICT. --Cliff
Re: Breaking something? Now is the time?
On Fri, 28 Jun 2002, Justin Erenkrantz wrote: > Sounds like SMS. We could never overcome speed limitations and we > always seemed to place blame on the function pointers as the reason > why the SMS performance wasn't as good as pools. We had function pointers *and* wrapper functions. We never tried it with the macros approach (even though I wanted to, I didn't have time to code it before SMS was summarily nuked). > I'd want to see performance metrics saying that we aren't going to > see a massive performance decrease with this. -- justin Knowing Brian, I'm sure he wouldn't suggest anything without considering the performance impact first and foremost. :-] But yeah, I'd like to see the numbers too. --Cliff
Re: Breaking something? Now is the time?
On Fri, Jun 28, 2002 at 12:11:09PM -0700, Brian Pane wrote: > I want to break something: binary compatibility for the pool API. > > This has been on my list for a long time, but I haven't yet had > time to implement it. > > What I'm thinking of is the following: > > * Preface the apr_pool_t structure with a set of function > pointers for the pool's "methods": alloc, free, destroy, > create subpool, etc. Sounds like SMS. We could never overcome speed limitations and we always seemed to place blame on the function pointers as the reason why the SMS performance wasn't as good as pools. I'd want to see performance metrics saying that we aren't going to see a massive performance decrease with this. -- justin
Re: Breaking something? Now is the time?
I want to break something: binary compatibility for the pool API. This has been on my list for a long time, but I haven't yet had time to implement it. What I'm thinking of is the following: * Preface the apr_pool_t structure with a set of function pointers for the pool's "methods": alloc, free, destroy, create subpool, etc. * Replace the current pool functions with macros that call the right method for a given pool: #define apr_palloc(p, size) (*(p->alloc_fn))(p, size) (The point of using macro for this is to avoid the performance impact of adding another function call per apr_palloc.) This will let us introduce new pool variants, like reaps for example, without requiring changes to either the pool framework or anyone's application code. --Brian
Re: Breaking something? Now is the time?
On Fri, 28 Jun 2002, William A. Rowe, Jr. wrote: > IMHO, the implementation is what people have tested, not the documented > behavior. Use the source, luke :-) But what I'm saying is that I don't think anybody *has* tested it. I couldn't find a single use case in Apache where the called function would ever return anything other than 1, meaning that this "early-termination" functionality is not used by Apache AFAICT. --Cliff
Re: Breaking something? Now is the time?
At 01:22 PM 6/28/2002, you wrote: On Fri, 28 Jun 2002, William A. Rowe, Jr. wrote: > If Cliff wants to commit the semantic change to apr_table_[v]do, I'll > +1 here and raise you a _NONSTD correction. Along with Sander's > changes to make the unsafe transparent apr_allocator.h structure > opaque, I'd say we have a bit of worthwhile breakage to inflict before > we go on. By the way, 99.5% of coders will be unaffected by any of > these three changes. They can take advantage of the apr_table_[v]do > change or ignore it. So you didn't indicate an opinion on whether the existing semantics of apr_table_vdo() match their documentation, and if not, whether it's the docs or the implementation that have it right. I need to know in order to proceed with the return-type change. IMHO, the implementation is what people have tested, not the documented behavior. Use the source, luke :-)
Re: Breaking something? Now is the time?
On Fri, 28 Jun 2002, William A. Rowe, Jr. wrote: > If Cliff wants to commit the semantic change to apr_table_[v]do, I'll > +1 here and raise you a _NONSTD correction. Along with Sander's > changes to make the unsafe transparent apr_allocator.h structure > opaque, I'd say we have a bit of worthwhile breakage to inflict before > we go on. By the way, 99.5% of coders will be unaffected by any of > these three changes. They can take advantage of the apr_table_[v]do > change or ignore it. So you didn't indicate an opinion on whether the existing semantics of apr_table_vdo() match their documentation, and if not, whether it's the docs or the implementation that have it right. I need to know in order to proceed with the return-type change. Thanks... --Cliff
new httpd build running on daedalus
...since Friday, 28-Jun-2002 10:43:24 PDT. This build is basically 2.0.39 with a patch to apr_sendfile to deal with a change to the FreeBSD sendfile() API. We had 88 sendfile_it_all asserts pop today, so I decided to put the new build into production even though there was a fair amount of traffic. My apologies if I interrupted your download. Please speak up if you notice anything weird about the new httpd. Any new coredumps need to be taken seriously. Greg
Re: apr_table_do
At 12:36 PM 6/28/2002, Ben Laurie wrote: We really ought to have a better name than NONSTD(), coz I can't remember precisely what it means, and I invented it! However, ISTR its required if you have ... in the args? "The APR Project: We can't name APIs to save our lives" :-) It simply means that the caller must pop the args from the stack. That makes it incompatible (on win32 and possibly elsewhere) with other languages borrowing that specific function. Where we have the choice, we have chosen to let the callee pop the args, which means it must know about the arguments passed. So you are right, ... always defeats that convention :) Letting the callee pop the args means the APR library remains as compatible as possible for integration into other languages, such as fortran, basic and pascal (callee popping the stack is sometimes called the pascal convention on win32.) For Windows this gives us another win. If someone adds another arg without recompiling the apps, the original symbol (say, [EMAIL PROTECTED]) isn't found, since the new function has a redecorated name ([EMAIL PROTECTED]). This prevents us from ever confusing binaries and libs that would cause the software to fault. Instead, the binary just won't load. Bill
Re: apr_table_do
William A. Rowe, Jr. wrote: At 02:13 AM 6/28/2002, Cliff Woolley wrote: [...] First, why is apr_table_do APR_DECLARE_NONSTD()'d in the header file, but APR_DECLARE()'d in the .c file? I'm guessing the _NONSTD() is the right one, but I'm still a bit hazy on these things. That is a bug, AFAICT. It should be picked up by Win32 unless we neglected to include the relevant declaration header when we hit the function's code. Since it didn't, that means MSVC had overridden our preference since our preference couldn't be honored. Unfortunately, as is their way, they never saw fit to _announce_ this with an emit. It has been _NONSTD() all this time. The do processor fn's themselves must be exported with APR_DECLARE_NONSTD(), and then _only_ if they need to be an exported entry point (e.g. 99% of the time the process fn is simply a static fn in the source file that declares it.) We really ought to have a better name than NONSTD(), coz I can't remember precisely what it means, and I invented it! However, ISTR its required if you have ... in the args? Cheers, Ben. -- http://www.apache-ssl.org/ben.html http://www.thebunker.net/ "There is no limit to what a man can do or how far he can go if he doesn't mind who gets the credit." - Robert Woodruff
Re: Breaking something? Now is the time?
On Fri, 28 Jun 2002, William A. Rowe, Jr. wrote: > I have one bit that must be broken before 1.0, and cannot be remedied easily. > I'd like to simply break these things before Apache 2.0.40 is tagged. +1 on all counts. 2.0.40 will already require a full recompile anyway. Other users of APR must understand that some things must be fixed prior to APR 1.0, and these are they. --Cliff
Re: apr_private.h not being built properly on daedalus (freebsd 4.6) today
On 28 Jun 2002, Jeff Trawick wrote: > The first fun comes during buildconf processing: > > $ ./buildconf > buildconf: checking installation... > buildconf: autoconf version 2.53 (ok) > buildconf: libtool version 1.3.4 (ok) > Copying libtool helper files ... > Creating include/arch/unix/apr_private.h.in ... > WARNING: Using auxiliary files such as `acconfig.h', `config.h.bot' > WARNING: and `config.h.top', to define templates for `config.h.in' > WARNING: is deprecated and discouraged. That's not specific to *BSD. It's just an overly verbose "this is deprecated" message. > during configure I get these messages like this when creating each of > the make files (probably not a fatal condition): > config.status: creating Makefile > mv: Makefile: set owner/group (was: 1121/0): Operation not permitted > make works, so I overreacted about all the messages. I saw the same in icarus. I have no bloody idea what it's talking about. This one *is* *BSD specific (maybe even FreeBSD 4.6 specific, not sure). Anyway, as you discovered, you can safely ignore this message. --Cliff
Building APR on win32
Does anyone know how to build APR on win32 with ldap support? On Unix you can use: ./configure --with-ldap --with-ldap-include=/include --with-ldap-li b=/lib I'm trying to figure out how to accomplish the same thing on NT4. If anyone could give me some pointers, I would greatly appreciate it. Thanks in advance. , Josh.
Re: Only ONE back-compat hole discovered.
At 07:18 AM 6/27/2002, Thom May wrote: * Justin Erenkrantz ([EMAIL PROTECTED]) wrote : > On Thu, Jun 27, 2002 at 09:42:12AM +0200, Sander Striker wrote: > > The options on this are: > > > 5) Do nothing as APR isn't released yet. > > Personally, I don't care about backwards compatibility until > we hit 1.0 of APR. However, the fact that so much of APR has > stayed the same probably means that we can jump to 1.0. hrrm. I'm a fairly strong -1 on bumping apr to 1.0 at this point - we need to get the API consistent and clean, otherwise we're stuck with it for far too long I agree here. There are just a few hiccups to work out yet. WHEN we bump, all old/deprecated symbols and interfaces go kapoof. That should happen when we are certain we aren't deprecating any others, for no good reason other than "we can't name things to save our lives." Sander's goof in not keeping the ordering of fields can't be remedied now. But we can, at least, put wrappers around such things so that they can't be hurt by casual coding against apr, such as our own apr-util bucket code. That includes not using the struct alignment as a macro of the sizeof(), but rather as a function that returns -this- versions' sizeof(). My objection to the apr_allocator issue isn't that it was borked before 1.0 went out. My objection is that we may discover ourselves borking it again post-1.0 which would be badness. Clean opaque types prevent us from repeating this mistake. Bill
Breaking something? Now is the time?
I have one bit that must be broken before 1.0, and cannot be remedied easily.
I'd like to simply break these things before Apache 2.0.40 is tagged.
apr_pstrcatv should have never been declared _NONSTD, it was and there
isn't much we can do about it without breaking binary compat or introducing
a replacement symbol name. Since we like our apr_pstrcat and apr_foov
conventions, I don't want to rename.
This is very similar to Cliff's requested change to apr_table_[v]do syntax,
returning an int instead of a void. We like the existing names there, too.
There are two subtle differences though.
On Win32 (the only platform that cares about _NONSTD), the symbol name
will actually _change_ when it goes from _NONSTD (where it is 'apr_pstrcatv')
over to APR_DECLARE() (where it will be [EMAIL PROTECTED] ... designating
the stack args size per MS exports convention.)
With the apr_table_[v]do change, we won't change the declaration. Only the
return arg will change. I agree the odds are that all platforms return as
register
and that the stack isn't affected. Odds are that the register will be silently
discarded without harm or foul. The possibility that the return value is
on the
stack in some c implementations remains, however.
If we insist on breaking things, occasionally, pre-1.0, this looks like the
time.
If Cliff wants to commit the semantic change to apr_table_[v]do, I'll +1
here and
raise you a _NONSTD correction. Along with Sander's changes to make the
unsafe transparent apr_allocator.h structure opaque, I'd say we have a bit
of worthwhile breakage to inflict before we go on.
By the way, 99.5% of coders will be unaffected by any of these three changes.
They can take advantage of the apr_table_[v]do change or ignore it. Most folks
haven't implemented custom apr_allocators just yet - those that have likely
followed its progress. And the Win32 change simply requires a recompile, which
is unfortunate, but can't be avoided due to the ap_allocator.h stuff anyways.
Comments?
Bill
--- include/apr_strings.h 12 May 2002 00:56:26 - 1.25
+++ include/apr_strings.h 28 Jun 2002 14:21:07 -
@@ -179,8 +179,8 @@
* @param nbytes (output) strlen of new string (pass in NULL to omit)
* @return The new string
*/
-APR_DECLARE_NONSTD(char *) apr_pstrcatv(apr_pool_t *p, const struct iovec
*vec,
-apr_size_t nvec, apr_size_t *nbytes);
+APR_DECLARE(char *) apr_pstrcatv(apr_pool_t *p, const struct iovec *vec,
+ apr_size_t nvec, apr_size_t *nbytes);
/**
* printf-style style printing routine. The data is output to a string
--- strings/apr_strings.c 13 May 2002 16:09:22 - 1.27
+++ strings/apr_strings.c 28 Jun 2002 14:21:07 -
@@ -177,8 +177,8 @@
return res;
}
-APR_DECLARE_NONSTD(char *) apr_pstrcatv(apr_pool_t *a, const struct iovec
*vec,
-apr_size_t nvec, apr_size_t *nbytes)
+APR_DECLARE(char *) apr_pstrcatv(apr_pool_t *a, const struct iovec *vec,
+ apr_size_t nvec, apr_size_t *nbytes)
{
apr_size_t i;
apr_size_t len;
Re: apr_private.h not being built properly on daedalus (freebsd 4.6) today
"Sander Striker" <[EMAIL PROTECTED]> writes: > > *slight complication... during this sed trauma, autoconf 2.53 became > > the default autoconf on daedalus, and it doesn't get along well with > > APR; > > Is that *BSD specific? ac2.53 works fine for me on linux with APR. perhaps... The first fun comes during buildconf processing: $ ./buildconf buildconf: checking installation... buildconf: autoconf version 2.53 (ok) buildconf: libtool version 1.3.4 (ok) Copying libtool helper files ... Creating include/arch/unix/apr_private.h.in ... WARNING: Using auxiliary files such as `acconfig.h', `config.h.bot' WARNING: and `config.h.top', to define templates for `config.h.in' WARNING: is deprecated and discouraged. WARNING: Using the third argument of `AC_DEFINE' and WARNING: `AC_DEFINE_UNQUOTED' allows to define a template without WARNING: `acconfig.h': WARNING: AC_DEFINE([NEED_MAIN], 1, WARNING: [Define if a function `main' is needed.]) WARNING: More sophisticated templates can also be produced, see the WARNING: documentation. autoheader: `include/arch/unix/apr_private.h.in' is created Creating configure ... during configure I get these messages like this when creating each of the make files (probably not a fatal condition): config.status: creating Makefile mv: Makefile: set owner/group (was: 1121/0): Operation not permitted make works, so I overreacted about all the messages. -- Jeff Trawick | [EMAIL PROTECTED] Born in Roswell... married an alien...
Re: apr_table_do
At 02:13 AM 6/28/2002, Cliff Woolley wrote: [...] First, why is apr_table_do APR_DECLARE_NONSTD()'d in the header file, but APR_DECLARE()'d in the .c file? I'm guessing the _NONSTD() is the right one, but I'm still a bit hazy on these things. That is a bug, AFAICT. It should be picked up by Win32 unless we neglected to include the relevant declaration header when we hit the function's code. Since it didn't, that means MSVC had overridden our preference since our preference couldn't be honored. Unfortunately, as is their way, they never saw fit to _announce_ this with an emit. It has been _NONSTD() all this time. The do processor fn's themselves must be exported with APR_DECLARE_NONSTD(), and then _only_ if they need to be an exported entry point (e.g. 99% of the time the process fn is simply a static fn in the source file that declares it.) -APR_DECLARE_NONSTD(void) apr_table_do(int (*comp)(void *, const char *, const char *), +APR_DECLARE_NONSTD(int) apr_table_do(int (*comp)(void *, const char *, const char *), void *rec, const apr_table_t *t, ...); Please create a new fn, and leave a fn apr_table_do() that simply invokes your new apr_table_ fn, discarding the return code. You can notate apr_table_do as deprecated in the next major release of apr (1.0) as we've done for the renames. Please don't break binary compat in this way when we have no proper versioning in place just yet to track such a change. In any case, I noticed this was odd when I looked at using the function once before (and this is the reason I never did.) Great thought, and following my suggestion above, +1! Bill
RE: apr_private.h not being built properly on daedalus (freebsd 4.6) today
> From: [EMAIL PROTECTED] > Sent: 28 June 2002 13:53 > Jeff Trawick <[EMAIL PROTECTED]> writes: > > > The header file defines in apr_private.h are busted and APR won't > > build. Here is a glaring example: > > Summary of ths solution: > > We picked up a bad sed from 4.6-STABLE, which broke this. We then > picked up a subsequent fix, and we build again on daedalus*. > > *slight complication... during this sed trauma, autoconf 2.53 became > the default autoconf on daedalus, and it doesn't get along well with > APR; Is that *BSD specific? ac2.53 works fine for me on linux with APR. > I used my own local install of pure GNU autoconf 2.13 to build > APR Sander
Re: apr_private.h not being built properly on daedalus (freebsd 4.6) today
Jeff Trawick <[EMAIL PROTECTED]> writes: > The header file defines in apr_private.h are busted and APR won't > build. Here is a glaring example: Summary of ths solution: We picked up a bad sed from 4.6-STABLE, which broke this. We then picked up a subsequent fix, and we build again on daedalus*. *slight complication... during this sed trauma, autoconf 2.53 became the default autoconf on daedalus, and it doesn't get along well with APR; I used my own local install of pure GNU autoconf 2.13 to build APR -- Jeff Trawick | [EMAIL PROTECTED] Born in Roswell... married an alien...
apr_table_do
[[ None of my emails from this evening seem to have actually gone out (misconfig on my end, I think), so here's this again. Sorry if it's a dupe. ]] -- Forwarded message -- Date: Fri, 28 Jun 2002 02:50:06 -0400 (EDT) From: Cliff Woolley <[EMAIL PROTECTED]> To: dev@apr.apache.org, Roy Fielding <[EMAIL PROTECTED]> Subject: apr_table_do So I was looking into changing apr_table_do() and apr_table_vdo() to return an int instead of void, as below. Basically what I want to do is to know whether the table_do stopped because one of the iterations returned false or whether it stopped because it finished processing the whole table. Note that I can't find any current users of apr_table_do() that actually use the return-false functionality; all of the (*comp) functions used just return 1; unconditionally AFAICT. And changing from void to int would not pose an inconvenience for existing callers anyway; they could just ignore the return value with no code changes needed of course. But I've run into two questions. First, why is apr_table_do APR_DECLARE_NONSTD()'d in the header file, but APR_DECLARE()'d in the .c file? I'm guessing the _NONSTD() is the right one, but I'm still a bit hazy on these things. Second, it doesn't seem to me that apr_table_vdo()'s "halt if false" functionality works right, or at least it is inconsistent with what I would have expected from the docs. You can pass to apr_table_vdo an optional list of keys, and then it will only run the (*comp) function against those entries in the table. Or you can leave that off and it will run against all items in the table. But the way it's implemented, the list of keys is the outer loop rather than the inner loop. It halts the INNER loop if (*comp) returns false, but then proceeds on to the next iteration. So if I said: apr_table_vdo(myfunc, myrec, mytable, "key1", "key2"); then all of mytable's elts are compared against "key1," and myfunc is run against any elt that matches as long as myfunc returns 1. But let's say myfunc returned 0 before reaching the end of the table. We'd still go on and start over at the beginning of the table with "key2", forgetting that we didn't finish the table for "key1". Is that intentional? I wouldn't think so. But if it is, then the documentation should definitely be made more verbose, because it's totally unclear as-is IMO. Anyway, ignoring the second question for a second because my use case actually doesn't pass any keys anyway, here's a sample patch to give you an idea of what I'm getting at. The real patch would have to account for item 2, probably by swapping the inner and outer loops of apr_table_vdo. Thanks, --Cliff Index: srclib/apr/include/apr_tables.h === RCS file: /home/cvs/apr/include/apr_tables.h,v retrieving revision 1.28 diff -u -d -r1.28 apr_tables.h --- srclib/apr/include/apr_tables.h 21 Jun 2002 14:24:25 - 1.28 +++ srclib/apr/include/apr_tables.h 28 Jun 2002 06:30:54 - @@ -361,8 +361,10 @@ * @param ... The vararg. If this is NULL, then all elements in the table are *run through the function, otherwise only those whose key matches *are run. + * @return FALSE if one of the comp() iterations returned zero; TRUE if all + *iterations returned non-zero */ -APR_DECLARE_NONSTD(void) apr_table_do(int (*comp)(void *, const char *, const char *), +APR_DECLARE_NONSTD(int) apr_table_do(int (*comp)(void *, const char *, const char *), void *rec, const apr_table_t *t, ...); /** @@ -377,9 +379,11 @@ * @param vp The vararg table. If this is NULL, then all elements in the *table are run through the function, otherwise only those *whose key matches are run. + * @return FALSE if one of the comp() iterations returned zero; TRUE if all + *iterations returned non-zero */ -APR_DECLARE(void) apr_table_vdo(int (*comp)(void *, const char *, const char *), -void *rec, const apr_table_t *t, va_list); +APR_DECLARE(int) apr_table_vdo(int (*comp)(void *, const char *, const char *), + void *rec, const apr_table_t *t, va_list); /** flag for overlap to use apr_table_setn */ #define APR_OVERLAP_TABLES_SET (0) Index: srclib/apr/tables/apr_tables.c === RCS file: /home/cvs/apr/tables/apr_tables.c,v retrieving revision 1.26 diff -u -d -r1.26 apr_tables.c --- srclib/apr/tables/apr_tables.c 11 Jun 2002 18:07:03 - 1.26 +++ srclib/apr/tables/apr_tables.c 28 Jun 2002 06:30:55 - @@ -695,20 +695,26 @@ * * So to make mod_file_cache easier to maintain, it's a good thing */ -APR_DECLARE(void) apr_table_do(int (*comp) (void *, const char *, const char *), - void *rec, const apr_table_t *t, ...) +APR_DECLARE_NONSTD(int)
