On Sat, Sep 17, 2016 at 1:30 PM, NormW <[email protected]> wrote:
> Hi,
> Having little to watch on TV, have played about with httpd-trunk and
> apr-1.6, to see where the hiccups are hiding... What follows are the issues
> found, and in most cases, fairly simple solutions.
Thanks Norm, applied (mostly, see below).
>
>> D:\Projects\svn\httpd-trunk>svn diff
>> Index: modules/filters/mod_crypto.c
>> ===================================================================
>> --- modules/filters/mod_crypto.c (revision 1760988)
>> +++ modules/filters/mod_crypto.c (working copy)
>> @@ -116,6 +116,7 @@
>> {
>> apr_status_t rv;
>> char ps = *arg;
>> + const char *name;
>>
>> if ('f' == ps && !strncmp(arg, "file:", 5)) {
>> arg += 5;
>> @@ -124,7 +125,7 @@
>> return apr_pstrcat(cmd->pool, "No filename specified", NULL);
>> }
>>
>> - const char *name = ap_server_root_relative(cmd->temp_pool, arg);
>> + name = ap_server_root_relative(cmd->temp_pool, arg);
>> if (name) {
>> apr_file_t *file;
http://svn.apache.org/r1761281
>>
>> Index: modules/http2/NWGNUmod_http2
>> ===================================================================
>> --- modules/http2/NWGNUmod_http2 (revision 1760988)
>> +++ modules/http2/NWGNUmod_http2 (working copy)
>> @@ -369,7 +369,7 @@
>> @echo $(DL) h2_proxy_res_ignore_header,$(DL) >> $@
>> @echo $(DL) h2_headers_add_h1,$(DL) >> $@
>> @echo $(DL) h2_req_create,$(DL) >> $@
>> - @echo $(DL) h2_req_createn,$(DL) >> $@
>> +# @echo $(DL) h2_req_createn,$(DL) >> $@
>> @echo $(DL) h2_util_camel_case_header,$(DL) >> $@
>> @echo $(DL) h2_util_frame_print,$(DL) >> $@
>> @echo $(DL) h2_util_ngheader_make_req,$(DL) >> $@
>
>
> These are fairly obvious but subject to what vintage compiler you prefer and
> what should be done about symbols that move about... For ease of review I've
> just commented out the offending symbol.
The h2_req_createn removal/move seems disputed (not sure why since
this is a "private" function, even though its shared..), so I didn't
apply this one...
Stefan, something to be done about this?
>
> Over in APR-1.6...
>
>> D:\Projects\svn\apr-1.6.x>svn diff
>> Index: build/NWGNUmakefile
>> ===================================================================
>> --- build/NWGNUmakefile (revision 1760898)
>> +++ build/NWGNUmakefile (working copy)
>> @@ -22,7 +22,7 @@
>> $(APU)/include/apr_ldap.h \
>> $(APU)/include/private/apu_config.h \
>> $(APU)/include/private/apu_select_dbm.h \
>> - $(APUXML)/expat/lib/expat_config.h \
>> + $(APUXML)/expat_config.h \
>> $(APR)/include/private/apr_escape_test_char.h \
>> $(EOLIST)
First commited in http://svn.apache.org/r1761283, but finally reverted
(r1761292), trunk's xml/NWGNUmakefile seems to reference
$(EXPATSRC)/lib for expat_config.h.
So I'm not sure here, do we need a change in trunk too?
>>
>> Index: locks/netware/proc_mutex.c
>> ===================================================================
>> --- locks/netware/proc_mutex.c (revision 1760898)
>> +++ locks/netware/proc_mutex.c (working copy)
>> @@ -72,7 +72,7 @@
>> return APR_ENOLOCK;
>> }
>>
>> -APR_DECLARE(apr_status_t) apr_proc_mutex_timedlock(apr_thread_mutex_t
>> *mutex,
>> +APR_DECLARE(apr_status_t) apr_proc_mutex_timedlock(apr_proc_mutex_t
>> *mutex,
>> apr_time_t timeout,
>> int absolute)
>> {
>> Index: locks/netware/thread_mutex.c
>> ===================================================================
>> --- locks/netware/thread_mutex.c (revision 1760898)
>> +++ locks/netware/thread_mutex.c (working copy)
>> @@ -55,7 +55,7 @@
>>
>> if (flags & APR_THREAD_MUTEX_TIMED) {
>> apr_status_t rv = apr_thread_cond_create(&new_mutex->cond, pool);
>> - if (rv != SUCCESS) {
>> + if (rv != APR_SUCCESS) {
>> NXMutexFree(new_mutex->mutex);
>> return rv;
>> }
>> @@ -121,7 +121,7 @@
>> if (mutex->locked) {
>> mutex->num_waiters++;
>> if (timeout < 0) {
>> - rv = apr_thread_cond_dwait(mutex->cond, mutex);
>> + rv = apr_thread_cond_wait(mutex->cond, mutex);
>> }
>> else {
>> if (absolute) {
>> Index: locks/netware/thread_cond.c
>> ===================================================================
>> --- locks/netware/thread_cond.c (revision 1760898)
>> +++ locks/netware/thread_cond.c (working copy)
>> @@ -73,7 +73,7 @@
>> rc = NXCondWait(cond->cond, mutex->mutex);
>> }
>> else {
>> - timeout = timeout * 1000 / XGetSystemTick();
>> + timeout = timeout * 1000 / NXGetSystemTick();
>> rc = NXCondTimedWait(cond->cond, mutex->mutex, timeout);
>> if (rc == NX_ETIMEDOUT) {
>> return APR_TIMEUP;
Applied in http://svn.apache.org/r1761279 (trunk had the same issue),
and backported to 1.6.x (r1761280).
>> Index: NWGNUmakefile
>> ===================================================================
>> --- NWGNUmakefile (revision 1760898)
>> +++ NWGNUmakefile (working copy)
>> @@ -341,6 +341,7 @@
>> $(OBJDIR)/pollcb.o \
>> $(OBJDIR)/pollset.o \
>> $(OBJDIR)/select.o \
>> + $(OBJDIR)/wakeup.o \
>> $(OBJDIR)/sendrecv.o \
>> $(OBJDIR)/sha2.o \
>> $(OBJDIR)/sha2_glue.o \
http://svn.apache.org/r1761287
>
>
> Mostly these are typo's in a non-released package so don't count as a
> critique; the major issue comes with .\include\apr_proc_mutex.h:
Hard to propagate changes to every supported platforms w/o being able
to (at least) compile check on all of them, sorry about the
locks/netware ones (my bad)...
Did/Can you try "make check" (or alike) on Netware regarding these APR changes?
>
>> APR_DECLARE(apr_status_t) apr_proc_mutex_timedlock(apr_proc_mutex_t
>> *mutex,
>> apr_time_t timeout,
>> int absolute);
>
> The NetWare proc_mutex.c requires it defined as:
>>
>> APR_DECLARE(apr_status_t) apr_proc_mutex_timedlock(apr_thread_mutex_t
>> *mutex,
>> apr_time_t timeout,
>> int absolute)
>
> I've just temporarily renamed it to see if it would compile and it did; how
> to solve this I leave to experts.
That's the good fix I think, bad copy/paste on my side.
>
>
> APR-UTIL-1.6 shows:
>>
>> D:\Projects\svn\apr-util-1.6.x>svn diff
>> Index: include/apu.hnw
>> ===================================================================
>> --- include/apu.hnw (revision 1760898)
>> +++ include/apu.hnw (working copy)
>> @@ -108,7 +108,7 @@
>> #define APU_HAVE_ODBC 0
>> #endif
>>
>> -#define APU_HAVE_CRYPTO 0
>> +#define APU_HAVE_CRYPTO 1
>>
>> #ifndef APU_DSO_MODULE_BUILD
>> #define APU_HAVE_OPENSSL 0
If it works on Netware I guess we can enable it (not done yet).
Does "make check" work here too (and/or tests framework for mod_crypto
in httpd, if any)?
>> Index: xml/NWGNUmakefile
>> ===================================================================
>> --- xml/NWGNUmakefile (revision 1760898)
>> +++ xml/NWGNUmakefile (working copy)
>> @@ -30,7 +30,7 @@
>> $(APU)/uri \
>> $(APU)/dbm/sdbm \
>> $(APU)/include/private \
>> - $(APUXML)/expat/lib \
>> + $(EXPATSRC)/lib \
>> $(EOLIST)
>>
>> #
>> @@ -248,7 +248,7 @@
>> # Any specialized rules here
>> #
>>
>> -vpath %.c expat/lib
>> +vpath %.c $(EXPATSRC)/lib
Same as above (related at least), I'd like to clarify trunk vs
apu-1.6.x/apu-1.5.x w.r.t. expat before applying this (though this
particular change looks good).
Maybe someone else could help with expat's build?
Anyway, thanks again for the patches Norm.
Regards,
Yann.
