Re: APR/APR-util commit messages
On Sun, 24 Nov 2002 [EMAIL PROTECTED] wrote: > Either we need to change the CVS commit script to handle this case and > copy both cvs lists*, or we need to somehow stop this from being allowed. > In the meantime, APR developers, please do not commit to APR from the top > of the httpd tree. Well, I wouldn't mind having the CVS messages magically arrive at the right lists, but then again, I've always thought it the case that we were *supposed* to be careful not to commit to APR from httpd's tree. So whether we just enforce this (is it possible?), trust people to be more careful next time, or magicify the commit mails doesn't matter to me. --Cliff
bucket brigades slides online
I forgot to mention this at the end of my talk... I've placed my slides (with detailed notes) from my ApacheCon talk on Bucket Brigades online. For those of you who have the CD-ROM from the conference, the slides haven't changed since then. For everybody else, you can find them in both .ppt and .pdf formats at: http://www.apache.org/~jwoolley/bucketbrigades/ --Cliff
[PATCH] the mmap problem
This patch should address the mmap problems. What it does is ditch the
ownership flag and replace it with something resembling a reference count.
It's not actually a refcount because .. where would you store the
refcount? I talked with a few of the guys about that possibility but
everything we came up with was kind of ugly. So an alternative that
somebody (Sander?) came up with was to put all of the dup'ed apr_mmap_t's
that refer to the same mmap'ed region in a ring with each other... that
way you don't have to have a single centralized location for the refcount
and you don't have lifetime issues to worry about anymore.
Greg Ames did some testing on this and says it looks good as far as he can
tell... additional eyes would be appreciated.
--Cliff
Index: srclib/apr/include/apr_mmap.h
===
RCS file: /home/cvs/apr/include/apr_mmap.h,v
retrieving revision 1.33
diff -u -d -r1.33 apr_mmap.h
--- srclib/apr/include/apr_mmap.h 10 Nov 2002 08:35:16 - 1.33
+++ srclib/apr/include/apr_mmap.h 21 Nov 2002 19:52:30 -
@@ -58,6 +58,7 @@
#include "apr.h"
#include "apr_pools.h"
#include "apr_errno.h"
+#include "apr_ring.h"
#include "apr_file_io.h"/* for apr_file_t */
#ifdef BEOS
@@ -115,8 +116,12 @@
void *mm;
/** The amount of data in the mmap */
apr_size_t size;
-/** Whether this object is reponsible for doing the munmap */
-int is_owner;
+/** @deprecated this field is no longer used and will be removed
+ * in APR 1.0 */
+int unused;
+/** ring of apr_mmap_t's that reference the same
+ * mmap'ed region; acts in place of a reference count */
+APR_RING_ENTRY(apr_mmap_t) link;
};
#if APR_HAS_MMAP || defined(DOXYGEN)
@@ -174,8 +179,8 @@
* @param new_mmap The structure to duplicate into.
* @param old_mmap The mmap to duplicate.
* @param p The pool to use for new_mmap.
- * @param transfer_ownership Whether responsibility for destroying
- * the memory-mapped segment is transferred from old_mmap to new_mmap
+ * @param transfer_ownership DEPRECATED: this param is now ignored
+ * and should be removed prior to APR 1.0
*/
APR_DECLARE(apr_status_t) apr_mmap_dup(apr_mmap_t **new_mmap,
apr_mmap_t *old_mmap,
@@ -185,10 +190,11 @@
#if defined(DOXYGEN)
/**
* Transfer the specified MMAP to a different pool
- * @param new_mmap The structure to duplicate into.
+ * @param new_mmap The structure to duplicate into.
* @param old_mmap The file to transfer.
* @param p The pool to use for new_mmap.
- * @remark After this call, old_mmap cannot be used
+ * @deprecated Just use apr_mmap_dup(). The transfer_ownership flag will
+ * go away soon anyway.
*/
APR_DECLARE(apr_status_t) apr_mmap_setaside(apr_mmap_t **new_mmap,
apr_mmap_t *old_mmap,
@@ -196,7 +202,6 @@
#else
#define apr_mmap_setaside(new_mmap, old_mmap, p) apr_mmap_dup(new_mmap,
old_mmap, p, 1)
#endif /* DOXYGEN */
-
/**
* Remove a mmap'ed.
Index: srclib/apr/mmap/unix/mmap.c
===
RCS file: /home/cvs/apr/mmap/unix/mmap.c,v
retrieving revision 1.43
diff -u -d -r1.43 mmap.c
--- srclib/apr/mmap/unix/mmap.c 16 Apr 2002 20:25:57 - 1.43
+++ srclib/apr/mmap/unix/mmap.c 21 Nov 2002 19:52:30 -
@@ -83,13 +83,17 @@
static apr_status_t mmap_cleanup(void *themmap)
{
apr_mmap_t *mm = themmap;
-int rv;
+apr_mmap_t *next = APR_RING_NEXT(mm,link);
+int rv = 0;
-if (!mm->is_owner) {
-return APR_EINVAL;
-}
-else if (mm->mm == (void *)-1) {
-return APR_ENOENT;
+/* we no longer refer to the mmaped region */
+APR_RING_REMOVE(mm,link);
+APR_RING_NEXT(mm,link) = NULL;
+APR_RING_PREV(mm,link) = NULL;
+
+if (next != mm) {
+/* more references exist, so we're done */
+return APR_SUCCESS;
}
#ifdef BEOS
@@ -163,7 +167,7 @@
(*new)->mm = mm;
(*new)->size = size;
(*new)->cntxt = cont;
-(*new)->is_owner = 1;
+APR_RING_ELEM_INIT(*new, link);
/* register the cleanup... */
apr_pool_cleanup_register((*new)->cntxt, (void*)(*new), mmap_cleanup,
@@ -179,21 +183,10 @@
*new_mmap = (apr_mmap_t *)apr_pmemdup(p, old_mmap, sizeof(apr_mmap_t));
(*new_mmap)->cntxt = p;
-/* The old_mmap can transfer ownership only if the old_mmap itself
- * is an owner of the mmap'ed segment.
- */
-if (old_mmap->is_owner) {
-if (transfer_ownership) {
-(*new_mmap)->is_owner = 1;
-old_mmap->is_owner = 0;
-apr_pool_cleanup_kill(old_mmap->cntxt, old_mmap, mmap_cleanup);
-apr_pool_cleanup_register(p, *new_mmap, mmap_cleanup,
- apr_pool_cleanup_null);
-}
-else {
-(*new_mmap)->is_owner = 0;
-}
-}
+APR_RING_INSERT_AFTER(old
Re: apr_mmap_t
On Wed, 20 Nov 2002 [EMAIL PROTECTED] wrote: > Stop using an incomplete type for MMAP files. The only sane way to really > store an MMAP file as far as I can see is as a void * and a length. BeOS > requires another variable, but it doesn't do any harm to expose that too. Well, there's more to it than that now. :) Not that much more in Unix, but more for sure on Win32 et al. > What are you adding to MMAPs? I'm working on a patch for the mmap cleanup problems that have been plaguing us. I'll post the patch when it's done (very soon). --Cliff
apr_mmap_t
Does anybody have any idea at all why apr_mmap_t is not an incomplete type? I have to change its size and that's a bit of a PITA because, while it SHOULD not, it COULD break binary compat for stupid modules who use sizeof(apr_mmap_t) for something. --Cliff
Re: UNIX -> APR sockets, file handlers
On Wed, 23 Oct 2002 [EMAIL PROTECTED] wrote: > The same thing exists for sockets. Just look in apr_portable.h. The > general rule, is that all APR types have a way to convert from the native > version to the APR version. Well, that's what I thought, but then I had gone to apr_portable.h and didn't see it. O, I know why. I searched for apr_os_socket_put() but it's called apr_os_sock_put(). Bah humbug. :) Thanks Ryan.. --Cliff
Re: UNIX -> APR sockets, file handlers
On Wed, 23 Oct 2002, Damir Dezeljin wrote:
> I'm looking how to convert an UNIX socket to APR socket (apr_socket_t).
> Can anyone help me please? Is there also any way to convert stdin/stdout
> to apr_socket_t (I'm trying to use APR to write a simple daemon example,
> one for inetd, so I need stdin/stdout, and the other one more general).
> Also is there any way to convert file descriptor (int and/or FILE *) to
> APR file descriptors?
Well, for int fd's, you can use apr_os_file_put(), or apr_os_pipe_put() if
it's a pipe [see apr_portable.h].
I'm not aware of any currently-existing equivalent for sockets, but we
might be able to add one.
So for stdin, for example:
{
int fd = 0;/* stdin */
apr_file_t *file;
return apr_os_file_put(&file, &fd, 0, pool);
}
This function, coincidentally, is a routine called apr_file_open_stdin(),
which is the more portable way to get an apr_file_t() for stdin (it's
implemented differently on windows, for example).
Hope this helps,
--Cliff
Re: APR_RING_* documentation?
On Thu, 17 Oct 2002, Dave Seidel wrote: > Is there ant documentation on the APR_RING_* macros besides the comments > in apr_ring.h, e.g., does anyone know of any articles on how these > macros should be used? Thanks in advance. The documentation in the header file is pretty much it. Though it's helpful to see how they're used by example; for that, take a look at the bucket brigades code in APR-util (apr_buckets.h and buckets/*.c). --Cliff
Re: Final tally for apr-20021001-aprpmcchair
On Mon, 14 Oct 2002, Aaron Bannert wrote: > many thanks to you Ryan for the energy you put in to this community. Indeed! --Cliff
RE: Final tally for apr-20021001-aprpmcchair
On Mon, 14 Oct 2002, Sander Striker wrote: > Congrats Cliff! I'm sure you'll make a fine Chairman. I'm in shock... I was all ready to congratulate one of you two guys. :) Thanks! --Cliff
Re: Built APR tarball?
On Tue, 1 Oct 2002, Pier Fumagalli wrote: > > And just to be clear, APR isn't a module -- it's a library. You mentioned > > Apache 1.3.26; I know that somebody was working to get an Apache 1.3 > > module using APR, but I seem to recall it was nontrivial. I guess I'm > > just curious what you're wanting APR for if not to develop and if not to > > run Apache 2.0 or Subversion with. > > If it is for mod_webapp, it's _so_trivial_... You can download the sources Oh, I bet that was it. But by nontrivial, I meant it was nontrivial to get APR and Apache 1.3 to coexist from a developer's perspective, not tha it was nontrivial to get whichever module it was built and installed. :)
Re: Built APR tarball?
On Mon, 30 Sep 2002, Luis Manuel Solla wrote: > I've been looking for a *built* tarball of the APR module, but I > can't find an exact URL. I prefer built tarballs rather than source > tarballs. Could anybody indicate an URL? Since we haven't released APR 1.0 yet, we haven't released any binary builds. Only source code. And just to be clear, APR isn't a module -- it's a library. You mentioned Apache 1.3.26; I know that somebody was working to get an Apache 1.3 module using APR, but I seem to recall it was nontrivial. I guess I'm just curious what you're wanting APR for if not to develop and if not to run Apache 2.0 or Subversion with. --Cliff
Re: cvs commit: apr-util CHANGES
On Mon, 23 Sep 2002, Greg Stein wrote: > The setaside is not your problem. You cannot and should not guess what is > going to or *might* happen. The brigades are supposed to be about *avoiding* > copies. If/when a copy *must* be made, then fine. But don't make judgements > in advance because you *don't have the info*. > > If the core_output filter is in use, then let it combine the small buckets. > Don't do it up front. There are too many other things that might happen, and > it isn't right to pre-judge. I'm with Greg on this one. Just make a bunch of transient buckets and pass the brigade. --Cliff
Re: Disabling the uid/gid setting of SysV shared memory
On Mon, 23 Sep 2002, Jim Jagielski wrote: > is intrinsicly non-threadsafe (via a static or global state var) > or one that requires a change to the apr_shm_create argument list. > Neither one sits well. I'd prefer it staying runtime rather than > compile-time. Ideas? How about apr_shm_create_ex()? That would be consistent with many other things in APR, whereby there's a "simple version" that has some (possibly undesirable) default behavior, and an "extra args" version that gives the user-code more control. --Cliff
Re: apr_os_thread_get() weirdness on Win32
On Thu, 5 Sep 2002, Aaron Bannert wrote: > It looks like there's a pointer mismatch, no? It'll work as long Ahh... right you are. Missed that.
Re: apr_os_thread_get() weirdness on Win32
On Fri, 6 Sep 2002, INOUE Seiichiro wrote: > It looks apr_os_thread_get() is wrong on Win32. > The types don't match. I fail to see the problem? apr_os_thread_get returns an apr_os_thread_t, not an apr_thread_t. --Cliff
Re: Bucket management strategies for async MPMs?
On Sat, 31 Aug 2002, Brian Pane wrote: > Wouldn't it be sufficient to guarantee that: > * each *bucket* can only be processed by one thread at a time, and > * allocating/freeing buckets is thread-safe? No. You'd need to also guarantee that all of the buckets sharing a private data structure (copies or splits of a single bucket) were, as a group, processed by only one thread at a time (and those buckets can exist across multiple brigades even). You'd also have to guarantee that no buckets are added/removed from a given brigade by more than one thread at a time. When you add up the implications of all these things, it basically ends up with the whole request being in one thread at a time.
Re: Bucket management strategies for async MPMs?
On Sat, 31 Aug 2002, Brian Pane wrote: > I don't think we can count on the assumption that each conn will > only be processed by one thread at a time. For example, this race Then we have to at least guarantee that each request can only be processed by one thread at a time, I think. *None* of the buckets code is threadsafe, and it's done that way intentionally. A brigade (and its allocator) can exist in exactly one thread at a time. --Cliff
Re: Bucket management strategies for async MPMs?
On Sat, 31 Aug 2002, Brian Pane wrote: > * The bucket allocator alloc/free code isn't > thread-safe, so bad things will happen if the > writer thread tries to free a bucket (that's > just been written to the client) at the same > time that a worker thread is allocating a new > bucket for a subsequent request on the same > connection. We designed with this in mind... basically what's supposed to happen is that rather than having a bucket allocator per thread you have a group of available bucket allocators, and you assign one to each new connection. Since each connection will be processed by at most one thread at a time, you're safe. When the connection is closed, the allocator is placed back into the list of available allocators for reuse on future connections. --Cliff
Re: A question about rings.
On Wed, 28 Aug 2002, Aaron Bannert wrote: > This is *really* good stuff, including the ascii art. We should find > a home for this in the docs. :) Tony already committed it to apr_ring.h. :) Thanks Tony! --Cliff
Re: A question about rings.
On Tue, 27 Aug 2002, Gregory (Grisha) Trubetskoy wrote: > I guess I am just having a difficult time picturing a scenario where > elements are on multiple rings but must share the same head. I suppose > there is a small gain in not having to allocate a head structure per > ring... You can get into tricky scenarios where you might want interlocking rings or something like that. It would become then important to know which of the sentinels you'd reached. If that makes sense. Anyway, I'm kind of reaching at this point because I have yet to see an actual use case for this. I'm sure there is one out there somewhere. Tony? Maybe you can shed some more light on this. --Cliff
Re: A question about rings.
On Tue, 27 Aug 2002, Gregory (Grisha) Trubetskoy wrote: > In other words, instead of > > #define APR_RING_SENTINEL(hp, elem, link) \ > (struct elem *)((char *)(hp) - APR_OFFSETOF(struct elem, link)) > > Why not: > > #define APR_RING_SENTINEL(hp, elem) \ > (struct elem *)(hp) > > It so happens that the APR_RING_ENTRY is always first in the element > structure, and so the offset is 0 which is why it works (and the macros > above yeld same result). But it seems to me that if the APR_RING_ENTRY > was't first, then the result of APR_RING_SENTINEL could be a pointer to > somewhere before beginning of head, or some place within head but before > APR_RING_HEAD, which could be some arbitrary and not necessarily constant > value. That's in fact exactly the point. The sentinel value is not really to be thought of as a pointer to anything in particular... it's just a magic number. A ring element structure can be set up so that each element can be on more than one ring at a time. In that case, each of the rings attached to any given head needs a unique sentinel value. So while it's true that this is hardly ever used and is optimized away, it's there just in case somebody wants to use it. Make more sense? --Cliff
Re: quick autoconf Q
On Tue, 13 Aug 2002, Justin Erenkrantz wrote: > I'd avoid 2.53 at all costs since it introduces autom4te.cache which > was just a really bad idea. -- justin AFAIK all versions of httpd >= 2.0.36 have been rolled with 2.53 -- we just delete with autom4te.cache with extreme prejudice. :) --Cliff
Re: cvs commit: apr-site versioning.html
On Tue, 13 Aug 2002 [EMAIL PROTECTED] wrote: > I continue to state that APR's time format should stay as it is. If you > want seconds, use time_t. Agreed. > Doesn't that completely ignore Cliff's message early in this thread that > specifically stated that he needed microsecond resolution? As long as there's an apr_time_now_in_microseconds() and I'm allowed to determine intervals somehow (which I'm now doing by subtracting two time values), I could care less what else happens. --Cliff
Re: cvs commit: apr configure.in
On 5 Aug 2002 [EMAIL PROTECTED] wrote: > martin 2002/08/05 02:28:24 > > Modified:.configure.in > Log: > Fix buggy substitution > > Revision ChangesPath > 1.474 +2 -2 apr/configure.in > > Index: configure.in > === > RCS file: /home/cvs/apr/configure.in,v > retrieving revision 1.473 > retrieving revision 1.474 > diff -u -r1.473 -r1.474 > --- configure.in4 Aug 2002 01:52:25 - 1.473 > +++ configure.in5 Aug 2002 09:28:24 - 1.474 > @@ -1142,7 +1142,7 @@ >elif test "$ac_cv_sizeof_off_t" = "$ac_cv_sizeof_long"; then >off_t_fmt='#define APR_OFF_T_FMT "ld"' >elif test "$ac_cv_sizeof_off_t" = "$ac_cv_sizeof_long_long"; then > -off_t_fmt='#define APR_OFF_T_FMT $int64_t_fmt' > +off_t_fmt='#define APR_OFF_T_FMT APR_INT64_T_FMT' >else >off_t_fmt='#error Can not determine the proper size for off_t' >fi Are we quite sure that if sizeof(off_t) != sizeof(long) && sizeof(off_t) == sizeof(long long) then sizeof(long long) == 64? --Cliff
RE: cvs commit: apr/poll/unix poll.c
On Fri, 2 Aug 2002, Ryan Bloom wrote: > Huh? We don't have any logic to tell apr_palloc how to die. Sure we do. You just set an abort function. See line 631 of apr_pools.c. --Cliff
Re: cvs commit: apr/poll/unix poll.c
On 2 Aug 2002 [EMAIL PROTECTED] wrote: > +#ifdef WIN32 > +SOCKET fd; > +#else >int fd; > +#endif That's what apr_os_sock_t is for. Please use it instead of the ifdefs. --Cliff
Re: apr_ring.h in APR and not APR-UTIL?
On Tue, 30 Jul 2002, Aaron Bannert wrote: > Any reason why this is in APR and not just APR-UTIL? It used to be in apr-util. As Justin reported, it was moved over because some APR code wanted to use it. It's small enough (one header file) that nobody seemed to mind. --Cliff
Re: cvs commit: apr-util/buckets apr_buckets_pipe.c apr_buckets_socket.c
On Tue, 16 Jul 2002, Brad Nicholes wrote: > protocol.c is compared against the macro APR_STATUS_IS_EAGAIN(). The > problem for both NetWare and Win32 is that this macros is defined to > test the rv against WSAEWOULDBLOCK rather than EWOULDBLOCK. Since the > call to select() in apr_poll() actually returns EWOULDBLOCK for a pipe, Now *that* makes sense. ++1 for checking for EWOULDBLOCK in APR_STATUS_IS_EAGAIN on those platforms. --Cliff
Re: cvs commit: apr-util/buckets apr_buckets_pipe.c apr_buckets_socket.c
On Tue, 16 Jul 2002, Brad Nicholes wrote: >The problem with the MOD_CGI case is that it *doesn't* call > pipe_bucket_read() with APR_BLOCK_READ. In fact it calls it with > APR_NONBLOCK_READ which is what prompted the modification in the first > place. If your analysis as to the definitions of APR_BLOCK_READ vs > APR_NONBLOCK_READ is correct, then let me dig a little deeper in the > code to figure out exactly why MOD_CGI is ultimately passing an > APR_NONBLOCK_READ (unless somebody else already knows). What I do know > is that if pipe_socket_read() is allowed to return with an EWOULDBLOCK > error code, I end up with a browser response of "Premature end of > script..." when in fact the data is there, just not at the right time. Justin's definition is correct as far as I'm aware. At the risk of stating the obvious: EWOULDBLOCK can only be returned in APR_NONBLOCK_READ mode. Any caller that does a nonblocking read *must* check for EWOULDBLOCK and loop or otherwise handle it. (A tight loop doesn't make any sense, of course... if that's all you're going to do, use APR_BLOCK_READ. APR_NONBLOCK_READ is only any good if you have other stuff to do while waiting on more input.) --Cliff
Re: cvs commit: apr/poll/unix pollacc.c
On Tue, 16 Jul 2002, Cliff Woolley wrote: > Uhh... What Sander said. How is my commit exporting a symbol? It's > *UN*exporting the symbol. :-) Ahh, I see you already responded to this under a different subject line. Looks like both of us need our coffee. :)
Re: cvs commit: apr/poll/unix pollacc.c
On Tue, 16 Jul 2002, William A. Rowe, Jr. wrote: > At 12:25 AM 7/16/2002, [EMAIL PROTECTED] wrote: > >jwoolley2002/07/15 22:25:44 > > > > Modified:poll/unix pollacc.c > > Log: > > Nuke a warning due to a function that should have been static AFAICT: > > > > pollacc.c:78: warning: no previous prototype for `find_poll_sock' > > > > --- pollacc.c 11 Jul 2002 15:32:18 - 1.1 > > +++ pollacc.c 16 Jul 2002 05:25:44 - 1.2 > > @@ -74,7 +74,7 @@ > >return APR_SUCCESS; > >} > > > > -APR_DECLARE(apr_pollfd_t*) find_poll_sock(apr_pollfd_t *aprset, > > apr_socket_t *sock) > > +static apr_pollfd_t *find_poll_sock(apr_pollfd_t *aprset, apr_socket_t > > *sock) > > Ummm... -1. You just exported a symbol without namespace protection ;-) Uhh... What Sander said. How is my commit exporting a symbol? It's *UN*exporting the symbol. :-) --Cliff
warning in pollacc.c
([EMAIL PROTECTED])/x1/home/jwoolley/httpd-2.0$ make >/dev/null pollacc.c:78: warning: no previous prototype for `find_poll_sock'
Re: bogus compiler warnings in macros that take `socket'
On Mon, 15 Jul 2002, Ian Holsman wrote: > I'm doing it now.. I just finished it. Where there's a will there's a way. :) About to commit.
Re: bogus compiler warnings in macros that take `socket'
On Tue, 16 Jul 2002, Cliff Woolley wrote: > Hmm... well if Ian wants it done tonight, I guess I can stay up a bit > longer and take a crack at it. If it compiles, it must work, right? Sorry, scratch that. I forgot I'm on my alternate notebook (left the main one at work and shut it down before I left :-( ) so I have no access to my apache sandboxes or private key. :-] I'll do it first thing in the AM unless somebody beats me to it. --Cliff
Re: bogus compiler warnings in macros that take `socket'
On 15 Jul 2002, Karl Fogel wrote: > > commit it tonight. we're about to do a .40 release in httpd, and > > I'll roll this into it. > > I unfortunately have other fish to fry tonight and won't be able to do > this. Sorry about that... (Maybe you can take care of it, though?) Hmm... well if Ian wants it done tonight, I guess I can stay up a bit longer and take a crack at it. If it compiles, it must work, right? hehe ;-) --Cliff
Re: bogus compiler warnings in macros that take `socket'
On 15 Jul 2002, Karl Fogel wrote: > > thesocket, mysocket, socket_param, something like that. > > Much better solution! I totally didn't think of that. > > Unless anyone objects, I will be committing this sometime in the next > few days. (Unless you want it, Cliff, in which case it's all yours.) Have at it, I'm busy getting ready for SIGGRAPH. :-) --Cliff
Re: bogus compiler warnings in macros that take `socket'
On 15 Jul 2002, Karl Fogel wrote: > #define APR_DECLARE_INHERIT_SET(name) \ > APR_DECLARE(apr_status_t) apr_##name##_inherit_set(apr_##name##_t *name) That's what I figured. :-/ Rather than having a separate parameter to the macro, I think I'd rather see something like this: #define APR_DECLARE_INHERIT_SET(name) \ APR_DECLARE(apr_status_t) apr_##name##_inherit_set( \ apr_##name##_t *the##name) thesocket, mysocket, socket_param, something like that. --Cliff
Re: bogus compiler warnings in macros that take `socket'
On Mon, 15 Jul 2002, Karl Fogel wrote: >APR_DECLARE_INHERIT_SET(socket); >APR_DECLARE_INHERIT_UNSET(socket); > > So it would be fair to call this a compiler bug, in that GCC is giving > overeager warnings. > > Unfortunately, fixing this in GCC might be a lot of work; possibly a > *real lot*, depending on the amount of automated analysis required and > when/where it needs to happen. This is not the kind of warning you would get at the preprocessor stage -- you'll only get this after macro expansion has already occurred. Sounds like a legitimate bug in APR to me. --Cliff
Re: Earth to APR????
On Tue, 16 Jul 2002, David Reid wrote: > BTW, this is getting embarrassing to watch - please can we bring this to a > head somehow? Party pooper. ;)
Re: approximating division by a million Re: Why not POSIX time_t?
On Mon, 15 Jul 2002, Brian Pane wrote: > seconds = (t >> 20) + (t >> 24) > > That probably isn't accurate enough, but you get the basic idea: > sum a couple of t/(2^n) terms to approximate t/100. > > What do you think? Sounds like the right idea. But I'm still not sure this isn't too complicated. ;-]
RE: Why not POSIX time_t?
On Mon, 15 Jul 2002, Ryan Bloom wrote: > Ok, cool. I thought I had forgotten something insanely simple about > basic math. Phew. :-D Programming, calculus, theory of computation, that stuff is easy. Arithmetic and algebra, that's hard. =-] You know you're in trouble when you can't even manage to subtract correctly in your head 90% of the time. Sigh. --Cliff
RE: Why not POSIX time_t?
On Mon, 15 Jul 2002, Cliff Woolley wrote: > realsecs = 22/21 * (realusecs >> 20) + 22/21; > realsecs = 44/21 * (realusecs >> 20); I'm obviously on crack. :) That last line is of course totally wrong and shouldn't have been there. :)
RE: Why not POSIX time_t?
On Mon, 15 Jul 2002, Bill Stoddard wrote: > Cliff demonstrates that there is significant loss of accuracy using just a > shift. I believe (faith? :-) that there is a simple solution to this. Don't > know what it is just now though... More numerical musings: Without compensation: 1-21 seconds come out 1 too low (ie, that range maps to 0-20 seconds) 22-43 seconds come out 2 too low (ie, that range maps to 20-41 seconds) 44-... come out 3 too low, etc. 440 comes out as 419 (21 too low), etc. The actual cutoff point is not precisely on the integral second value, of course. Take the number of microseconds, convert it to hex, and drop the least-significant five digits, and that's the resulting value we're talking about here. So it's something close to: realsecs = ((realusecs >> 20) + (realsecs/22) + 1); Which simplifies out as: realsecs = 22/21 * ((realusecs >> 20) + 1); realsecs = 22/21 * (realusecs >> 20) + 22/21; realsecs = 44/21 * (realusecs >> 20); This is almost certainly imprecise for high numbers of seconds. :) --Cliff
Re: Why not POSIX time_t?
On Mon, 15 Jul 2002, Brian Pane wrote: > (seconds << 20) + microseconds Yeah, but addition and subtraction of the resulting scalars would require just as many carry/underflow checks as a structure would... I mean, that's all that really is: a bitfield. --Cliff
RE: Why not POSIX time_t?
On Mon, 15 Jul 2002, Bill Stoddard wrote: > > 1. gettimeofday > >(fast, no loss of accuracy) > We cannot avoid this, right? Right. > > > 2. 64-bit multiplication to build an apr_time_t > >(slow (on lots of current platforms), no loss of accuracy) > > Do we eliminate this by representing apr_time_t as busec? Yes. Rather than having to do (seconds * 100 + microseconds), you just do ((seconds << 20) | binarymicroseconds). > > 3. 64-bit shifts to get approximate seconds > >(fast, but loss of accuracy) > > If you convert from microseconds to integer seconds (which is what httpd > requires), you loose -resolution- no matter how you do it. If the accuracy > you loose is smaller than the resolution, then what does it matter that you > loose some accuracy? It's not always smaller: 30 seconds = 30,000,000 microseconds 3000 base 10 = 11100 100111 111000 base 2 11100 100111 111000 >> 20 = 11100 base 2 11100 base 2 = 28 base 10 So your approximation of 30 seconds gets turned into 28 seconds. Oops. You'd have to do lots of extra work to make sure you always accounted for the "lost" 48576 microseconds per second. I've been thinking about it all day and have yet to come up with a non-dividing way to do that. > I must have blinked... How does step 2 work with busec? See above. > I think apr_time_t works for Cliff now (Cliff am I mistaken?) and I am > proposing that we not change apr_time_t... So I must be missing something. It works for me as long as apr_time_now() [and apr_time_sub() should we decide we need one for some silly reason] can give me a resolution of at least tenths or hundredths of milliseconds. So yes, apr_time_t works for me now. So would struct timeval, but it would be more of a pain. time_t clearly would *not* work for me. --Cliff
RE: more notes on the apr_time_t issue
On Sun, 14 Jul 2002, William A. Rowe, Jr. wrote: > Drop _t since this is a scalar type and not a structure/opaque? Let's plze not do that. We've managed to be beautifully consistent in APR on that front, let's not backtrack now. > With apr_time... our functions can stay as is. I think I agree with rbb at this point... apr_time_t is the only thing that makes any sense. I'd thought of the utime == unsigned time problem a day or two ago but forgot to mention it. And while apr_time_busec_t is a bit better than apr_utime_t, I agree people will say "wtf is a busec". And it means we'd have to rename all our time functions for no good reason. "apr_time_now" just makes so much sense. "apr_time_t" makes so much sense. That's just got to be it. How to resolve Aaron's veto? The #ifndef NEW_TIME_FMT thing is interesting. :) --Cliff
Re: cvs commit: apr-util/buckets apr_buckets_pipe.c
On 12 Jul 2002 [EMAIL PROTECTED] wrote:
> -apr_file_pipe_timeout_set(p, 0);
> +// Only mess with the timeout if we are in a blocking state
> +// otherwise we are already nonblocking so don't worry about it.
> +if (timeout < 0) {
> +apr_file_pipe_timeout_set(p, 0);
> +}
I'm totally confused by this. So you're saying you want to have a timeout
in nonblocking mode? How is that possible? And why < and not > ?
--Cliff
Re: why apr_size_t?
On Thu, 11 Jul 2002, Roy T. Fielding wrote: > As near as I can tell from looking at the code and cvs logs, the only > reason we have apr_size_t and apr_ssize_t is because win32 wants to > define apr_ssize_t. Is that because win32 doesn't have ssize_t? > Is there a reason why we don't simply define ssize_t on that platform? We've run into problems before doing things like this, well I suppose more with functions than with typenames. But in general, if we're going to export something, IMO it must always be apr_ namespaced. For example, there was a case recently where if strchr() or something was missing, we would just define it. But that would break binary compatibility with later patchlevels of the same OS that suddenly *did* define it, as we'd get duplicate definitions. Obviously the binary-compatibility argument doesn't hold for typenames, but I still think it wise to have everything consistently namespaced. It makes it easier for the programmer too -- if you're using APR, then all your types start with "apr_". It would be annoying to have to remember "oh, APR gives me this one, but there's no apr_ on it. APR gives me this other one but there *is* an apr_ on that." --Cliff
Re: Votes - majority or consensus?
On Thu, 11 Jul 2002, Justin Erenkrantz wrote: > Regarding this apr_time_t 'debate,' are we going to be operating > under majority voting or consensus voting? Questions about the code are always resolved by consensus or lazy consensus (which clearly does not apply here :). Majority is only ever used in managerial issues (as in issues that would fall to the PMC to decide). --Cliff
Re: cvs commit: apr STATUS
On 12 Jul 2002 [EMAIL PROTECTED] wrote: > + [fielding; Cliff says he has a sample app. I still don't know how > + he uses them without making implementation assumptions about > + apr_time_t everywhere (there is no print routine for microsecond > + resolution), but I'll accept the need for microsecond resolution > + in addition to second resolution.] Up until now I've been allowed to make assumptions about apr_time_t everywhere -- namely that they are microseconds and that they are scalars which I can subtract. I'm willing to adjust on that front, as long as I don't lose said precision. --Cliff
Re: cvs commit: apr STATUS
> + others that I haven't checked. In any case, since we use the
> + system's time_t time() function to get the time value everywhere
> + except Win32 w/SYSTEMTIME, we only ever have a resolution of
> + seconds or milliseconds. So, why the hell are we storing usecs?
> + We don't use them. We don't even have display functions for them.
> + We have to do a stupid conversion every time we actually do
> something
> + useful with time in order to support somebody's wet dream of a
> + potentially useful feature? That's crap! This is exactly why
> + I hate portability libraries that aren't based on the demonstrated
> + needs of a specific application.]
Um, Roy? WTF are you talking about?
>From apr/time/unix/time.c:
APR_DECLARE(apr_time_t) apr_time_now(void)
{
struct timeval tv;
gettimeofday(&tv, NULL);
return tv.tv_sec * APR_USEC_PER_SEC + tv.tv_usec;
}
And as for "demonstrated needs," you're thinking too Apache-centric by a
longshot. I very frequently use APR in my research work, which is
graphics-rendering-related (thus, nothing to do with Apache at all). I
*need* microsecond resolution in real time or as close to it as I can get.
Even millisecond resolution usually isn't enough for my timing needs
(think about it -- 60 frames per second means I need timing information at
a very high resolution, since I get a grand total of a whopping 16ms for
each frame, and I have to make decisions on how to spend that time).
Resolution of time only in seconds would do me zero good.
The fact that APR has provided me with a platform-neutral way to get
high-resolution time has been a *huge* boon to my work. While I realize
this is a limited example, the same applies to many other use cases I can
think of.
Stop hand-waving-saying-we're-hand-waving, please. IMO there's definitely
a very valid case for having sub-second time resolution in this
portability library, as getting high-resolution time across platforms is a
*bear* to do yourself.
--Cliff
Re: cvs commit: apr STATUS
On 11 Jul 2002 [EMAIL PROTECTED] wrote: > + -1: aaron [veto for reusing the apr_time_t identifier for a new > use. > +I'm ok with apr_timeval_t.] If we're avoiding apr_time_t because of confusion with the POSIX time_t, it makes no sense whatsoever to use "apr_timeval_t" either, since POSIX also defines "struct timeval" (with a seconds component and a usecs component). --Cliff
Re: [PATCH 2] example binary BUSEC patch for benchmarking only
On Thu, 11 Jul 2002, Greg Marr wrote: > I keep thinking that APR_USEC_PER_SEC should be (1 << 20), or now > (1 << APR_USEC_BITS) instead of the magical constant. I have no way > of verifying with a quick glance that 1048576 is really 2^20. You don't know your powers of 2? Memorize, Greg, Memorize. ;)
RE: [PATCH] example BUSEC patch for benchmarking only
On Wed, 10 Jul 2002, Bill Stoddard wrote: > > It's definitely a valid optimization. I just checked gcc on > > Sparc and it generates a shift rather than a division. But > > I've not looked at the generated code, but profiling indicates that an > additional division is happening, adding an extra 231 instructions. > (xlc_r -O2) Note that even gcc will only do this optimization if -fstrength-reduce is specified (granted, for gcc that optimization is turned on automatically with -O2 and -O3). --Cliff
Re: cvs commit: apr renames_pending
On Wed, 10 Jul 2002, Thom May wrote: > Hrm, think I prefer apr_(u|g)id_ Likewise. --Cliff
Re: Binary borkedness and memchr/strncasecmp
On Mon, 8 Jul 2002, William A. Rowe, Jr. wrote: > Can we please use apr_memchr and apr_strncasecmp? I don't mind > if we #define to the platform fn's, and blow up when downgrading the > clib or moving to an older machine without those functions. +1 --Cliff
Re: next steps for changing apr_time_t?
On Mon, 8 Jul 2002, Tony Finch wrote: > I note that apr_time_t is a bad name in the first place, because POSIX > reserves all names ending in _t for the implementation. Feel free to > ignore this exceedingly irritating naming rule :-) I think we uniformly ignore this rule already. Everything in APR that I know of already has a _t suffix (APR-util doesn't follow this standard consistently -- many things in APR-util, including buckets, XML stuff, and strmatch patterns, leave off the _t). I remember when we had this discussion last, and as I recall, everybody agreed it was a stupid rule. :-] --Cliff
RE: proposal to add apr_check_dir_empty() to APR
On Tue, 2 Jul 2002, Ryan Bloom wrote: > P.S. I am getting a say in the child's name, just not as big a say as > Kelly. :-) As long as you don't name him/her "void" or "apr_bloom_child" or something, you'll be fine. ;-) --Cliff
Re: proposal to add apr_check_dir_empty() to APR
On Tue, 2 Jul 2002, Aaron Bannert wrote: > Non-APR functions that want to use APR's apr_status_t codes don't have > the luxury of being able to add another code, so given the performance > tradeoff maybe this is the way to go... Well, they *can*, but APR doesn't know what to do with them. That's not a problem in general -- apr-util does it for example. The part that strikes me as silly is having this massive explosion of status codes when all we're trying to return is "true" or "false". Why not just provide for that in APR, leaving 3rd party status codes for things that REALLY need them (actual unusual conditions, errors, statuses, whatever). Why make people add their own "APR_FALSE"? --Cliff
Re: proposal to add apr_check_dir_empty() to APR
On Tue, 2 Jul 2002, Aaron Bannert wrote: > Adding another apr_status_t code would be ok. Maybe APR_FAILURE to > complement APR_SUCCESS, or we could go all out and add APR_TRUE > and APR_FALSE... I was thinking about that. But then APR_FALSE == APR_SUCCESS for the normal semantics to work out. Does APR_TRUE != APR_SUCCESS make sense? I'm not sure. With APR_FAILURE, it would seem to me as easy target for abuse in the same way that APR_EGENERAL has been since it was created. Originally it was very tightly scoped in meaning, but since then it's been used for all kinds of things other than what was originally intended. My suggestion was going to be to just add a parameter that is an int* and return true/false that way. If false is returned, you can check the apr_status_t to see what happened. Or maybe it should be the other way around, with int returned and an apr_status_t* in the parameter list. Whatever. --Cliff
Re: [PATCH] FreeBSD sendfile [was: new httpd build running on daedalus]
On Tue, 2 Jul 2002, Greg Ames wrote: > This patch tries to maintain binary compatibility on FreeBSD 4.4 and > above by doing a runtime kernel version test. It works on daedalus. > Jeff said he'd try it on a 3.x FreeBSD box, where the runtime test I'm > using isn't available. +1 (untested). For anybody interested in the kernel patch that killed us on this one, the full URL is: http://www.freebsd.org/cgi/cvsweb.cgi/src/sys/kern/uipc_syscalls.c.diff?r1=1.65.2.9&r2=1.65.2.10 --Cliff
Re: cvs commit: apr-util/buckets apr_brigade.c
On 2 Jul 2002 [EMAIL PROTECTED] wrote: > wrowe 2002/07/02 11:20:57 > > Modified:buckets apr_brigade.c > Log: > New emit on win32. Since this is a heap bucket, we are always dealing > in start offsets that fit in apr_size_t. This doesn't need the file > sized apr_off_t resolution. > > -remaining = h->alloc_len - (e->length + e->start); > +/* HEAP bucket start offsets are always in-memory, safe to cast */ > +remaining = h->alloc_len - (e->length + (apr_size_t)e->start); >buf = h->base + e->start + e->length; >} Okay, so why on earth would the remaining= line need that cast when the buf= line doesn't? Just curious. --Cliff
Re: interval times Re: watch out for thread_cond_timedwait problems
On Tue, 2 Jul 2002, Aaron Bannert wrote: > I'm not really comfortable doing a change like this without changing > the name for apr_time_t. If we are going to change the semantics > we need to create a new identifier. AFAIK, changing the name has been part of the plan all along. I was under the impression that Brian was just doing the preliminary legwork. That said, apr_time_t (whatever it ends up being called) being binary usecs means that apr_interval_time_t (whatever it ends up being called) *must* also be binary usecs. apr_interval_time_t is defined as "the difference between two apr_time_t's". If somebody wants to pass a value to a syscall, that's not the purpose of apr_interval_time_t. Besides, I'd think those values would often be in seconds and not microseconds anyway, no? --Cliff
Re: cvs commit: apr/test testatomic.c
On 2 Jul 2002 [EMAIL PROTECTED] wrote: > wrowe 2002/07/02 11:18:52 > > Modified:test testatomic.c > Log: > We shouldn't presume any specific int sizes here, no? > > +apr_atomic_t oldval; > +apr_atomic_t casval = 0; This will fix some things and break others. apr_atomic_t is not necessarily a scalar -- on some platforms it's a struct. In other parts of testatomic there are printf()'s on oldval and casval I believe (bad! bad!). --Cliff
Re: [PATCH] apr_file_setaside
On Sat, 29 Jun 2002, Brian Pane wrote: > #define apr_mmap_setaside(new, old, pool) apr_mmap_dup(new, old, pool, 1); > /* the '1' at the end is the "transfer ownership" flag +1, sounds good. --Cliff
Re: [PATCH] apr_file_setaside
On Sat, 29 Jun 2002, Brian Pane wrote: > Which dup() do you mean: the dup(2) or the naming convention of > using "_dup*()" for this family of apr_file_t functions? Sorry, I should have been more precise. apr_file_dup() was what I was referring to. On second thought though, I suppose I wouldn't object to apr_file_setaside() if we changed apr_mmap_dup() to be apr_mmap_setaside() as well... would that make sense? I'm just looking for consistency here, not objecting to the concept. --Cliff
Re: [PATCH] apr_file_setaside
On Sat, 29 Jun 2002, Brian Pane wrote: > This patch adds an apr_file_setaside() function that moves > an apr_file_t from one pool to another without doing any > dup(2) or mmap(2) operations. Why not keep the dup() and add in a "transfer ownership" feature like the mmap dup uses? After all, the whole reason we added an apr_mmap_dup() was to allow the apr_mmap_t to be moved from one pool to another. It would be mildly annoying to have the mmap and file code diverge again... --cliff
Re: [PATCH] Re: 2.0 performance Re: Breaking something? Now is the time?
On Sat, 29 Jun 2002, Cliff Woolley wrote: > some way that would allow us to coalesce the writes. Alignment issues would kill us here, aren't they? That sucks. G.
Re: [PATCH] Re: 2.0 performance Re: Breaking something? Now is the time?
On Sat, 29 Jun 2002, Cliff Woolley wrote: > Also, isn't it true that your patch now causes the buffer bucket to always > have 0-7 unused bytes at the end? Oh duh, nevermind on this point, my fault. I misread the loop condition. --Cliff
Re: [PATCH] Re: 2.0 performance Re: Breaking something? Now is the time?
On Sat, 29 Jun 2002, Brian Pane wrote: > I tried this, and it didn't unroll the loop. That's probably > because some of information needed to unroll the loop effectively > is unknown to the compiler. Hm. Okay, well, if we're going to do this, can we split it out into a separate macro (my_strncpy or something) so it's clear what's going on and to avoid cluttering up that function? Also, isn't it true that your patch now causes the buffer bucket to always have 0-7 unused bytes at the end? I'd have to go back and look more carefully to be sure, but that was the impression I got from first glance. I also feel like there *has* to be some better way to check for EOS... some way that would allow us to coalesce the writes. But I haven't figured out what that is yet. I'll keep thinking about it. --Cliff
Re: [PATCH] Re: 2.0 performance Re: Breaking something? Now is the time?
On Fri, 28 Jun 2002, Brian Pane wrote: > I remembered why memcpy won't help here: we don't know the > length in advance. But I managed to speed up apr_brigade_puts() > by about 30% in my tests by optimizing its main loop. Does this > patch reduce the apr_brigade_puts() overhead in your test environment? Why won't the compiler unroll this loop for you? gcc -O3 -funroll-loops --Cliff
Re: A common method for determining system utilization
On Fri, 28 Jun 2002, Ian Holsman wrote: > do you think that it is worthwhile having a APR function to determine > system load ? That could be nice.
Re: bug in apr_brigade_write?
On Fri, 28 Jun 2002, Brian Pane wrote: > remaining = h->alloc_len - e->length; > buf = h->base + e->start + e->length; > > Shouldn't that be "remaining = h->alloc_len - (e->length + e->start)"? Ouch, quite likely. Will look into it. --Cliff
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: 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: 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, 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, 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, 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
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
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
Re: [PATCH] Regarding semun definition in proc_mutex.h
On 27 Jun 2002, Jeff Trawick wrote:
> So I guess sizeof(long) is 8 bytes on 64-bit HP-UX? sizeof(long) is
> still 4 on 64-bit AIX.
$ uname -a
HP-UX nova126 B.11.00 U 9000/785 2010844465 unlimited-user license
$ cat test.c
#include
int main()
{
printf("%d\n",sizeof(long));
}
$ ./test
4
$ uname -a
HP-UX nova189 B.11.20 U ia64 2252765325 unlimited-user license
$ cat test.c
#include
int main()
{
printf("%d\n",sizeof(long));
}
$ ./test
4
So, nope. Or maybe you have to compile in 64-bit mode? How does one do
that with HP-UX's cc?
--Cliff
Re: your mail
On Tue, 18 Jun 2002, it was written: > the attached patch may not be correct. sorry. Well, just keep me posted. Thx, Cliff
Re: random number generator
Huh, that's odd. I swear I tested this. I'll look into it tomorrow and place this in patches/apply_to_2.0.39/ if need be. Thanks, Cliff On Wed, 19 Jun 2002 itojun@iijlab.net wrote: > regarding to random number device in apr - thanks for > --with-randomdev=foo configure option in 2.0.39. it seems to me that > there's a bug in the handling. without the patch, > DEV_RANDOM will end up defined as $apr_randomdev (verbatim). > (i requested bug database account but it does not seem to send out > password for me...) > > itojun > > > --- srclib/apr/configure.in- Wed Jun 19 08:23:52 2002 > +++ srclib/apr/configure.in Wed Jun 19 08:32:09 2002 > @@ -1561,8 +1561,8 @@ > fi >elif test "$apr_devrandom" != "no"; then > if test -r "$apr_devrandom"; then > - AC_DEFINE(DEV_RANDOM, [$apr_devrandom]) > + AC_DEFINE_UNQUOTED(DEV_RANDOM, "$apr_devrandom") >AC_MSG_RESULT($apr_devrandom) >rand="1" > else >AC_ERROR([$apr_devrandom not found or unreadable.]) >
Re: random number generator
On Mon, 17 Jun 2002, hiroyuki hanai wrote: > Ben Laurie wrote: > > > you can specify with a ./configure argument which one you want. It will > > > be part of 2.0.38. > > > > I still say it should be runtime configurable. > > I strongly agree with Ben. Well, so do I. But somebody's got to sit down and code the thing. :) What we have now is better than what we had before, but that's not to say it couldn't be better still. --Cliff
Re: random number generator
On Sun, 16 Jun 2002 itojun@iijlab.net wrote: > on unix platforms, apr shipped with httpd 2.0.36 asks for truely- > random number (/dev/random) instead of so-so random number > (/dev/urandom). question: is it really necessary to require > /dev/random instead of /dev/urandom? if not, does the following patch > make sense? We already had a big debate about this. We decided it's best not to pick the least secure option by default, so the patch below (which I already wrote :), isn't the way to go. Instead, I overhauled that whole system so you can specify with a ./configure argument which one you want. It will be part of 2.0.38. --Cliff
Re: cvs commit: apr/include apr_time.h
On Wed, 12 Jun 2002, William A. Rowe, Jr. wrote: > time [(apr_time_t]] Enter > APR_USEC_PER_SEC [(apr_time_t)100] Enter Ah. I missed the fact that APR_USEC_PER_SEC has a built-in cast. Nevermind. --Cliff
Re: cvs commit: apr/include apr_time.h
On Wed, 12 Jun 2002, William A. Rowe, Jr. wrote:
> It seems that we've decided sometime back that all macro-fns should be
> declared in ucase. Now, we can debate that issue again, but if someone
> could pull up a reference to that thread in the archives it would be
> most cool.
Somewhere in [EMAIL PROTECTED] when we talked about APR_BUCKET_DELETE() vs
apr_bucket_delete() or one of its friends there was a brief discussion
about it.
Basically it came down to this: If the macro *could* be a function (by
virtue of no unintended side-effects as you mentioned), then give it a
lowercase name. If side-effects might occur, give it an uppercase name.
(This is all basically what you said.) In practice, if any one of a group
of macro-fn's gets an uppercase name, typically all of its related ones
do, too, for ease-of-remembering.
In this case, I'd say lowercase is fine by majority. For the one
exception you mentioned, we can wrap the macro body with {} and declare a
temporary variable inside that scope in which to store the value passed
to the macro, thereby regaining function-like semantics. There's
precedent in the code for this already...
--Cliff
Re: APR doubts
On Wed, 12 Jun 2002, Miguel Camargo wrote: > First of all,thank you Cliff and Ryan for your answers! No problem. :) > ../apr/include/arch/unix/fileio.h: > > at the begining: > > #if APR_HAVE_NEWFEATURE_H > #include > #endif Right. > APR code... > > #if APR_HAS_NEWFEATURE ? /* or also APR_HAVE_NEWFEATURE_H */ > my code > #endif APR_HAS_NEWFEATURE is fine, assuming NEWFEATURE is some "broad feature" and not just a single function call or something. You can set up configure.in to only define APR_HAS_NEWFEATURE to 1 if all of the functions and headers and stuff that it would need are actually present, meaning that you don't need to test APR_HAVE_FUNCTION1 && APR_HAVE_FUNCTION2 && .. && APR_HAS_NEWFEATURE, since APR_HAS_NEWFEATURE should already only be true if the others are all true. Which means the other macros don't even really need to exist if you don't want them to. All you should need is APR_HAVE_NEWFEATURE_H and APR_HAS_NEWFEATURE. --Cliff
Re: cvs commit: apr/include apr_time.h
On 12 Jun 2002 [EMAIL PROTECTED] wrote: > +#define APR_TIME_USEC(time) ((apr_int32_t)(time) % APR_USEC_PER_SEC) > + > +#define APR_TIME_SEC(time) ((apr_int64_t)(time) / APR_USEC_PER_SEC) > + > +#define APR_TIME_FROM_SEC(sec) ((apr_time_t)(sec) * APR_USEC_PER_SEC) > + > +#define APR_TIME_MAKE(sec, usec) ((apr_time_t)(sec) * APR_USEC_PER_SEC + > usec) Aren't these all missing a set of parens? In other words, I think the cast should occur after the division/multiplication. Such as: #define APR_TIME_USEC(time) ((apr_int32_t)((time) % APR_USEC_PER_SEC)) etc. No? --Cliff
APR_HAVE_ vs APR_HAS_ (was Re: APR doubts)
On Wed, 12 Jun 2002, Cliff Woolley wrote: > There are a *few* cases where we use HAVE where we should be using HAS, > and those ought to be cleaned up. But we're mostly consistent. :) Responding to this message caused me to take a second and actually go look at how consistently we're using this naming convention. We're *very* close to consistent, I think. There are just a few cases where I believe it should be APR_HAS and instead it's APR_HAVE. Here's a stub of a patch that illustrates the ones I think should be changed. Obviously a real patch would be much much bigger, in that configure.in and a million "users" of these macros would need to change. But anyway, what do you think of this? --Cliff Index: apr.h.in === RCS file: /home/cvs/apr/include/apr.h.in,v retrieving revision 1.108 diff -u -d -r1.108 apr.h.in --- apr.h.in7 May 2002 04:12:44 - 1.108 +++ apr.h.in12 Jun 2002 16:59:54 - @@ -65,14 +65,6 @@ #define APR_HAVE_TIME_H @timeh@ #define APR_HAVE_UNISTD_H@unistdh@ -#define APR_HAVE_SHMEM_MMAP_TMP @havemmaptmp@ -#define APR_HAVE_SHMEM_MMAP_SHM @havemmapshm@ -#define APR_HAVE_SHMEM_MMAP_ZERO@havemmapzero@ -#define APR_HAVE_SHMEM_SHMGET_ANON @haveshmgetanon@ -#define APR_HAVE_SHMEM_SHMGET @haveshmget@ -#define APR_HAVE_SHMEM_MMAP_ANON@havemmapanon@ -#define APR_HAVE_SHMEM_BEOS @havebeosarea@ - #define APR_USE_SHMEM_MMAP_TMP @usemmaptmp@ #define APR_USE_SHMEM_MMAP_SHM @usemmapshm@ #define APR_USE_SHMEM_MMAP_ZERO@usemmapzero@ @@ -81,6 +73,14 @@ #define APR_USE_SHMEM_MMAP_ANON@usemmapanon@ #define APR_USE_SHMEM_BEOS @usebeosarea@ +#define APR_HAS_SHMEM_MMAP_TMP @hasmmaptmp@ +#define APR_HAS_SHMEM_MMAP_SHM @hasmmapshm@ +#define APR_HAS_SHMEM_MMAP_ZERO@hasmmapzero@ +#define APR_HAS_SHMEM_SHMGET_ANON @hasshmgetanon@ +#define APR_HAS_SHMEM_SHMGET @hasshmget@ +#define APR_HAS_SHMEM_MMAP_ANON@hasmmapanon@ +#define APR_HAS_SHMEM_BEOS @hasbeosarea@ + #define APR_USE_FLOCK_SERIALIZE @flockser@ #define APR_USE_SYSVSEM_SERIALIZE @sysvser@ #define APR_USE_POSIXSEM_SERIALIZE@posixser@ @@ -97,12 +97,10 @@ #define APR_PROCESS_LOCK_IS_GLOBAL@proclockglobal@ -#define APR_HAVE_CORKABLE_TCP @have_corkable_tcp@ #define APR_HAVE_GETRLIMIT @have_getrlimit@ #define APR_HAVE_IN_ADDR@have_in_addr@ #define APR_HAVE_INET_ADDR @have_inet_addr@ #define APR_HAVE_INET_NETWORK @have_inet_network@ -#define APR_HAVE_IPV6 @have_ipv6@ #define APR_HAVE_MEMMOVE@have_memmove@ #define APR_HAVE_SETRLIMIT @have_setrlimit@ #define APR_HAVE_SIGACTION @have_sigaction@ @@ -131,6 +129,8 @@ #endif /* APR Feature Macros */ +#define APR_HAS_CORKABLE_TCP @has_corkable_tcp@ +#define APR_HAS_IPV6 @has_ipv6@ #define APR_HAS_SHARED_MEMORY @sharedmem@ #define APR_HAS_THREADS @threads@ #define APR_HAS_SENDFILE @sendfile@
Re: APR doubts
On Wed, 12 Jun 2002, Miguel Camargo wrote: > I am trying to modify the APR library just to add some specific features > in it and use it in Apache. What kinds of things, just out of curiosity? > But i have some doubts about the meaning and the differences among the > macro definitions of: The first thing to note is that all of these are always defined -- you check whether their value is 0 or 1 -- as opposed to the HAVE_FOO macros that autoconf provides, which are either defined or undefined. That means the test you should use on all of them is #if and not #ifdef. > APR_HAVE_... These are ones that come straight out of what autoconf provides us, except we've namespace-protected them (and done the 0/1 instead of undef/def thing I mentioned above). So basically this means the system provides some item (typically a header file, a given function, a structure, etc); so APR_HAVE_STDIO_H tells you whether or not stdio.h exists. APR_HAVE_MEMMOVE tells you whether or not the memmove() function is provided by the system, etc. > APR_USE_... These are used where multiple choices are provided by the OS; they tell APR which one to use by default. For example, there might be multiple methods of getting a shared memory segment; one of the APR_USE_SHMEM_* macros will be 1 to tell APR which one to prefer. > APR_HAS_... These are "feature" macros; they tell us that APR has support for some broad feature available (where "broad feature" doesn't map to a single function call or header but rather a whole group of them). For example, APR_HAS_MMAP tells us whether APR's MMAP support is available. APR_HAS_THREADS tells us whether APR was built in multi-threaded mode. And so on. There are a *few* cases where we use HAVE where we should be using HAS, and those ought to be cleaned up. But we're mostly consistent. :) Hope this helps. --Cliff
Re: apr_time_t --> apr_time_usec_t
On Tue, 11 Jun 2002, William A. Rowe, Jr. wrote: > However, we aught to define convenience macros; > #define APR_BUSEC_PER_SEC 1048576 I was thinking the same thing. +1. --Cliff
Re: apr_time_t --> apr_time_usec_t
On Mon, 10 Jun 2002, Aaron Bannert wrote: > On Mon, Jun 10, 2002 at 03:57:29PM -0700, Justin Erenkrantz wrote: > > How about using the value we already have: > > > > typedef apr_int32_t apr_short_interval_time_t; > > Unfortunately, that type still has units of milliseconds. > Seems like Roy needs an "apr_seconds_t", and apr_short_interval_time_t > should be renamed "apr_milliseconds_t". No, MICROseconds. For both of them. apr_time_t: a 64 bit quantity specifying microseconds since the epoch apr_interval_time_t: a 64 bit quantity specifying the microsecond difference between two apr_time_t's. apr_short_interval_time_t: a 32 bit quantity specifying the microsecond difference between two apr_time_t's that are very close together (what is it, like 36 minutes max?) If you want some of those to be seconds or some alternative ones to be seconds, fine, but let's be precise. :)
Re: apr_time_t --> apr_time_usec_t
On Mon, 10 Jun 2002, Roy T. Fielding wrote: > I know of one existing bug in httpd that I would consider a > showstopper, if I were RM, due to the way APR handles time. Are you going to tell me what it is? :)
Re: [APR PATCH] Bootstrapping problem on Mac OS X
On Mon, 10 Jun 2002, Pier Fumagalli wrote: > Now, the question is _why_ that thing is called and why libsvn_ra_local.so > does not exist (if shared is disabled, it should be compiled in, right?)... I was under the impression that it was one of those things of SVN attempting to load all extensions (with no preconceived notions of which ones exist and which ones don't). Right? --Cliff
apr_bucket_poll
[Moved here from [EMAIL PROTECTED] On Thu, 6 Jun 2002, Justin Erenkrantz wrote: > On Thu, Jun 06, 2002 at 04:51:10PM -0700, Aaron Bannert wrote: > > I'd *love* to see progress on something like this, but whenver I start > > thinking about this kind of a MPM I run into the problem where brigades > > (or filter chains) can't be multiplexed over. Is this what you have > > in mind? (Let's get a discussion going.) > > Haven't we talked about adding an apr_bucket_poll function? I > don't think I saw any objections to that the previous two times > we've discussed it, so I guess it's waiting for someone to come > along and do it. -- justin I've objected to it off and on ... in particular forms. I just don't see what good apr_bucket_poll would be if you can only poll one bucket, which is what you'd have to do to be consistent with the other apr_bucket_foo() functions' semantics. Polling one bucket at a time is no different than doing a nonblocking read on each one in a tight loop. It's the tight loop obviously that's the problem in either case. So what we really would need is some way to add a bucket (or a brigade) to a "pollset" of sorts. In either case it means that we would need a way to add individual buckets to a pollset. But that kind of depends on having poll() or select() or whatever being able to interact with all of the kinds of buckets from which reads might block, and that might not be possible. I'm kind of just thinking out loud here, but I do think this is easier said than done... --Cliff
Re: New apr_os_file_put flags value APR_FILE_PIPECHECK?
On Mon, 3 Jun 2002, William A. Rowe, Jr. wrote: > Would it make sense to introduce APR_FILE_PIPECHECK to ask > apr_os_file_put() to first check an apr_file_info_get() of the handle, and > set the flags correctly if this is a pipe device (e.g. pass this flag for > apr_file_open_stdin/out/err when those are pipes?) I'm of two minds on this. On one hand, it makes sense, but at the same time, "PIPECHECK" seems to be a bit more information about the internal implementation than usual. Conversely I suppose we could go with a flag to say "this is definitely a file," but then again most of the callers to os_file_put are probably already right, so it makes sense to have the burden of the flag only on the infrequent case (the one where it ISN'T already known to be a file). Hmm... --Cliff
