++1 on the readability factor (non-binding vote :) I'm also very happy with the flexibility this will give us.
-aaron > Note that one or two bugs were fixed in the PROC_PTHREAD_SERIALIZE > support, but I wouldn't bet on it working... maybe we've never > tried this with APR, or at least not in a long time? > > 1) bugs: > > a) APR_PROCESS_LOCK_IS_GLOBAL and the idea of selecting the > mechanism at run-time doesn't seem safe... need something like > APR_FCNTL_LOCK_IS_GLOBAL to make it work correctly > > b) create_lock() makes a copy of the filename parm even if not > needed for the desired mechanism > > c) apr_os_lock_get/apr_os_lock_put/apr_os_lock_t > > I haven't attempted to straighten out the definition of > apr_os_lock_t. We'll need to use separate fields for > the "int crossproc" mechanisms vs. the "pthread_mutex_t > *crossproc" mechanism. > > I dunno what else... I haven't tested it yet :) > > 2) misfeatures: > > APR_HAVE_xxx_SERIALIZE is not in apr.h > > maybe the pthread rw lock stuff needs to use the method ptrs > too... we could add a acquire_read and acquire_write field... > less code specific to rw lock would then be needed > > 3) minimal further work needed to allow lock mechanism selection: > > add appropriate parm to apr_lock_create(), replace my logic in > locks.c:create_lock() with logic to look at parameter and > APR_HAVE_* defines rather than APR_USE_* defines > > Any reason I shouldn't proceed with testing/committing? This is > enough function to allow someone else to carry it further, and > meanwhile we can see what build tweaks are needed on various Unix > flavors, as this will start to compile more code into APR.
