On Sat, Sep 17, 2016 at 1:30 PM, NormW <no...@gknw.net> 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.

Reply via email to