Re: Remaining breakage on apr-1.7.x branch

2022-10-17 Thread William A Rowe Jr
On Tue, Sep 13, 2022 at 12:21 PM Ivan Zhakov  wrote:
>
> Btw I've recently backported several fixes to 1.7.x branch:
> [[[
>   *) Fix attempt to free invalid memory on exit when apr_app is used
>  on Windows. [Ivan Zhakov]
>   *) Fix double free on exit when apr_app is used on Windows. [Ivan Zhakov]
>
>   *) Fix a regression in apr_stat() for root path on Windows. [Ivan Zhakov]
> ]]]
>
> And CMake/GitHub Actions stuff.

Thanks for all of these! I'll proceed in the morning to tag 1.7.1, and
then will be
happy to mentor someone to tag 1.8.0 once we trust that this breaks nothing in
terms of minor versioning rev rules, and that the new features have maintainers.


Re: Remaining breakage on apr-1.7.x branch

2022-09-12 Thread William A Rowe Jr
After deep study of the whole ring strict alias issue, I've come to
the conclusion that
the -resulting- binary is effectively the same, and my test case was
swapping the
apr .so files between builds of httpd 2.4.54 / apr 1.7.0 and httpd
2.4.x/ apr 1.7.x.
Running httpd test, no errors were introduced, all binaries loaded and ran fine.
Someone building Subversion might want to repeat that exercise.

So this was entirely a work-around to ensure the grammar didn't escape notice
when types and values were switched around in ways that a compiler might
over-optimize. There were instances where -O3 could break apr at one point,
and I'm wondering if this might have been that root cause?

I think we can accept this very unusual (for a stable release branch)
contortion,
since code correctly compiled on either 1.7.0 or 1.7.branch appear to accomplish
the same and are binary-stable, only using different syntaxes to get there.

Any final thoughts before we simply release 1.7 branch as-is?

Bill


On Wed, Sep 7, 2022 at 11:06 AM Eric Covener  wrote:
>
> On Thu, Jul 28, 2022 at 12:59 PM William A Rowe Jr  
> wrote:
> >
> > Hello friends and collaborators,
> >
> > I'm still rather certain this change is ABI and API contract-breaking
> > on the apr-1.7.x branch, which holds the project hostage for providing
> > a well-understood security fix on the legacy branch. Such things
> > can't persist, our individual maintenance branches must also observe
> > semver, such that anyone could consume a maintenance branch and
> > not be API nor ABI broken against the prior sub-version release;
> >
> > https://github.com/apache/apr/compare/1.7.0...1.7.x#diff-86c28ef8e95257a8d60ce73f3262290e91fc5ca3eb722144a6c99559ddf717cf
> >
> > I'd propose a radical solution, either all of include/* excluding most
> > of private / arch/* have no adverse effects on a rebuild, or we revert
> > the whole branch to the last stable release, in 7 days. Allow a 7 day
> > window to reintroduce critical and desired changes that don't break
> > our dev guidelines;
> >
> > https://apr.apache.org/versioning.html.
> >
> > All other questions were resolved at least 4 weeks ago, as can
> > be observed from include/* changes which alter the incremental
> > consumer's observations, github makes this simple;
> >
> > https://github.com/apache/apr/compare/1.7.0...1.7.x#
> >
> > So I think we are nearly ready 14 days from now to tag 1.7.1
> > and solve this conundrum. If it can be proven than the existing
> > 1.7.0 or 1.7.1 builds would be entirely compatible and not break
> > developer's expectations when deployed against either 1.7.1
> > or 1.7.0 in the next 7 days, my objection is withdrawn and we
> > will tag this code in one week, not waiting for 2 weeks.
> >
> > Who wants to help prove up or down that it should persist and
> > work out the back-out as needed? Otherwise I ask the project
> > member's permissions to work from a new 1.7.x-rebuild branch
> > for the following week based on 1.7.0, and replace 1.7.x with
> > the 1.7.x-rebuild branch one week later.
>
> Should we just back out r1896748 (ring strict aliasing thing) out at
> this stage?  I think the radical solution is too labor intensive and
> unlikely to progress.


Re: Remaining breakage on apr-1.7.x branch

2022-07-28 Thread William A Rowe Jr
The links below didn't resolve as I expected them to, pay specific attention to

   include/apr_ring.h

delta between 1.7.0 tag and 1.7.x branch. I'll continue to try to coax github
to make an appropriate representation by following a link.

On Thu, Jul 28, 2022 at 11:58 AM William A Rowe Jr  wrote:
>
> Hello friends and collaborators,
>
> I'm still rather certain this change is ABI and API contract-breaking
> on the apr-1.7.x branch, which holds the project hostage for providing
> a well-understood security fix on the legacy branch. Such things
> can't persist, our individual maintenance branches must also observe
> semver, such that anyone could consume a maintenance branch and
> not be API nor ABI broken against the prior sub-version release;
>
> https://github.com/apache/apr/compare/1.7.0...1.7.x#diff-86c28ef8e95257a8d60ce73f3262290e91fc5ca3eb722144a6c99559ddf717cf
>
> I'd propose a radical solution, either all of include/* excluding most
> of private / arch/* have no adverse effects on a rebuild, or we revert
> the whole branch to the last stable release, in 7 days. Allow a 7 day
> window to reintroduce critical and desired changes that don't break
> our dev guidelines;
>
> https://apr.apache.org/versioning.html.
>
> All other questions were resolved at least 4 weeks ago, as can
> be observed from include/* changes which alter the incremental
> consumer's observations, github makes this simple;
>
> https://github.com/apache/apr/compare/1.7.0...1.7.x#
>
> So I think we are nearly ready 14 days from now to tag 1.7.1
> and solve this conundrum. If it can be proven than the existing
> 1.7.0 or 1.7.1 builds would be entirely compatible and not break
> developer's expectations when deployed against either 1.7.1
> or 1.7.0 in the next 7 days, my objection is withdrawn and we
> will tag this code in one week, not waiting for 2 weeks.
>
> Who wants to help prove up or down that it should persist and
> work out the back-out as needed? Otherwise I ask the project
> member's permissions to work from a new 1.7.x-rebuild branch
> for the following week based on 1.7.0, and replace 1.7.x with
> the 1.7.x-rebuild branch one week later.
>
> I hope none of us are looking to land in this quicksand again,
>
> Bill


Remaining breakage on apr-1.7.x branch

2022-07-28 Thread William A Rowe Jr
Hello friends and collaborators,

I'm still rather certain this change is ABI and API contract-breaking
on the apr-1.7.x branch, which holds the project hostage for providing
a well-understood security fix on the legacy branch. Such things
can't persist, our individual maintenance branches must also observe
semver, such that anyone could consume a maintenance branch and
not be API nor ABI broken against the prior sub-version release;

https://github.com/apache/apr/compare/1.7.0...1.7.x#diff-86c28ef8e95257a8d60ce73f3262290e91fc5ca3eb722144a6c99559ddf717cf

I'd propose a radical solution, either all of include/* excluding most
of private / arch/* have no adverse effects on a rebuild, or we revert
the whole branch to the last stable release, in 7 days. Allow a 7 day
window to reintroduce critical and desired changes that don't break
our dev guidelines;

https://apr.apache.org/versioning.html.

All other questions were resolved at least 4 weeks ago, as can
be observed from include/* changes which alter the incremental
consumer's observations, github makes this simple;

https://github.com/apache/apr/compare/1.7.0...1.7.x#

So I think we are nearly ready 14 days from now to tag 1.7.1
and solve this conundrum. If it can be proven than the existing
1.7.0 or 1.7.1 builds would be entirely compatible and not break
developer's expectations when deployed against either 1.7.1
or 1.7.0 in the next 7 days, my objection is withdrawn and we
will tag this code in one week, not waiting for 2 weeks.

Who wants to help prove up or down that it should persist and
work out the back-out as needed? Otherwise I ask the project
member's permissions to work from a new 1.7.x-rebuild branch
for the following week based on 1.7.0, and replace 1.7.x with
the 1.7.x-rebuild branch one week later.

I hope none of us are looking to land in this quicksand again,

Bill


Re: svn commit: r1895528 - /apr/apr/trunk/CMakeLists.txt

2022-07-28 Thread William A Rowe Jr
Hi Mladen,

you tweaked this, and failed to update build/find_apr.m4, so this
broke CMake builds of
httpd against APR. httpd simply borrows what apr provides, so it isn't
on that project.

Are you able to update the find macro appropriately so it functions to
discover the true
path of apr, or should this be reverted?

Thanks,

Bill

On Thu, Dec 2, 2021 at 11:19 PM  wrote:
>
> Author: mturk
> Date: Fri Dec  3 05:19:47 2021
> New Revision: 1895528
>
> URL: http://svn.apache.org/viewvc?rev=1895528=rev
> Log:
> Use the correct location for installing apr-2 headers (same layout as on 
> nixes)
>
> Modified:
> apr/apr/trunk/CMakeLists.txt
>
> Modified: apr/apr/trunk/CMakeLists.txt
> URL: 
> http://svn.apache.org/viewvc/apr/apr/trunk/CMakeLists.txt?rev=1895528=1895527=1895528=diff
> ==
> --- apr/apr/trunk/CMakeLists.txt (original)
> +++ apr/apr/trunk/CMakeLists.txt Fri Dec  3 05:19:47 2021
> @@ -17,7 +17,7 @@
>
>  PROJECT(APR C)
>
> -CMAKE_MINIMUM_REQUIRED(VERSION 2.8)
> +CMAKE_MINIMUM_REQUIRED(VERSION 3.0)
>
>  OPTION(APR_MINIMAL_BUILD"Create minimal APR build"   OFF)
>  IF(NOT APR_MINIMAL_BUILD)
> @@ -664,7 +664,7 @@ IF(INSTALL_PDB)
>CONFIGURATIONS RelWithDebInfo Debug)
>  ENDIF()
>
> -INSTALL(FILES ${APR_PUBLIC_HEADERS_STATIC} ${APR_PUBLIC_HEADERS_GENERATED} 
> DESTINATION include)
> +INSTALL(FILES ${APR_PUBLIC_HEADERS_STATIC} ${APR_PUBLIC_HEADERS_GENERATED} 
> DESTINATION include/apr-2)
>  IF(APR_INSTALL_PRIVATE_H)
># Kludges for unexpected dependencies of httpd 2.x, not installed by 
> default
>SET(APR_PRIVATE_H_FOR_HTTPD
> @@ -673,7 +673,7 @@ IF(APR_INSTALL_PRIVATE_H)
>  include/arch/win32/apr_arch_utf8.h
>  include/arch/win32/apr_private.h
>  )
> -  INSTALL(FILES ${APR_PRIVATE_H_FOR_HTTPD} DESTINATION include/arch/win32)
> +  INSTALL(FILES ${APR_PRIVATE_H_FOR_HTTPD} DESTINATION 
> include/apr-2/arch/win32)
>  ENDIF()
>
>  STRING(TOUPPER "${CMAKE_BUILD_TYPE}" buildtype)
>
>


Re: svn commit: r1896510 - in /apr/apr/branches/1.7.x: ./ file_io/win32/ include/arch/unix/ include/arch/win32/ network_io/os2/ poll/os2/ poll/unix/

2022-06-30 Thread William A Rowe Jr
The answer to your question is whether a consumer who built against
apr<1.7.0 is going to blow up, whether they borrowed "private" API's
or not. If they were exported, it's effectively public.

Or, whether a consumer built against 1.7.1 would blow up against 1.7.0
- if that's true, we need to revert.

Those are the last of my obvious observations, I'll need to diff the
resulting include/... tree between 1.7.0 and 1.7.x branch to know for
sure, but thanks everyone for reviewing these questions.

Cheers,

Bill

On Tue, Feb 15, 2022 at 7:24 AM Ivan Zhakov  wrote:
>
> On Wed, 9 Feb 2022 at 13:47, Ivan Zhakov  wrote:
>>
>> On Tue, 8 Feb 2022 at 21:58, Evgeny Kotkov  
>> wrote:
>>>
>>> Ivan Zhakov  writes:
>>>
>>> > This part is now in the following branch:
>>> > https://svn.ostyserver.net/svn/asf/apr/apr/branches/win32-pollset-wakeup-no-file-socket-emulation
>>> >
>>> > What do you think?
>>> >
>>> > It would be great if someone could take a look on the implementation from
>>> > the *nix perspective.
>>> > After that, I propose to merge the branch into trunk.
>>>
>>> In case this helps, I have tested the branch on Ubuntu x64 and it seems
>>> to compile and pass the tests.
>>>
>>>
>>
>> Thanks! Merged branch to trunk on r1897895.
>>
>> Backporting changes to 1.8.x in my TODO list.
>>
> Nominated this change to 1.8.x branch. Please review.
>
> --
> Ivan Zhakov


Re: svn commit: r1897209 - in /apr/apr/branches/1.7.x: ./ include/apr_pools.h memory/unix/apr_pools.c

2022-06-30 Thread William A Rowe Jr
Do we agree you can't change the API, whether it is ABI compatible or
not? Or was there a follow-up commit I missed?

On Mon, Jan 24, 2022 at 11:20 AM William A Rowe Jr  wrote:
>
> On Thu, Jan 20, 2022 at 9:38 AM Yann Ylavic  wrote:
> >
> > On Thu, Jan 20, 2022 at 3:56 PM William A Rowe Jr  
> > wrote:
> > >
> > > On Thu, Jan 20, 2022 at 8:50 AM Eric Covener  wrote:
> > > >
> > > > > Code that will compile on 1.7.1 release
> > > > > still won't compile on 1.7.0, unless I misunderstand something.
> > > >
> > > > Is it a requirement in this direction?
> > > > https://apr.apache.org/versioning.html says "later versions"
> > >
> > > That's why I raise the question, I don't have the authoritative answer.
> > >
> > > Our SVN participants who adopted and recommended strongly that
> > > APR follow the semantic versioning [1] approach so they could -also-
> > > adopt it probably feel more strongly about this.
> > >
> > > "Bug fixes not affecting the API increment the patch version, backwards 
> > > compatible API additions/changes increment the minor version, and 
> > > backwards incompatible API changes increment the major version."
> >
> > It was proposed in another thread to add the APU macros to 1.7.x to
> > ease later migration to 2.x, I don't think that adding these apr_pool_
> > macros can have more compat issues..
>
> I've been looking for follow up... seeing none, I'd point to SemVer 2.0.0 spec
> bullet points 6. and 7. which clarify patch and minor bumps.
>
> Adding the pool macros changed the API, so therefore they cause APR 1.8.0,
> adding apu->apr compatibility macros will change the API, and therefore
> cause APR-util 1.8.0. Even a noop macro introduces an API.
>
> Are the authors willing to revert all manner of version breakage so we can 
> jump
> forward first with the patch release tag, and then the 1.8.0 (even an
> -rc1) tags?


Re: svn commit: r1897222 - in /apr/apr/branches/1.7.x: ./ file_io/win32/ include/arch/unix/ include/arch/win32/ network_io/os2/ poll/os2/ poll/unix/

2022-06-30 Thread William A Rowe Jr
This might be the most concerning break between apr 1.7.0 and 1.7.1,
and hope it was also evaluated for suitability for 1.8.0, because that
must remain binary compatible.

The fact that these are all prefaced apr_XXX and not apr__XXX doesn't
give me a lot of reassurance that someone isn't borrowing them, might
have to be an apr-2.0.0 breaking change.

Thoughts?

Bill

On Wed, Jan 19, 2022 at 5:17 PM  wrote:
>
> Author: ylavic
> Date: Wed Jan 19 23:17:18 2022
> New Revision: 1897222
>
> URL: http://svn.apache.org/viewvc?rev=1897222=rev
> Log:
> Revert r1896510.
>
> Per https://lists.apache.org/thread/rt6ggcztj7w1vwrmhz27q5fz27pr57w9
>
>
> Modified:
> apr/apr/branches/1.7.x/   (props changed)
> apr/apr/branches/1.7.x/file_io/win32/pipe.c
> apr/apr/branches/1.7.x/file_io/win32/readwrite.c
> apr/apr/branches/1.7.x/include/arch/unix/apr_arch_poll_private.h
> apr/apr/branches/1.7.x/include/arch/win32/apr_arch_file_io.h
> apr/apr/branches/1.7.x/network_io/os2/sockopt.c
> apr/apr/branches/1.7.x/poll/os2/pollset.c
> apr/apr/branches/1.7.x/poll/unix/epoll.c
> apr/apr/branches/1.7.x/poll/unix/kqueue.c
> apr/apr/branches/1.7.x/poll/unix/poll.c
> apr/apr/branches/1.7.x/poll/unix/pollcb.c
> apr/apr/branches/1.7.x/poll/unix/pollset.c
> apr/apr/branches/1.7.x/poll/unix/port.c
> apr/apr/branches/1.7.x/poll/unix/select.c
> apr/apr/branches/1.7.x/poll/unix/wakeup.c
>
> Propchange: apr/apr/branches/1.7.x/
> --
>   Reverse-merged /apr/apr/trunk:r1895106,1895111,1895175,1895181,1895465
>
> Modified: apr/apr/branches/1.7.x/file_io/win32/pipe.c
> URL: 
> http://svn.apache.org/viewvc/apr/apr/branches/1.7.x/file_io/win32/pipe.c?rev=1897222=1897221=1897222=diff
> ==
> --- apr/apr/branches/1.7.x/file_io/win32/pipe.c (original)
> +++ apr/apr/branches/1.7.x/file_io/win32/pipe.c Wed Jan 19 23:17:18 2022
> @@ -43,7 +43,7 @@ APR_DECLARE(apr_status_t) apr_file_pipe_
>  thepipe->timeout = timeout;
>  return APR_SUCCESS;
>  }
> -if (thepipe->ftype != APR_FILETYPE_PIPE) {
> +if (!thepipe->pipe) {
>  return APR_ENOTIMPL;
>  }
>  if (timeout && !(thepipe->pOverlapped)) {
> @@ -108,7 +108,7 @@ APR_DECLARE(apr_status_t) apr_file_pipe_
>  (*in) = (apr_file_t *)apr_pcalloc(pool_in, sizeof(apr_file_t));
>  (*in)->pool = pool_in;
>  (*in)->fname = NULL;
> -(*in)->ftype = APR_FILETYPE_PIPE;
> +(*in)->pipe = 1;
>  (*in)->timeout = -1;
>  (*in)->ungetchar = -1;
>  (*in)->eof_hit = 0;
> @@ -123,7 +123,7 @@ APR_DECLARE(apr_status_t) apr_file_pipe_
>  (*out) = (apr_file_t *)apr_pcalloc(pool_out, sizeof(apr_file_t));
>  (*out)->pool = pool_out;
>  (*out)->fname = NULL;
> -(*out)->ftype = APR_FILETYPE_PIPE;
> +(*out)->pipe = 1;
>  (*out)->timeout = -1;
>  (*out)->ungetchar = -1;
>  (*out)->eof_hit = 0;
> @@ -250,7 +250,7 @@ APR_DECLARE(apr_status_t) apr_os_pipe_pu
>  {
>  (*file) = apr_pcalloc(pool, sizeof(apr_file_t));
>  (*file)->pool = pool;
> -(*file)->ftype = APR_FILETYPE_PIPE;
> +(*file)->pipe = 1;
>  (*file)->timeout = -1;
>  (*file)->ungetchar = -1;
>  (*file)->filehand = *thefile;
> @@ -364,28 +364,32 @@ static apr_status_t create_socket_pipe(S
>  rv =  apr_get_netos_error();
>  goto cleanup;
>  }
> -/* Verify the connection by reading/waiting for the identification */
> -bm = 0;
> -if (ioctlsocket(*rd, FIONBIO, ) == SOCKET_ERROR) {
> -rv = apr_get_netos_error();
> -goto cleanup;
> +/* Verify the connection by reading the send identification.
> + */
> +do {
> +if (nc++)
> +Sleep(1);
> +nrd = recv(*rd, (char *)iid, sizeof(iid), 0);
> +rv = nrd == SOCKET_ERROR ? apr_get_netos_error() : APR_SUCCESS;
> +} while (APR_STATUS_IS_EAGAIN(rv));
> +
> +if (nrd == sizeof(iid)) {
> +if (memcmp(uid, iid, sizeof(uid)) == 0) {
> +/* Wow, we recived what we send.
> + * Put read side of the pipe to the blocking
> + * mode and return.
> + */
> +bm = 0;
> +if (ioctlsocket(*rd, FIONBIO, ) == SOCKET_ERROR) {
> +rv = apr_get_netos_error();
> +goto cleanup;
> +}
> +break;
> +}
>  }
> -nrd = recv(*rd, (char *)iid, sizeof(iid), 0);
> -if (nrd == SOCKET_ERROR) {
> -rv = apr_get_netos_error();
> +else if (nrd == SOCKET_ERROR) {
>  goto cleanup;
>  }
> -if (nrd == (int)sizeof(uid) && memcmp(iid, uid, sizeof(uid)) == 0) {
> -/* Got the right identifier, put the poll()able read side of
> -  

Re: svn commit: r1897210 - in /apr/apr/branches/1.7.x: ./ memory/unix/apr_pools.c threadproc/beos/thread.c threadproc/netware/thread.c threadproc/os2/thread.c threadproc/unix/thread.c threadproc/win32

2022-06-30 Thread William A Rowe Jr
I presume this is covered under our prior conversation about adding
API's to a subrevision, and is resolved?

On Wed, Jan 19, 2022 at 11:06 AM  wrote:
>
> Author: ylavic
> Date: Wed Jan 19 17:06:36 2022
> New Revision: 1897210
>
> URL: http://svn.apache.org/viewvc?rev=1897210=rev
> Log:
> apr_thread: Follow up to r1884078: Unmanaged pools for attached threads too.
>
> [Note 1.7.x: this removes apr__pool_unmanage() thus r1884103's ABI issues]
>
> r1884078 fixed lifetime issues with detached threads by using unmanaged pool
> destroyed by the thread itself on exit, with no binding to the parent pool.
>
> This commit makes use of unmanaged pools for attached threads too, they needed
> their own allocator anyway due to apr_thread_detach() being callable anytime
> later. apr__pool_unmanage() was a hack to detach a subpool from its parent, 
> but
> if a subpool needs its own allocator for this to work correctly there is no
> point in creating a subpool for threads (no memory reuse on destroy for short
> living threads for instance).
>
> Since an attached thread has its own lifetime now, apr_thread_join() must be
> called to free its resources/pool, though it's no different than before when
> destroying the parent pool was UB if the thread was still running (i.e.  not
> joined yet).
>
> Let's acknoledge that threads want no binding with the pool passed to them at
> creation time, besides the abort_fn which they can steal :)
>
>
> Merges r1897179 from trunk.
> Submitted by: ylavic
>
> Modified:
> apr/apr/branches/1.7.x/   (props changed)
> apr/apr/branches/1.7.x/memory/unix/apr_pools.c
> apr/apr/branches/1.7.x/threadproc/beos/thread.c
> apr/apr/branches/1.7.x/threadproc/netware/thread.c
> apr/apr/branches/1.7.x/threadproc/os2/thread.c
> apr/apr/branches/1.7.x/threadproc/unix/thread.c
> apr/apr/branches/1.7.x/threadproc/win32/thread.c
>
> Propchange: apr/apr/branches/1.7.x/
> --
>   Merged /apr/apr/trunk:r1897179
>
> Modified: apr/apr/branches/1.7.x/memory/unix/apr_pools.c
> URL: 
> http://svn.apache.org/viewvc/apr/apr/branches/1.7.x/memory/unix/apr_pools.c?rev=1897210=1897209=1897210=diff
> ==
> --- apr/apr/branches/1.7.x/memory/unix/apr_pools.c (original)
> +++ apr/apr/branches/1.7.x/memory/unix/apr_pools.c Wed Jan 19 17:06:36 2022
> @@ -2324,45 +2324,6 @@ APR_DECLARE(void) apr_pool_lock(apr_pool
>
>  #endif /* !APR_POOL_DEBUG */
>
> -/* For APR internal use only (for now).
> - * Detach the pool from its/any parent (i.e. un-manage).
> - */
> -apr_status_t apr__pool_unmanage(apr_pool_t *pool);
> -apr_status_t apr__pool_unmanage(apr_pool_t *pool)
> -{
> -apr_pool_t *parent = pool->parent;
> -
> -if (!parent) {
> -return APR_NOTFOUND;
> -}
> -
> -#if APR_POOL_DEBUG
> -if (pool->allocator && pool->allocator == parent->allocator) {
> -return APR_EINVAL;
> -}
> -apr_thread_mutex_lock(parent->mutex);
> -#else
> -if (pool->allocator == parent->allocator) {
> -return APR_EINVAL;
> -}
> -allocator_lock(parent->allocator);
> -#endif
> -
> -/* Remove the pool from the parent's children */
> -if ((*pool->ref = pool->sibling) != NULL) {
> -pool->sibling->ref = pool->ref;
> -}
> -pool->parent = NULL;
> -
> -#if APR_POOL_DEBUG
> -apr_thread_mutex_unlock(parent->mutex);
> -#else
> -allocator_unlock(parent->allocator);
> -#endif
> -
> -return APR_SUCCESS;
> -}
> -
>  #ifdef NETWARE
>  void netware_pool_proc_cleanup ()
>  {
>
> Modified: apr/apr/branches/1.7.x/threadproc/beos/thread.c
> URL: 
> http://svn.apache.org/viewvc/apr/apr/branches/1.7.x/threadproc/beos/thread.c?rev=1897210=1897209=1897210=diff
> ==
> --- apr/apr/branches/1.7.x/threadproc/beos/thread.c (original)
> +++ apr/apr/branches/1.7.x/threadproc/beos/thread.c Wed Jan 19 17:06:36 2022
> @@ -17,9 +17,6 @@
>  #include "apr_arch_threadproc.h"
>  #include "apr_portable.h"
>
> -/* Internal (from apr_pools.c) */
> -extern apr_status_t apr__pool_unmanage(apr_pool_t *pool);
> -
>  APR_DECLARE(apr_status_t) apr_threadattr_create(apr_threadattr_t **new, 
> apr_pool_t *pool)
>  {
>  (*new) = (apr_threadattr_t *)apr_palloc(pool,
> @@ -83,63 +80,56 @@ APR_DECLARE(apr_status_t) apr_thread_cre
>  {
>  int32 temp;
>  apr_status_t stat;
> +apr_allocator_t *allocator;
>
>  (*new) = (apr_thread_t *)apr_pcalloc(pool, sizeof(apr_thread_t));
>  if ((*new) == NULL) {
>  return APR_ENOMEM;
>  }
>
> -(*new)->data = data;
> -(*new)->func = func;
> -(*new)->exitval = -1;
> -
> -(*new)->detached = (attr && apr_threadattr_detach_get(attr) == 
> APR_DETACH);
> -if ((*new)->detached) {
> -stat = apr_pool_create_unmanaged_ex(&(*new)->pool,
> -

Re: svn commit: r1896748 - in /apr/apr/branches/1.7.x: ./ include/apr_ring.h

2022-06-30 Thread William A Rowe Jr
This commit especially troubles me, w.r.t. semver rules for 1.7.0 ->
1.7.1, since it actually changes the behavior for the rest of 1.x, and
compatibility with previous 1.x releases. If we know that this code
executes correctly compiled under 1.7.1/1.8.0 against a 1.7.0
libapr.so, then it's fine. And contrariwise, the code isn't worse
executed by a program compiled against apr-1.7.0 against apr 1.7.1+,
INCLUDING libapr.so 1.8.0.

If that review has been completed, please confirm. Otherwise we need
to reconsider until APR 2.0.

Thanks,

Bill

On Thu, Jan 6, 2022 at 6:11 AM  wrote:
>
> Author: ylavic
> Date: Thu Jan  6 12:11:48 2022
> New Revision: 1896748
>
> URL: http://svn.apache.org/viewvc?rev=1896748=rev
> Log:
> Merge r1896535, r1896747 from trunk:
>
>
> apr_ring: Don't break strict-aliasing rules in APR_RING_SPLICE_{HEAD,TAIL}().
>
> GCC-11 complains (see [1]) about apr_brigade_split_ex() seemingly issuing an
> out of bounds access, the cause being that APR_RING_SPLICE_{HEAD,TAIL}() is
> dereferencing an APR_RING_SENTINEL() and the cast there in not very aliasing
> friendly (see [2] for a great explanation by Martin Sebor).
>
> The APR (and user code) should never dereference APR_RING_SENTINEL(), it's 
> fine
> as a pointer only (i.e. for comparing pointers). So this commit modifies the
> APR_RING_SPLICE_{HEAD,TAIL}() and APR_RING_{CONCAT,PREPEND}() macros to use 
> the
> passed in APR_RING_HEAD's prev/next pointers directly instead of passing the
> APR_RING_SENTINEL() to APR_RING_SPLICE_{BEFORE,AFTER}().
>
> Semantically, this also clarifies that 
> APR_RING_{SPLICE,INSERT}_{BEFORE,AFTER}()
> should be called for an APR_RING_ENTRY while APR_RING_SPLICE_{HEAD,TAIL}() and
> APR_RING_{CONCAT,PREPEND}() are to be called with an APR_RING_HEAD.
>
> [1]
> In file included from ./include/apr_mmap.h:28,
>  from ./include/apr_buckets.h:32,
>  from buckets/apr_brigade.c:22:
> buckets/apr_brigade.c: In function ‘apr_brigade_split’:
> ./include/apr_ring.h:177:43: error: array subscript ‘struct apr_bucket[0]’ is 
> partly outside array bounds of ‘unsigned char[32]’ [-Werror=array-bounds]
>   177 | #define APR_RING_NEXT(ep, link) (ep)->link.next
>   | ~~^
> ./include/apr_ring.h:247:38: note: in expansion of macro ‘APR_RING_NEXT’
>   247 | APR_RING_NEXT((epN), link) = APR_RING_NEXT((lep), link);  
>   \
>   |  ^
> ./include/apr_ring.h:287:9: note: in expansion of macro 
> ‘APR_RING_SPLICE_AFTER’
>   287 | APR_RING_SPLICE_AFTER(APR_RING_SENTINEL((hp), elem, link),
>   \
>   | ^
> buckets/apr_brigade.c:118:9: note: in expansion of macro 
> ‘APR_RING_SPLICE_HEAD’
>   118 | APR_RING_SPLICE_HEAD(>list, e, f, apr_bucket, link);
>   | ^~~~
> buckets/apr_brigade.c:90:9: note: referencing an object of size 32 allocated 
> by ‘apr_palloc’
>90 | b = apr_palloc(p, sizeof(*b));
>   | ^
>
> [2] https://bugzilla.redhat.com/show_bug.cgi?id=1957353#c2
> (Note that the original comment from Martin Sebor talks about the struct
>  "_direntry" and the global variable "anchor" which relate to some httpd
>  code using an APR_RING in a similar way than apr_bucket_brigade does. So
>  below I allowed myself to edit the original comment to replace "_direntry"
>  by "apr_bucket" and "anchor" by "list" (the apr_bucket_brigade's member used
>  as the head of the ring) to make the link with the above commit message)
> "
> The message is a bit cryptic but it says that the code accesses an object
> (list) of some anonymous type as it was struct apr_bucket.  That's invalid
> because objects can only be accessed by lvalues of compatible types (or char).
>
> The APR_RING_ENTRY macro is defined like so:
>
> #define APR_RING_ENTRY(elem)\
> struct {\
> struct elem * volatile next;\
> struct elem * volatile prev;\
> }
>
> so given the definition:
>
> APR_RING_ENTRY(apr_bucket) list;
>
> list can only be accessed by lvalues of its (anonymous) type but the
> APR_RING_SENTINEL() macro defined like so:
>
> #define APR_RING_SENTINEL(hp, elem, link)   \
> (struct elem *)((char *)(&(hp)->next) - APR_OFFSETOF(struct elem, link))
>
> casts the address of list's next member minus some constant to a pointer to
> struct apr_bucket and that pointer is then used to access the prev pointer.
> The anonymous struct and struct apr_bucket are unrelated and cannot be used
> each other's members even if the corresponding members have the same type and
> are at the same offset within the bounds of the object.
> "
>
>
> apr_ring: Follow up to r1896535: Use APR_RING_{FIRST,LAST} macros.
>

Re: svn commit: r1890230 - in /apr/apr/branches/1.7.x: CHANGES include/arch/win32/apr_arch_misc.h misc/win32/misc.c network_io/win32/sockets.c

2022-06-30 Thread William A Rowe Jr
This is another "breaking" change. However, whatever enum values are
used, it should always be as a comparison operator, and this is an
internal value. It would prove unreliable if someone looked for
Windows 10+ and looked against apr-1.7.0.dll, but that just means a
feature available isn't recognized.

I think this one is safe for the 1.7.0 -> 1.7.1 based on semver, any concerns?

On Wed, May 26, 2021 at 2:57 PM  wrote:
>
> Author: michaelo
> Date: Wed May 26 19:57:29 2021
> New Revision: 1890230
>
> URL: http://svn.apache.org/viewvc?rev=1890230=rev
> Log:
> Backport r1861050, r1805309, r1861045, r1861046, r1861049, r1861053, 
> r1861054, r1861061 from trunk:
>
> * misc/win32/misc.c
>   (apr_get_oslevel): Fix condition to actually return APR_EGENERAL on
>unsupported OS.
>
> ===
>
> apr_socket_listen(): Allow larger backlog queue lengths on Windows 8+.
>
> Starting with Windows 8, the socket listen() function accepts a special
> SOMAXCONN_HINT(N) argument that allows making the backlog queue
> length larger than the otherwise predefined limit of around 200:
>
>   https://msdn.microsoft.com/en-us/library/windows/desktop/ms739168
>   
> https://blogs.msdn.microsoft.com/winsdk/2015/06/01/winsocks-listen-backlog-offers-more-flexibility-in-windows-8/
>
> Having a larger listen backlog can be used for certain high performance
> applications that need to handle lots of incoming connections.  One
> example would be the httpd server with it's "ListenBacklog" directive
> where setting it to a larger value currently allows serving more concurrent
> connections on Windows with mpm_winnt.
>
> * include/arch/win32/apr_arch_misc.h
>   (enum apr_oslevel_e): Add APR_WIN_8.
>
> * misc/win32/misc.c
>   (apr_get_oslevel): Determine whether we are running on Windows 7 or
>on Windows 8+.
>
> * network_io/win32/sockets.c
>   (SOMAXCONN_HINT): Define this macro in case we are building against
>an older version of Windows SDK.
>   (apr_socket_listen): Use SOMAXCONN_HINT() for the backlog queue length
>if it's supported by the Windows version we are running on.
>
> Patch by: Evgeny Kotkov 
>
> ===
>
> * misc/win32/misc.c
>   (apr_get_oslevel): Check return code from GetVersionEx().
>
> ===
>
> * misc/win32/misc.c
>   (apr_get_oslevel): Do not use static variables to protect from potential
>race condition.
>
> ===
>
> * misc/win32/misc.c
>   (apr_get_oslevel): Use GetVersionExW() with OSVERSIONINFOEXW to version
>information with service pack info.
>
> ===
>
> * include/arch/win32/apr_arch_misc.h
>   (enum apr_oslevel_e): Add APR_WIN_8_1.
>
> * misc/win32/misc.c
>   (apr_get_oslevel): Determine whether we are running on Windows 8 or
>on Windows 8.1+.
>
> ===
>
> Fix problem that apr_get_oslevel() was returning APR_WIN_XP on Windows 10.
>
> * include/arch/win32/apr_arch_misc.h
>   (enum apr_oslevel_e): Add APR_WIN_10.
>
> * misc/win32/misc.c
>   (apr_get_oslevel): Return APR_WIN_10 when dwMajorVersion is greater than 6.
>
> ===
>
> * include/arch/win32/apr_arch_misc.h
>   (enum apr_oslevel_e): Add APR_WIN_7_SP1.
>
> * misc/win32/misc.c
>   (apr_get_oslevel): Determine whether we are running on Windows 7 or on
>Windows 7 SP1.
>
> Modified:
> apr/apr/branches/1.7.x/CHANGES
> apr/apr/branches/1.7.x/include/arch/win32/apr_arch_misc.h
> apr/apr/branches/1.7.x/misc/win32/misc.c
> apr/apr/branches/1.7.x/network_io/win32/sockets.c
>
> Modified: apr/apr/branches/1.7.x/CHANGES
> URL: 
> http://svn.apache.org/viewvc/apr/apr/branches/1.7.x/CHANGES?rev=1890230=1890229=1890230=diff
> ==
> --- apr/apr/branches/1.7.x/CHANGES [utf-8] (original)
> +++ apr/apr/branches/1.7.x/CHANGES [utf-8] Wed May 26 19:57:29 2021
> @@ -47,6 +47,11 @@ Changes for APR 1.7.1
>   events are emitted on pool clear/destroy for proper accounting.
>   [Brane Čibej]
>
> +  *) apr_socket_listen: Allow larger listen backlog values on Windows 8+.
> + [Evgeny Kotkov ]
> +
> +  *) Fixed: apr_get_oslevel() was returning APR_WIN_XP on Windows 10
> +
>  Changes for APR 1.7.0
>
>*) apr_dir_read: [Unix] Dropped the preference of the dirread_r() flavor
>
> Modified: apr/apr/branches/1.7.x/include/arch/win32/apr_arch_misc.h
> URL: 
> http://svn.apache.org/viewvc/apr/apr/branches/1.7.x/include/arch/win32/apr_arch_misc.h?rev=1890230=1890229=1890230=diff
> ==
> --- apr/apr/branches/1.7.x/include/arch/win32/apr_arch_misc.h (original)
> +++ apr/apr/branches/1.7.x/include/arch/win32/apr_arch_misc.h Wed May 26 
> 19:57:29 2021
> @@ -110,7 +110,11 @@ typedef enum {
>  APR_WIN_XP_SP2 =   62,
>  APR_WIN_2003 = 70,
>  APR_WIN_VISTA =80,
> -APR_WIN_7 =90
> +APR_WIN_7 =90,
> +APR_WIN_7_SP1 =91,
> +APR_WIN_8  =   100,
> +APR_WIN_8_1 =  110,
> +APR_WIN_10 =   120
>  } apr_oslevel_e;
>
>  extern APR_DECLARE_DATA 

Re: svn commit: r1884103 - in /apr/apr/branches/1.7.x: ./ include/arch/beos/ include/arch/netware/ include/arch/unix/ include/arch/win32/ memory/unix/ threadproc/beos/ threadproc/netware/ threadproc/o

2022-06-30 Thread William A Rowe Jr
Hopefully we aren't waiting any longer for 1.8.0 than 1.7.1, so that
shouldn't be a real risk

On Thu, Jun 30, 2022 at 7:02 AM Yann Ylavic  wrote:
>
> On Wed, Jun 29, 2022 at 6:09 PM Yann Ylavic  wrote:
> >
> > By making such impracticable policies for revision changes (semver
> > sense), we are going to simply not fix anything but trivial changes
> > without bumping minor, and no one would/should care about the old
> > (though still "maintained") minor versions. What's the point of
> > releasing 1.7.1 then? Let's go with 1.8.0 only, simpler (less
> > maintenance) and better for everybody..
>
> OK, by re-reading this I realize I was a bit harsh, a bit of
> frustration here (fixes that don't get in..), sorry about that.
>
> After a bit of diving into svn history, this commit was mainly to
> address UAF with apr_thread_pool code when it was using detached
> threads, which is not the case anymore (r1884110, r1889219).
> So the scope of the fix might be for user detached thread usage "only"
> (which may not be common), and APR_POOL_DEBUG (which is mainly never
> by devs only, though something happening in APR_POOL_DEBUG mode might
> happen without when the planets are no longer aligned).
> I'll try to revert and see how it goes for, e.g., httpd's ci (where
> mod_watchdog uses thread pools).
>
> Anyway, if we get bug/crash reports for 1.7.1 and some (time
> consuming) invertigations shows that it's fixed in 1.8, I'm going to
> be a bit frustated too ;)
>
> >
> > Regards;
> > Yann.


Re: svn commit: r1887497 - in /apr/apr/branches/1.7.x: ./ CHANGES mmap/unix/mmap.c test/data/mmap_large_datafile.txt test/testfnmatch.c test/testmmap.c

2022-06-30 Thread William A Rowe Jr
IMNSO opinion, this seems like a somewhat safe 1.8 change. What you
want to ask is how someone who "discovers" apr.so.1 (.8) would react
with 1.7 code, and if that's clean and using only public API's, it
makes sense to me. That's not the case for other changes, but for this
one it shouldn't shock anyone who doesn't explicitly ask for -lapr
against libapr.so.1.7

On Thu, Jun 30, 2022 at 7:39 AM Yann Ylavic  wrote:
>
> On Wed, Jun 29, 2022 at 4:49 PM William A Rowe Jr  wrote:
> >
> > The eventual change still is present for apr
> > 1.8 adopters.
> >
> > Thoughts? ylavic are you willing to revert?
>
> It seems that no one is using apr_mmap_create() with an offset != 0 or
> the bug would have been reported (PR 65158 proved to be a cifs
> filesystem bug finally). Well, apr_bucket_file reads can use an mmap
> offset if planets misalign still, and that's possibly why we had quite
> some issues for "EnableMMap on" in httpd (IIRC), but since it was
> never nailed to this bug it usually ends with "please EnableMMap =>
> off"..
> So let's revert, if something comes up around this, it's probably
> another case where I'll first ask: can you reproduce with apr-1.8?
>
> For 1.8.x then (we can fix it there right?), I'd rather we revert this
> change and r1887508 to merge trunk's r1887060 and r1887506 directly
> (i.e. without the API workaround-try of this 1.7.x merge).
> That is, add the "poffset" field to (the end of) apr_mmap_t and be
> done with no tricks. But since I'm not sure what we can or not do with
> a minor bump I'll ask, can we do that?
>
> Regards;
> Yann.


Re: svn commit: r1887497 - in /apr/apr/branches/1.7.x: ./ CHANGES mmap/unix/mmap.c test/data/mmap_large_datafile.txt test/testfnmatch.c test/testmmap.c

2022-06-29 Thread William A Rowe Jr
This change does introduce an API change, and a somewhat possibly dangerous
structure size alteration, and seems to be out of bounds for apr 1.7.x
branch, as
identified in the commit message.

Anyone using a build crossing runtimes between apr 1.7.0 and 1.7.1 may
experience
unintended side-effects. The eventual change still is present for apr
1.8 adopters.

Thoughts? ylavic are you willing to revert?

On Thu, Mar 11, 2021 at 10:21 AM  wrote:
>
> Author: ylavic
> Date: Thu Mar 11 16:21:07 2021
> New Revision: 1887497
>
> URL: http://svn.apache.org/viewvc?rev=1887497=rev
> Log:
> Merge r1887060 from trunk:
>
> Align apr_mmap()ing offset to a page boundary.  PR 65158.
>
> This is requirement for unix systems, and otherwise can cause failures or
> undefined behaviour with some filesystems.
>
> Add apr_mmap_t->poffset on unixes to track the offset to the previous page,
> relative to the user requested/actual offset. This allows ->mm and ->size to
> still point to the actual data, while munmap() now deletes the full range
> from the start of the page.
>
> Tests updated accordingly.
>
>
> * Changes between r1887060 and this 1.7.x backport
>
> Since the layout of struct apr_mmap_t cannot change (non opaque), the poffset
> is over-allocated in mmap/unix/mmap.cmmap.c which uses this layout internally:
> struct mm_layout {
> apr_mmap_t mm;
> apr_off_t poffset;
> };
>
> This works for all the apr_mmap_t created by apr_mmap_create() with which any
> apr_mmap_*() function can then be called.
>
> If a user creates an apr_mmap_t "by hand", with this change it cannot be 
> passed
> to apr_mmap_dup() without causing an out-of-bound read. This is hardly a new
> limitation though, the refcounting which is the point of apr_mmap_dup() and
> apr_mmap_delete() comes from the private function mmap_cleanup() anyway, so
> there is no point in using these two functions with hand made apr_mmap_t (if
> there ever was a point in the first place to create an hand made 
> apr_mmap_t..).
>
>
> Submitted by: ylavic
>
> Added:
> apr/apr/branches/1.7.x/test/data/mmap_large_datafile.txt
>   - copied unchanged from r1887060, 
> apr/apr/trunk/test/data/mmap_large_datafile.txt
> Modified:
> apr/apr/branches/1.7.x/   (props changed)
> apr/apr/branches/1.7.x/CHANGES
> apr/apr/branches/1.7.x/mmap/unix/mmap.c
> apr/apr/branches/1.7.x/test/testfnmatch.c
> apr/apr/branches/1.7.x/test/testmmap.c
>
> Propchange: apr/apr/branches/1.7.x/
> --
>   Merged /apr/apr/trunk:r1887060
>
> Modified: apr/apr/branches/1.7.x/CHANGES
> URL: 
> http://svn.apache.org/viewvc/apr/apr/branches/1.7.x/CHANGES?rev=1887497=1887496=1887497=diff
> ==
> --- apr/apr/branches/1.7.x/CHANGES [utf-8] (original)
> +++ apr/apr/branches/1.7.x/CHANGES [utf-8] Thu Mar 11 16:21:07 2021
> @@ -1,6 +1,8 @@
>   -*- coding: utf-8 -*-
>  Changes for APR 1.7.1
>
> +  *) Align apr_mmap()ing offset to a page boundary.  PR 65158.  [Yann Ylavic]
> +
>*) Don't silently set APR_FOPEN_NOCLEANUP for apr_file_mktemp() created 
> file
>   to avoid a fd and inode leak when/if later passed to 
> apr_file_setaside().
>   [Yann Ylavic]
>
> Modified: apr/apr/branches/1.7.x/mmap/unix/mmap.c
> URL: 
> http://svn.apache.org/viewvc/apr/apr/branches/1.7.x/mmap/unix/mmap.c?rev=1887497=1887496=1887497=diff
> ==
> --- apr/apr/branches/1.7.x/mmap/unix/mmap.c (original)
> +++ apr/apr/branches/1.7.x/mmap/unix/mmap.c Thu Mar 11 16:21:07 2021
> @@ -14,6 +14,8 @@
>   * limitations under the License.
>   */
>
> +#include 
> +
>  #include "apr.h"
>  #include "apr_private.h"
>  #include "apr_general.h"
> @@ -33,6 +35,9 @@
>  #if APR_HAVE_STDIO_H
>  #include 
>  #endif
> +#if APR_HAVE_UNISTD_H
> +#include   /* for sysconf() */
> +#endif
>  #ifdef HAVE_SYS_STAT_H
>  #include 
>  #endif
> @@ -42,6 +47,34 @@
>
>  #if APR_HAS_MMAP || defined(BEOS)
>
> +#ifndef BEOS
> +struct mm_layout {
> +apr_mmap_t mm;
> +apr_off_t poffset;
> +};
> +
> +static APR_INLINE
> +apr_mmap_t *alloc_with_poffset(apr_pool_t *p)
> +{
> +struct mm_layout *layout = apr_pcalloc(p, sizeof(*layout));
> +return >mm;
> +}
> +
> +static APR_INLINE
> +void set_poffset(apr_mmap_t *mm, apr_off_t poffset)
> +{
> +struct mm_layout *layout = (struct mm_layout *)mm;
> +layout->poffset = poffset;
> +}
> +
> +static APR_INLINE
> +apr_off_t get_poffset(apr_mmap_t *mm)
> +{
> +struct mm_layout *layout = (struct mm_layout *)mm;
> +return layout->poffset;
> +}
> +#endif
> +
>  static apr_status_t mmap_cleanup(void *themmap)
>  {
>  apr_mmap_t *mm = themmap;
> @@ -61,7 +94,7 @@ static apr_status_t mmap_cleanup(void *t
>  #ifdef BEOS
>  rv = delete_area(mm->area);
>  #else
> -rv = munmap(mm->mm, 

Re: svn commit: r1884103 - in /apr/apr/branches/1.7.x: ./ include/arch/beos/ include/arch/netware/ include/arch/unix/ include/arch/win32/ memory/unix/ threadproc/beos/ threadproc/netware/ threadproc/o

2022-06-29 Thread William A Rowe Jr
This change does introduce an API change, and a somewhat possibly dangerous
structure size alteration, and seems to be out of bounds for apr 1.7.x branch.

Anyone using a build crossing runtimes between apr 1.7.0 and 1.7.1 may
experience
unintended side-effects. The eventual change still is present for apr
1.8 adopters.

While it's true that include/arch and include/private are not
considered generally
supported, folks are known to use them, and macros may cause the library to
bleed internals.

On Fri, Dec 4, 2020 at 12:15 PM  wrote:
>
> Author: ylavic
> Date: Fri Dec  4 18:15:55 2020
> New Revision: 1884103
>
> URL: http://svn.apache.org/viewvc?rev=1884103=rev
> Log:
> Merge r936323, r1460182, r1884077, r1884078 from trunk:
>
> OS/2: Clean up a thread's pool when it terminates.
>
>
> 
> Add apr_pool_owner_set function to allow use of pool debugging with threads
>
> Actually this function has been mentioned in the docs for over 10 years
> but has never been implemented.
> 
>
> Also consistently destroy the thread's pool when it exits normally, not only
> on apr_thread_exit(). This was already done on OS2.
>
> Other platforms than unix are untested.
>
>
> apr_thread: destroy the thread's pool at _join() time, unless _detach()ed.
>
> Destroying a joinable thread pool from apr_thread_exit() or when the thread
> function returns, i.e. from inside the thread itself, is racy or deadlocky
> with APR_POOL_DEBUG, with the parent pool being destroyed.
>
> This commit adds a ->detached flag in each arch's apr_thread_t struct to track
> whether a thread is detached (either at _create() or _detach() time). If
> detached, the pool is destroyed when the thread exits, otherwise when the
> thread is joined with apr_thread_join().
>
>
> apr_thread: use unmanaged pools for detached threads.
>
> A detached thread is by definition out of control, unjoinable, unmanaged,
> and it can terminate/exit after its parent pool is detroyed.
>
> To avoid use-after-free in this case, let's use an unmanaged pool for detached
> threads, either by creating an unmanaged pool from the start if the thread
> is created detached, or by "unmanaging" the pool if the thread is detached
> later with apr_thread_detach().
>
> To "umanage" the pool, provide a new internal helper, apr__pool_unmanage()
> which takes care of removing the pool from its parent's list.
>
>
> Submitted by: bjh, sf, ylavic, ylavic
>
> Modified:
> apr/apr/branches/1.7.x/   (props changed)
> apr/apr/branches/1.7.x/include/arch/beos/apr_arch_threadproc.h
> apr/apr/branches/1.7.x/include/arch/netware/apr_arch_threadproc.h
> apr/apr/branches/1.7.x/include/arch/unix/apr_arch_threadproc.h
> apr/apr/branches/1.7.x/include/arch/win32/apr_arch_threadproc.h
> apr/apr/branches/1.7.x/memory/unix/apr_pools.c
> apr/apr/branches/1.7.x/threadproc/beos/thread.c
> apr/apr/branches/1.7.x/threadproc/netware/thread.c
> apr/apr/branches/1.7.x/threadproc/os2/thread.c
> apr/apr/branches/1.7.x/threadproc/unix/thread.c
> apr/apr/branches/1.7.x/threadproc/win32/thread.c
>
> Propchange: apr/apr/branches/1.7.x/
> --
>   Merged /apr/apr/trunk:r936323,1460182,1884077-1884078
>
> Modified: apr/apr/branches/1.7.x/include/arch/beos/apr_arch_threadproc.h
> URL: 
> http://svn.apache.org/viewvc/apr/apr/branches/1.7.x/include/arch/beos/apr_arch_threadproc.h?rev=1884103=1884102=1884103=diff
> ==
> --- apr/apr/branches/1.7.x/include/arch/beos/apr_arch_threadproc.h (original)
> +++ apr/apr/branches/1.7.x/include/arch/beos/apr_arch_threadproc.h Fri Dec  4 
> 18:15:55 2020
> @@ -45,6 +45,7 @@ struct apr_thread_t {
>  void *data;
>  apr_thread_start_t func;
>  apr_status_t exitval;
> +int detached;
>  };
>
>  struct apr_threadattr_t {
>
> Modified: apr/apr/branches/1.7.x/include/arch/netware/apr_arch_threadproc.h
> URL: 
> http://svn.apache.org/viewvc/apr/apr/branches/1.7.x/include/arch/netware/apr_arch_threadproc.h?rev=1884103=1884102=1884103=diff
> ==
> --- apr/apr/branches/1.7.x/include/arch/netware/apr_arch_threadproc.h 
> (original)
> +++ apr/apr/branches/1.7.x/include/arch/netware/apr_arch_threadproc.h Fri Dec 
>  4 18:15:55 2020
> @@ -36,6 +36,7 @@ struct apr_thread_t {
>  void *data;
>  apr_thread_start_t func;
>  apr_status_t exitval;
> +int detached;
>  };
>
>  struct apr_threadattr_t {
>
> Modified: apr/apr/branches/1.7.x/include/arch/unix/apr_arch_threadproc.h
> URL: 
> http://svn.apache.org/viewvc/apr/apr/branches/1.7.x/include/arch/unix/apr_arch_threadproc.h?rev=1884103=1884102=1884103=diff
> ==
> --- apr/apr/branches/1.7.x/include/arch/unix/apr_arch_threadproc.h (original)
> +++ 

Re: svn commit: r1863234 - in /apr/apr/branches/1.7.x: ./ include/apr_pools.h memory/unix/apr_pools.c

2022-06-29 Thread William A Rowe Jr
This is the first commit which appeared to break APR in such a way
that code compiled
against apr 1.7.1 would be unable to run against apr 1.7.0 binaries.

Thoughts, rjung? Are you willing to revert? This would persist on apr
1.8.x for those
interested in this new feature.

On Wed, Jul 17, 2019 at 2:04 PM  wrote:
>
> Author: rjung
> Date: Wed Jul 17 19:04:43 2019
> New Revision: 1863234
>
> URL: http://svn.apache.org/viewvc?rev=1863234=rev
> Log:
> Add empty stubs for APR pool functions, that are
> only needed when APR_POOL_DEBUG is defined,
> but can be aor should be called from app code.
>
> Providing stubs allows the app code to stay the
> same when running with or without debugging APR
> lib (no need for app recompilation).
>
> Backport of r1863217 from trunk.
>
> Modified:
> apr/apr/branches/1.7.x/   (props changed)
> apr/apr/branches/1.7.x/include/apr_pools.h
> apr/apr/branches/1.7.x/memory/unix/apr_pools.c
>
> Propchange: apr/apr/branches/1.7.x/
> --
> --- svn:mergeinfo (original)
> +++ svn:mergeinfo Wed Jul 17 19:04:43 2019
> @@ -1,4 +1,4 @@
>  /apr/apr/branches/1.4.x:1003369,1101301
> -/apr/apr/trunk:733052,739635,741862,741866-741867,741869,741871,745763-745764,746310,747990,748080,748361,748371,748565,74,748902,748988,749810,760443,767895,775683,782838,783398,783958,784633,784773,788588,789050,793192-793193,794118,794485,795267,799497,800627,809745,809854,810472,811455,813063,821306,829490,831641,832904,835607,888669,892028,892159,892435,892909,896382,896653,899905,901088,902077,902090,908427,910419,910597,917819,917837-917838,923311,923320,925965,929796,930508,931973,932585,951771,960665,960671,979891,983618,989450,990435,1003338,100,107,1055657,1072165,1078845,1081462,1081495,1082177,1083038,1083242,1084662,1086695,1088023,1089031,1089129,1089438,1099348,1103310,1183683,1183685-1183686,1183688,1183693,1183698,1213382,1235047,1236970,1237078,1237507,1240472,1340286,1340288,1340470,1341193,1341196,1343233,1343243,1367050,1368819,1370494,1372018,1372022,1372093,1372849,1376957,1384764,1389077,1400200,1402868,1405985,1406690,1420106,1420109,1425356,142
>  
> 8809,1438940,1438957-1438959,1442903,1449568,1456418,1459994,1460179-1460180,1460184,1460241,1460399,1460405,1462738,1462813,1470186,1470348,1475509,1478905,1480067,1481186,1481262,1481265,1484271,1487796,1489517,1496407,1502804,1510354,1516261,1523384,1523479,1523484,1523505,1523521,1523604,1523613,1523615,1523844-1523845,1523853,1524014,1524031,1528797,1528809,1529488,1529495,1529515,1529521,1529668,1530786,1530800,1530988,1531554,1531768,1531884,1532022,1533104,1533111,1533979,1534882,1535027,1535157,1536744,1538171,1539374,1539389,1539455,1539603,1541054,1541061,1541486,1541655,1541666,1541744,1542601,1542779,1543033,1543056,1548575,1550907,1551650,1551659,1558905,1559382,1559873,1559975,1561040,1561260,1561265,1561321,1561347,1561356,1561361,1561394,1561555,1571894,1575509,1578420,1587045,1587063,1587543,1587545,1588878,1588937,1589982,1593611,1593614-1593615,1593680,1594684,1594708,1595549,1597797,1597803,1604590,1604596,1604598,1605104,1610854,1611023,1611107,160,167,
>  
> 1611120,1611125,1611184,1611193,1611466,1611515,1611517,1625173,1626564,1634615,1642159,1648830,1664406,1664447,1664451,1664471,1664769-1664770,1664775,1664904,1664911,1664958,1666341,1666411,1666458,111,1667420-1667421,1667423,1667900-1667901,1667903,1667914-1667916,1667962,1669077,1671292,1671329,1671356,1671386,1671389,1671513-1671514,1671957,1672354,1672366,1672495,1672575,1674566,1675644,1675656,1675668,1675967,1675970,1675982,1676013,1683521,1685929,1696140,1696767,1722547,1722557,1726928,1727020,1727160,1727175,1727199,1728957,1732582,1733451,1733594,1733694,1733706,1733708,1733775,1734816,1736552,1738791,1738925,1750374,1755709,1755740,1755746,1755758,1755954,1761279,1762326,1774712,1774973,1775069,1776994,1776998,1788334,1788337,1788929,1789947,1789998,1790045,1790200,1790296,1790302-1790304,1790330-1790331,1790436,1790439,1790444,1790446,1790488,1790521,1790523,1790569,1790632,1791598,1791718,1791728,1792620-1792622,1792625,1792961,1792963,1794266,1797415,1798105,18053
>  
> 80,1808039,1808836,1808910,1809649,1810452,1813286,1813330,1814239-1814240,1814326,1814329,1814331,1816527,1816628,1817485,1819857-1819858,1819860-1819861,1819934-1819935,1820080,1820755,1822357,1827534,1832203,1832691,1832985,1834253,1834494,1834541,1836235,1839068,1839493,1839622,1839769,1840372,1841078,1846806,1850087,1850095,1851541-1851542,1854123,1855049,1855347,1855443-1855444,1855839-1855840,1855855,1855864,1855867,1855877,1855949,1856022,1856042-1856043,1856046,1856050,1856063,1856089,1856096,1856178,1856192,1856196,1856756,1858596,1863205
> 

Re: Build errors in branches/1.8.x on Windows

2022-06-14 Thread William A Rowe Jr
Not sure what you have going on here. A too-old flavor of the Windows
SDK, or a bad choice
of the minimum SDK version? It seems the HAVE_IF_INDEXTONAME is set in the
include/arch/win32/apr_private.h, which is included by
include/arch/win32/apr_arch_misc.h,
which then does this;
#if defined(HAVE_IF_INDEXTONAME) && defined(_MSC_VER)
#include 
#endif

If the if_indextoname/if_nametoindex functions aren't resolved, this
logic must be avoided;
#ifdef if_nametoindex
#undef if_nametoindex
#endif
APR_DECLARE_LATE_DLL_FUNC(DLL_IPHLPAPI, NET_IFINDEX, WINAPI,
if_nametoindex, 0, (
IN PCSTR InterfaceName),
(InterfaceName));
#define if_nametoindex apr_winapi_if_nametoindex

#ifdef if_indextoname
#undef if_indextoname
#endif
APR_DECLARE_LATE_DLL_FUNC(DLL_IPHLPAPI, PCHAR, NETIOAPI_API_,
if_indextoname, 0, (
NET_IFINDEX InterfaceIndex,
PCHAR   InterfaceName),
(InterfaceIndex, InterfaceName));
#define if_indextoname apr_winapi_if_indextoname

Obviously those NET_IFINDEX declarations are required for this code to succeed.

But the biggest point is that _WIN32_WINNT needs to be set to latest
0x1010 and Windows
SDK needs to be up to date in order to conditionally support all
modern API calls, which is
most of what winsock wants to provide for modern apps (even truer unix
domain sockets).
The entire logic is to gracefully not support those calls,
conditionally, when the function
doesn't exist on an older version of the OS.

I checked, it's building fine here, did you forget to mention your
arguments to the cmake
invocation so we can sort this out? Which MSVC and which Windows SDK?



On Tue, Jun 14, 2022 at 8:02 AM Ivan Zhakov via dev  wrote:
>
> When building using CMake:
>   
> C:\PROGRA~1\MIB055~1\2022\PROFES~1\VC\Tools\MSVC\1432~1.313\bin\Hostx64\x64\cl.exe
>   /nologo -DAPR_APP -DWINNT -IC:\Ivan\SVN\apr\apr-1.8.x\out\build\x64-Debug 
> -IC:\Ivan\SVN\apr\apr-1.8.x\include 
> -IC:\Ivan\SVN\apr\apr-1.8.x\include\arch\win32 
> -IC:\Ivan\SVN\apr\apr-1.8.x\include\arch\unix 
> -IC:\Ivan\SVN\apr\apr-1.8.x\include\private /DWIN32 /D_WINDOWS /W3 /MDd /Zi 
> /Ob0 /Od /RTC1 /showIncludes 
> /FoCMakeFiles\libaprapp-1.dir\misc\win32\apr_app.c.obj 
> /FdCMakeFiles\libaprapp-1.dir\libaprapp-1.pdb /FS -c 
> C:\Ivan\SVN\apr\apr-1.8.x\misc\win32\apr_app.c
> C:\Ivan\SVN\apr\apr-1.8.x\include\arch\win32\apr_arch_misc.h(507): error 
> C2143: syntax error: missing ')' before '*'
> C:\Ivan\SVN\apr\apr-1.8.x\include\arch\win32\apr_arch_misc.h(507): error 
> C2143: syntax error: missing '{' before '*'
> C:\Ivan\SVN\apr\apr-1.8.x\include\arch\win32\apr_arch_misc.h(507): error 
> C2059: syntax error: ')'
> C:\Ivan\SVN\apr\apr-1.8.x\include\arch\win32\apr_arch_misc.h(507): error 
> C2061: syntax error: identifier 'apr_winapi_pfn_if_indextoname'
> C:\Ivan\SVN\apr\apr-1.8.x\include\arch\win32\apr_arch_misc.h(507): error 
> C2059: syntax error: ';'
> C:\Ivan\SVN\apr\apr-1.8.x\include\arch\win32\apr_arch_misc.h(504): error 
> C2513: ' ': no variable declared before '='
> C:\Ivan\SVN\apr\apr-1.8.x\include\arch\win32\apr_arch_misc.h(507): error 
> C2065: 'apr_winapi_pfn_if_indextoname': undeclared identifier
> C:\Ivan\SVN\apr\apr-1.8.x\include\arch\win32\apr_arch_misc.h(507): warning 
> C4047: '=': 'int' differs in levels of indirection from 'int *(__cdecl 
> *)(NET_IFINDEX,PCHAR)'
> C:\Ivan\SVN\apr\apr-1.8.x\include\arch\win32\apr_arch_misc.h(507): error 
> C2146: syntax error: missing ';' before identifier 'apr_load_dll_func'
> C:\Ivan\SVN\apr\apr-1.8.x\include\arch\win32\apr_arch_misc.h(504): error 
> C2100: illegal indirection
> C:\Ivan\SVN\apr\apr-1.8.x\include\arch\win32\apr_arch_misc.h(504): error 
> C2064: term does not evaluate to a function taking 0 arguments
> C:\Ivan\SVN\apr\apr-1.8.x\include\arch\win32\apr_arch_misc.h(504): warning 
> C4033: 'apr_winapi_if_indextoname' must return a value
>   [3/239] Building C object CMakeFiles\aprapp-1.dir\misc\win32\internal.c.obj
>   FAILED: CMakeFiles/aprapp-1.dir/misc/win32/internal.c.obj
>   
> C:\PROGRA~1\MIB055~1\2022\PROFES~1\VC\Tools\MSVC\1432~1.313\bin\Hostx64\x64\cl.exe
>   /nologo -DAPR_APP -DAPR_DECLARE_STATIC -DWINNT 
> -IC:\Ivan\SVN\apr\apr-1.8.x\out\build\x64-Debug 
> -IC:\Ivan\SVN\apr\apr-1.8.x\include 
> -IC:\Ivan\SVN\apr\apr-1.8.x\include\arch\win32 
> -IC:\Ivan\SVN\apr\apr-1.8.x\include\arch\unix 
> -IC:\Ivan\SVN\apr\apr-1.8.x\include\private /DWIN32 /D_WINDOWS /W3 /MDd /Zi 
> /Ob0 /Od /RTC1 /showIncludes 
> /FoCMakeFiles\aprapp-1.dir\misc\win32\internal.c.obj 
> /FdCMakeFiles\aprapp-1.dir\aprapp-1.pdb /FS -c 
> C:\Ivan\SVN\apr\apr-1.8.x\misc\win32\internal.c
> C:\Ivan\SVN\apr\apr-1.8.x\include\arch\win32\apr_arch_misc.h(498): error 
> C2143: syntax error: missing ')' before '__cdecl'
> C:\Ivan\SVN\apr\apr-1.8.x\include\arch\win32\apr_arch_misc.h(498): error 
> C2143: syntax error: missing '{' before '__cdecl'
> C:\Ivan\SVN\apr\apr-1.8.x\include\arch\win32\apr_arch_misc.h(498): error 
> C2059: syntax error: ')'
> 

Re: Need APR and APR-Util signing keys

2022-02-02 Thread William A Rowe Jr
https://downloads.apache.org/apr/KEYS

On Wed, Feb 2, 2022 at 4:37 PM Becky Flarity  wrote:
>
> Looking for GPG public signature keys for APR and APR-Util.  (See below)
>
> Don’t see them on https://people.apache.org/keys/committer.
>
> Where is a trusted source for obtaining these keys?
>
> gpg --verify apr-1.7.0.tar.gz.asc apr-1.7.0.tar.gz
> gpg: Signature made Mon 01 Apr 2019 12:56:25 PM CDT
> gpg:using RSA key 1E1F909911C78735
> gpg: Can't check signature: No public key
>
>  gpg --verify apr-util-1.6.1.tar.gz.asc apr-util-1.6.1.tar.gz
> gpg: Signature made Wed 18 Oct 2017 09:51:43 AM CDT
> gpg:using RSA key 8EFB19629088F565
> gpg: Can't check signature: No public key
>
>
>
> Thanks,
> Becky Flarity


Re: svn commit: r1897209 - in /apr/apr/branches/1.7.x: ./ include/apr_pools.h memory/unix/apr_pools.c

2022-01-24 Thread William A Rowe Jr
On Thu, Jan 20, 2022 at 9:38 AM Yann Ylavic  wrote:
>
> On Thu, Jan 20, 2022 at 3:56 PM William A Rowe Jr  wrote:
> >
> > On Thu, Jan 20, 2022 at 8:50 AM Eric Covener  wrote:
> > >
> > > > Code that will compile on 1.7.1 release
> > > > still won't compile on 1.7.0, unless I misunderstand something.
> > >
> > > Is it a requirement in this direction?
> > > https://apr.apache.org/versioning.html says "later versions"
> >
> > That's why I raise the question, I don't have the authoritative answer.
> >
> > Our SVN participants who adopted and recommended strongly that
> > APR follow the semantic versioning [1] approach so they could -also-
> > adopt it probably feel more strongly about this.
> >
> > "Bug fixes not affecting the API increment the patch version, backwards 
> > compatible API additions/changes increment the minor version, and backwards 
> > incompatible API changes increment the major version."
>
> It was proposed in another thread to add the APU macros to 1.7.x to
> ease later migration to 2.x, I don't think that adding these apr_pool_
> macros can have more compat issues..

I've been looking for follow up... seeing none, I'd point to SemVer 2.0.0 spec
bullet points 6. and 7. which clarify patch and minor bumps.

Adding the pool macros changed the API, so therefore they cause APR 1.8.0,
adding apu->apr compatibility macros will change the API, and therefore
cause APR-util 1.8.0. Even a noop macro introduces an API.

Are the authors willing to revert all manner of version breakage so we can jump
forward first with the patch release tag, and then the 1.8.0 (even an
-rc1) tags?


Re: svn commit: r1895508 - in /apr/apr/trunk: dso/win32/ file_io/win32/ locks/win32/ misc/unix/ misc/win32/ network_io/unix/ network_io/win32/ passwd/ shmem/win32/ threadproc/win32/ time/win32/ user/w

2022-01-20 Thread William A Rowe Jr
On Thu, Jan 20, 2022 at 12:49 PM Ivan Zhakov  wrote:
>
> On Thu, 20 Jan 2022 at 00:43, William A Rowe Jr  wrote:
>>
>> On Wed, Jan 19, 2022 at 11:31 AM Yann Ylavic  wrote:
>> >
>> > I'm a bit surprised that _WIN32_WCE used CreateThread() while more
>> > recent ones use _beginthreadex(), shouldn't it be the opposite?
>> > CreateThread() aligns more with the usual/modern naming convention on 
>> > Windows..
>>
>> Ignore naming conventions. We are a library that sits on top of
>> MSVCRT/LIBCMT, and
>> so the stdc lib we use needs to know a thread has started.
>> CreateThread doesn't let
>> the clib particpate and this causes specific problems for .dll loaded
>> modules like our lib.
>
> That's true.
>
> But Microsoft finally fixed this in Universal CRT ( Visual Studio 2015). In 
> UCRT beginthreadex() is almost a wrapper around CreateThread(). So maybe it 
> makes sense to require VS 2015 for APR trunk and just use CreateThread?

This is my docker build env... you won't find any argument from me;
https://github.com/envoyproxy/envoy-build-tools/blob/main/build_container/build_container_windows.ps1


Re: svn commit: r1897209 - in /apr/apr/branches/1.7.x: ./ include/apr_pools.h memory/unix/apr_pools.c

2022-01-20 Thread William A Rowe Jr
On Thu, Jan 20, 2022 at 12:32 PM William A Rowe Jr  wrote:
>
> On Thu, Jan 20, 2022 at 9:44 AM Yann Ylavic  wrote:
> >
> > My bad, re-reading the thread the proposal was a compat apu_ -> apr_
> > layer in APR *util* 1.7.x, yet since apr-util 1.7.0 is released
> > already it would be the same kind of compat issue.
>
> No worries. The larger question is about whether to do this during
> 1.7.0 -> 1.7.1?
> Or 1.7.0 -> 1.8.0, in anticipation of 2.0.0.

And the actual implications can totally live on 1.8.x branch, that's
why I forked it. Thanks for your efforts!


Re: svn commit: r1897209 - in /apr/apr/branches/1.7.x: ./ include/apr_pools.h memory/unix/apr_pools.c

2022-01-20 Thread William A Rowe Jr
On Thu, Jan 20, 2022 at 9:44 AM Yann Ylavic  wrote:
>
>
> My bad, re-reading the thread the proposal was a compat apu_ -> apr_
> layer in APR *util* 1.7.x, yet since apr-util 1.7.0 is released
> already it would be the same kind of compat issue.

No worries. The larger question is about whether to do this during
1.7.0 -> 1.7.1?
Or 1.7.0 -> 1.8.0, in anticipation of 2.0.0.


Re: svn commit: r1897209 - in /apr/apr/branches/1.7.x: ./ include/apr_pools.h memory/unix/apr_pools.c

2022-01-20 Thread William A Rowe Jr
On Thu, Jan 20, 2022 at 8:50 AM Eric Covener  wrote:
>
> > Code that will compile on 1.7.1 release
> > still won't compile on 1.7.0, unless I misunderstand something.
>
> Is it a requirement in this direction?
> https://apr.apache.org/versioning.html says "later versions"

That's why I raise the question, I don't have the authoritative answer.

Our SVN participants who adopted and recommended strongly that
APR follow the semantic versioning [1] approach so they could -also-
adopt it probably feel more strongly about this.

Bill.

[1] https://semver.org/


Re: svn commit: r1897209 - in /apr/apr/branches/1.7.x: ./ include/apr_pools.h memory/unix/apr_pools.c

2022-01-20 Thread William A Rowe Jr
I agree this ticks the ABI compatibility box.

I doesn't check the API compatibility box, IMO. Code that will compile
on 1.7.1 release
still won't compile on 1.7.0, unless I misunderstand something.

I'd really like us to start on the 1.7 releases today, but I'd have to
vote against this
candidate.

What are others' opinions?


On Wed, Jan 19, 2022 at 10:53 AM  wrote:
>
> Author: ylavic
> Date: Wed Jan 19 16:53:54 2022
> New Revision: 1897209
>
> URL: http://svn.apache.org/viewvc?rev=1897209=rev
> Log:
> apr_pools: Follow up to r1863234: APR_POOL_DEBUG macros for compat.
>
> To preserve ABI, define apr_pool_{join,find,num_bytes,lock}() as no-op
> macros in 1.7.x.
>
>
> Modified:
> apr/apr/branches/1.7.x/   (props changed)
> apr/apr/branches/1.7.x/include/apr_pools.h
> apr/apr/branches/1.7.x/memory/unix/apr_pools.c
>
> Propchange: apr/apr/branches/1.7.x/
> --
>   Reverse-merged /apr/apr/trunk:r1863217
>
> Modified: apr/apr/branches/1.7.x/include/apr_pools.h
> URL: 
> http://svn.apache.org/viewvc/apr/apr/branches/1.7.x/include/apr_pools.h?rev=1897209=1897208=1897209=diff
> ==
> --- apr/apr/branches/1.7.x/include/apr_pools.h (original)
> +++ apr/apr/branches/1.7.x/include/apr_pools.h Wed Jan 19 16:53:54 2022
> @@ -754,12 +754,14 @@ APR_DECLARE(void) apr_pool_cleanup_for_e
>   * In this case the caller must call apr_pool_join() to indicate this
>   * guarantee to the APR_POOL_DEBUG code.
>   *
> - * These functions have an empty implementation if APR is compiled
> - * with #APR_POOL_DEBUG not set.
> + * These functions are implemented when #APR_POOL_DEBUG is set and
> + * defined as no-op macros otherwise.
>   *
>   * @{
>   */
>
> +#if APR_POOL_DEBUG || defined(DOXYGEN)
> +
>  /**
>   * Guarantee that a subpool has the same lifetime as the parent.
>   * @param p The parent pool
> @@ -791,7 +793,21 @@ APR_DECLARE(apr_size_t) apr_pool_num_byt
>   */
>  APR_DECLARE(void) apr_pool_lock(apr_pool_t *pool, int flag);
>
> -/** @} */
> +#else /* APR_POOL_DEBUG or DOXYGEN */
> +
> +#undef apr_pool_join
> +#define apr_pool_join(a,b)
> +
> +#undef apr_pool_find
> +#define apr_pool_find(mem) (NULL)
> +
> +#undef apr_pool_num_bytes
> +#define apr_pool_num_bytes(p, rec) ((apr_size_t)0)
> +
> +#undef apr_pool_lock
> +#define apr_pool_lock(pool, lock)
> +
> +#endif /* APR_POOL_DEBUG or DOXYGEN */
>
>  /** @} */
>
>
> Modified: apr/apr/branches/1.7.x/memory/unix/apr_pools.c
> URL: 
> http://svn.apache.org/viewvc/apr/apr/branches/1.7.x/memory/unix/apr_pools.c?rev=1897209=1897208=1897209=diff
> ==
> --- apr/apr/branches/1.7.x/memory/unix/apr_pools.c (original)
> +++ apr/apr/branches/1.7.x/memory/unix/apr_pools.c Wed Jan 19 16:53:54 2022
> @@ -2929,29 +2929,6 @@ APR_DECLARE(apr_status_t) apr_pool_creat
>  return apr_pool_create_unmanaged_ex(newpool, abort_fn, allocator);
>  }
>
> -/*
> - * Other stubs, for people who are running
> - * mixed release/debug enviroments.
> - */
> -
> -APR_DECLARE(void) apr_pool_join(apr_pool_t *p, apr_pool_t *sub)
> -{
> -}
> -
> -APR_DECLARE(apr_pool_t *) apr_pool_find(const void *mem)
> -{
> -return NULL;
> -}
> -
> -APR_DECLARE(apr_size_t) apr_pool_num_bytes(apr_pool_t *pool, int recurse)
> -{
> -return 0;
> -}
> -
> -APR_DECLARE(void) apr_pool_lock(apr_pool_t *pool, int flag)
> -{
> -}
> -
>  #else /* APR_POOL_DEBUG */
>
>  #undef apr_palloc
>
>


Re: svn commit: r1895508 - in /apr/apr/trunk: dso/win32/ file_io/win32/ locks/win32/ misc/unix/ misc/win32/ network_io/unix/ network_io/win32/ passwd/ shmem/win32/ threadproc/win32/ time/win32/ user/w

2022-01-19 Thread William A Rowe Jr
On Wed, Jan 19, 2022 at 11:31 AM Yann Ylavic  wrote:
>
> I'm a bit surprised that _WIN32_WCE used CreateThread() while more
> recent ones use _beginthreadex(), shouldn't it be the opposite?
> CreateThread() aligns more with the usual/modern naming convention on 
> Windows..

Ignore naming conventions. We are a library that sits on top of
MSVCRT/LIBCMT, and
so the stdc lib we use needs to know a thread has started.
CreateThread doesn't let
the clib particpate and this causes specific problems for .dll loaded
modules like our lib.


Re: apr/trunk netware and os2

2022-01-18 Thread William A Rowe Jr
And RS2000?

On Mon, Jan 17, 2022 at 7:15 PM Yann Ylavic  wrote:
>
> On Tue, Jan 18, 2022 at 2:08 AM Yann Ylavic  wrote:
> >
> > On Tue, Nov 30, 2021 at 11:46 PM Mladen Turk  wrote:
> > >
> > > According to Wikipedia NetWare was discontinued in 2009
> > > and OS/2 in 2001, so if anyone can explain why we have
> > > those in trunk/apr-2 (that will be hopefully released one day).
> >
> > What about BeOS? We have some specifics for that too.
>
> Ditto for os390, also present in include/arch.
>
> >
> > Regards;
> > Yann.


Re: Get rid of APU api in favor of APR for apr/turk

2022-01-17 Thread William A Rowe Jr
On Sun, Jan 16, 2022 at 11:08 PM Greg Stein  wrote:
>
> On Sun, Jan 16, 2022 at 11:02 PM William A Rowe Jr  
> wrote:
>>
>> People will need to change their code to apr-2.x. But in the interim,
>> apr-util 1.7.0
>> aught to have a compat layer to let authors adopt apu_ -> apr_ conventions
>> ahead of time. Do we concur?
>
>
> If a release is coming up, then sure: adding new APIs to 1.7 is totally 
> in-bounds.

There are 1.7 and 1.8 releases arriving this month. I meant to type 1.8 for the
new compat wrappers, but you are right, we could inject macros any time.
We just can't change exported symbols within the lifespan of 1.X where X is
any integer.


Re: Get rid of APU api in favor of APR for apr/turk

2022-01-16 Thread William A Rowe Jr
On Sun, Jan 16, 2022 at 10:46 PM Greg Stein  wrote:
>
> On Sun, Jan 16, 2022 at 3:38 PM William A Rowe Jr  wrote:
>>
>> On Mon, Dec 13, 2021 at 7:43 AM Mladen Turk  wrote:
>> > On 08/12/2021 08:33, Ruediger Pluem wrote:
>> > > I assumed this as a trunk/2.0 only proposal. Is my assumption wrong?
>> >
>> > Yes, trunk only.
>> > Just a simple copy/paste leftover cleanup, mostly for internals
>>
>> Within that scope, yes, make it happen. Temp macros for the lifespan
>> of 2.0.x seems appropriate,
>> ending at release 2.1.0
>
>
> Given the clarification this is for 2.x, then I'd say: skip the temp macros. 
> They couldn't be removed in a point release anyways.
>
> That said, I'd be +0.5 for an "apu_legacy.h" to assist with people porting 
> 1.x code to 2.x. Downstream users would just throw in that #include into 
> appropriate source files, and call it a day. That header would be super easy 
> to maintain going forwards (ie. rarely, if *ever* change), so wouldn't really 
> be a maintenance burden.

I think we totally agree... and apr_legacy.h can be dragged in by apr.h IMHO.

KISS.

People will need to change their code to apr-2.x. But in the interim,
apr-util 1.7.0
aught to have a compat layer to let authors adopt apu_ -> apr_ conventions
ahead of time. Do we concur?

Cheers,

Bill


Re: apr/trunk Minimum version is Windows 10!?

2022-01-16 Thread William A Rowe Jr
On Wed, Dec 1, 2021 at 2:55 PM Mladen Turk  wrote:
>
> My proposal is to use 0x0603 (aka windows 8.1, windows server 2012r2)
> Those are still supported until 2023

-1. Again, that simply limits us to never pick up "modern" tcp/ip symbol
names from windows sdk headers. It doesn't prevent us from linking to
something we shouldn't, you are asking to pretend 10 years of MS OS
don't exist. It's pretty silly.

We need to make sure we use late binding on fn calls in the 2012R2
and Windows 8.1 for the next 20 months.

I'd like to understand what "defects" you found bumping to 0A0A (10'th
rev of Windows 10). I'd be shocked if it broke anything, because I've
done this to many projects which have never attempted it.

Happy to work with you to resolve it.


> It also ensures it can be compiled with Visual Studio 15
> and all that new 'performance' API works.
>
> Index: include/apr.hw
> ===
> --- include/apr.hw  (revision 1895478)
> +++ include/apr.hw  (working copy)
> @@ -194,7 +194,7 @@
>   #define WIN32_LEAN_AND_MEAN
>   #endif
>   #ifndef _WIN32_WINNT
> -#define _WIN32_WINNT 0x0A00
> +#define _WIN32_WINNT 0x0603
>   #endif
>   #ifndef NOUSER
>   #define NOUSER
> Index: include/apr.hwc
> ===
> --- include/apr.hwc (revision 1895478)
> +++ include/apr.hwc (working copy)
> @@ -194,7 +194,7 @@
>   #define WIN32_LEAN_AND_MEAN
>   #endif
>   #ifndef _WIN32_WINNT
> -#define _WIN32_WINNT 0x0A00
> +#define _WIN32_WINNT 0x0603
>   #endif
>   #ifndef NOUSER
>   #define NOUSER
>
>
> On 01/12/2021 21:03, Mladen Turk wrote:
> > Well, according to my friend Bill
> > with r1881421 we have
> >
> >
> >   #ifndef _WIN32_WINNT
> > -#define _WIN32_WINNT 0x0601
> > +#define _WIN32_WINNT 0x0A00
> >   #endif
> >
>
>
> Regards
> --
> ^TM


Re: svn commit: r1895541 - /apr/apr/trunk/CMakeLists.txt

2022-01-16 Thread William A Rowe Jr
On Fri, Dec 3, 2021 at 5:39 AM Mladen Turk  wrote:
>
> On 03/12/2021 12:35, Yann Ylavic wrote:
> > After all those CMakeLists changes, is building with cmake now
> > dedicated to Windows or does/can it work on *nixes too?
> > I never tried building with cmake so far on *nix, so just wondering..
>
> Even before the changes CMakeLists was windows only.
> All unixes works perfectly with configure && make.
>
> Anyhow, here's the latest status
> https://pastebin.com/gBxYtSZE

FWIW, on my current project I have closer to 40 dependencies, not just
a dozen. And nearly every one is built under cmake. And not with cmake's
ideas of rules, but Bazel injection of cflags/ldflags for unix or windows,
and I realized we can choose a platform, and trick cmake to not do much
extra anything by injecting a "bazel" target, not debug/release etc.

That's where I've been spinning my wheels.

It would be really useful if apr supported cmake builds on linux. I'm ok
with helping to see that happen. And it makes cmake a first class build
schema for us, whether one chooses autocruft ./configure, or cmake.
It should all produce a usable result


Re: Adding support for Windows/arm64

2022-01-16 Thread William A Rowe Jr
Mladen,

Sounds awesome, I think although some people do build from .mak or .dep files,
cmake is the project's entire solution. I'm happy to help and review
your work, if
you are looking for feedback, and it should be landed on apr 1.8 as
well as trunk
if you can do that.

Cheers,

Bill

On Tue, Dec 7, 2021 at 11:45 AM Mladen Turk  wrote:
>
> Hi,
>
> I plan to add support for Windows/arm64.
> The current win32 code presumes x86 architecture
> mostly for atomics and endianness.
>
>
>
>
> Regards
> --
> ^TM


Re: Get rid of APU api in favor of APR for apr/turk

2022-01-16 Thread William A Rowe Jr
On Mon, Dec 13, 2021 at 7:43 AM Mladen Turk  wrote:
>
> On 08/12/2021 08:33, Ruediger Pluem wrote:
> > I assumed this as a trunk/2.0 only proposal. Is my assumption wrong?
> >
>
> Yes, trunk only.
> Just a simple copy/paste leftover cleanup, mostly for internals

Within that scope, yes, make it happen. Temp macros for the lifespan
of 2.0.x seems appropriate,
ending at release 2.1.0


Re: svn commit: r1897021 - /apr/apr/branches/1.8.x/

2022-01-15 Thread William A Rowe Jr
On Sat, Jan 15, 2022 at 4:37 AM Yann Ylavic  wrote:
>
> On Sat, Jan 15, 2022 at 2:00 AM William A Rowe Jr  wrote:
> >
> At this point I question the relevance of releasing 1.7.1 at all, I
> think most people will be interested in 1.8.0 and I'd advise distros
> to upgrade to that too. Who will be using 1.7.1? What is the rationale
> for releasing both and how confusing will that be?'
> Hopefully Ivan can address the Windows apr_file_t (as suitable) soon
> enough, in this case could we release 1.8.0 only?

My expectation is it lives 1 1/2 hot seconds before there is something more
interesting.

But, if 1.8.0 release fails, it is a smart fallback, that's all. As
devs, I understand
our hesitancy to offering fallbacks, but there are consumers to consider.


Re: svn commit: r1897021 - /apr/apr/branches/1.8.x/

2022-01-14 Thread William A Rowe Jr
On Fri, Jan 14, 2022 at 5:07 AM Yann Ylavic  wrote:
>
> On Fri, Jan 14, 2022 at 1:59 AM William A Rowe Jr  wrote:
> >
> > A quick review of nm against libapr-1.so between 1.7.0 and 1.7.x
> > indicates that 1.7.1
> > isn't releasable as-is. Any hints?
> >
> > +T apr_pool_find
> > +T apr_pool_join
> > +T apr_pool_lock
> > +T apr_pool_num_bytes
>
> Those come from r1863234 [1], possibly we can let them as empty macros
> in 1.7.x and still add the ones for apr_pool_join() and
> apr_pool_num_bytes() which were missing for !APR_POOL_DEBUG?
> Yet it might make apps using them from 1.7.1 (w/o #ifdef
> APR_POOL_DEBUG) not compilable with 1.7.0 (though still ABI compatible
> with all 1.7.x), but no one would be able to use them as function
> pointers for instance, which would be a ABI/runtime breakage. Could
> that hurt actually?

Worse, a macro -> function without a major bump? That's awkward. Remember
the .so remains loadable throughout apr-1.so lifespan.

Afraid this has to be rejected outright on apr-1.7. There are contracts with our
developers that they can load the 1.7.0 .so in place of a binary built
against 1.7.2
and **nothing bad can happen** (other than the 'whoops - you wanted
that bug fix.)
The binaries won't fail to load. This sets us up for failure, and why put macro
stubs here if they will be no-ops? What purpose is this serving to break what
has been a pretty reliable solution in developers' hands?

See https://apr.apache.org/versioning.html

What's the actual reservation about 1.8.0? (After a prompt 1.7.1)?


> > +T apr__pool_unmanage
>
> That one is for internal use only (from [2]), not APR_DECLAREd so
> should be fine?

Still, exported on linux, curious how to avoid that. Even as an
internal, not an issue
in 1.8.0, an issue on 1.7.x branch now.


Re: Possible apr 1.7.1 showstopper from httpd test framework

2022-01-14 Thread William A Rowe Jr
On Fri, Jan 14, 2022 at 5:44 AM Joe Orton  wrote:
>
> On Fri, Jan 14, 2022 at 11:37:35AM +0100, Ruediger Pluem wrote:
> >
> > On 1/14/22 6:47 AM, William A Rowe Jr wrote:
> > > In addition to a broken release of brotli (where enc/dec don't specify
> > > -lbrotlicommon,
> > > even on trunk, for openssl and other consumers to ferret out that 
> > > binding), and
> > > lots of fun changes to build flags in curl 7.81 minor release (who does 
> > > that?)
> > > there appears to be one test failure with date formatting in httpd 2.4.x 
> > > branch
> > > (including release 2.4.52) and apr 1.7.x branch (including release 1.7.0);
> > >
> > > t/modules/include.t . 56/98 # Failed test 64 in
> > > t/modules/include.t at line 373
> > >
> > > Have not had time to investigate whether this is a change in perl 
> > > behavior, or
> > > possibly a regression caused by apr datetime handling in 1.7.x itself., 
> > > but any
> > > release apr-side should hold off just a bit to resolve this question.
> >
> > I cannot reproduce this with APR 1.7.x on RedHat 8 and our Travis builds at 
> > least for some builds
> > on Ubuntu use APR 1.7 as well and do not fail.
> > Is this probably a Windows specific issue? Can anyone reproduce on Windows?

Fun guess. Not on Windows at the moment. This is on ubuntu 18.04
running as a vmw
workstation guest vm with the following snapshot revisions (all just a
bit beyond the
current releases, including apr 1.7;

apr_ver=1.7.x-1896808
aprutil_ver=1.7.x-1894932
brotli_rev=e83c7b8
brotli_ver=master
curl_rev=d4492b6d1
curl_ver=master
expat_rev=9c42ebdd
expat_ver=master
httpd_ver=2.4.x-1896743
httpdtest_rev=763e0201
httpdtest_ver=trunk
jansson_rev=addeeef
jansson_ver=master
libxml2_rev=dea91c97
libxml2_ver=master
lua_ver=5.4.3
nghttp2_rev=02e6cad1
nghttp2_ver=master
openssl_rev=2080da84a4
openssl_ver=master
pcre_rev=81d3729
pcre_ver=master
zlib_rev=cacf7f1
zlib_ver=master

> IIRC there is some test case which can be sensitive to filesystems, and
> e.g. sometimes fails if NFS mounted?  I may be mixing it up with another
> test. The output of "./t/TEST -v t/modules/include.t" should help
> diagnose.

I will dig deeper and check some other linux flavors.

> That phrase "including release 1.7.0" implies it's not a 1.7.x
> regression if it also failed with 1.7.0, Bill? i.e. no reason to hold up
> a release?

It's a good point, I just don't care to tag a release when we have been making
changes to apr time logic as part of the next release. Hopefully it is something
interesting in the local deployment and perhaps a bad assumption in the test?
Will report back.


Re: svn commit: r1896724 - /apr/apr/trunk/file_io/win32/filestat.c

2022-01-13 Thread William A Rowe Jr
Reviewing, and writing tests. Thanks for your analysis!

On Wed, Jan 5, 2022 at 11:45 AM  wrote:
>
> Author: ivan
> Date: Wed Jan  5 17:45:33 2022
> New Revision: 1896724
>
> URL: http://svn.apache.org/viewvc?rev=1896724=rev
> Log:
> Win32: Minor optimization of apr_stat() and apr_file_info_get().
>
> * file_io/win32/filestat.c
>   (reparse_point_is_link, apr_stat): Use FindFirstFileExW(FindExInfoBasic)
>instead of FindFirstFileW().
>
> Modified:
> apr/apr/trunk/file_io/win32/filestat.c
>
> Modified: apr/apr/trunk/file_io/win32/filestat.c
> URL: 
> http://svn.apache.org/viewvc/apr/apr/trunk/file_io/win32/filestat.c?rev=1896724=1896723=1896724=diff
> ==
> --- apr/apr/trunk/file_io/win32/filestat.c (original)
> +++ apr/apr/trunk/file_io/win32/filestat.c Wed Jan  5 17:45:33 2022
> @@ -240,7 +240,8 @@ static int reparse_point_is_link(WIN32_F
>  return 0;
>  }
>
> -hFind = FindFirstFileW(wfname, );
> +hFind = FindFirstFileExW(wfname, FindExInfoBasic, , 
> FindExSearchNameMatch,
> + NULL, 0);
>  if (hFind == INVALID_HANDLE_VALUE) {
>  return 0;
>  }
> @@ -653,7 +654,8 @@ APR_DECLARE(apr_status_t) apr_stat(apr_f
>  if ((rv = test_safe_name(fname)) != APR_SUCCESS) {
>  return rv;
>  }
> -hFind = FindFirstFileW(wfname, );
> +hFind = FindFirstFileExW(wfname, FindExInfoBasic, ,
> + FindExSearchNameMatch, NULL, 0);
>  if (hFind == INVALID_HANDLE_VALUE)
>  return apr_get_os_error();
>  FindClose(hFind);
>
>


Re: Minimum msvc version to compile apr/trunk on Windows

2022-01-13 Thread William A Rowe Jr
On Thu, Dec 2, 2021 at 1:33 PM Mladen Turk  wrote:
>
> On 02/12/2021 13:21, Ivan Zhakov wrote:
> > On Wed, 1 Dec 2021 at 21:26, Mladen Turk  > > wrote:
> >
> > So no uuidof used in recent Windows SDK.
> >
> > Btw Visual Studio 2008 support ended April 10, 2018 [1].
>
> Sure, being abld to build with VS2008 is not a problem.
> Issues with API beyond Windows SDK 7.1.A
> (Latest one supporting Windows 7.1sp1)

Got it backwards. We'll need -builders- to use an SDK that supports 20H2
to pick up the logic for AF_UNIX and 21H2 for as advanced tcp constructs
to resolve origin ip etc.

The fact the SDK is for a newer OS does not render it incompatible
with the older OS.
Only once we use the new API binding and try to load it directly in
the old OS, without
using delayed binding and fallback on failure to resolve the new function.


Re: Minimum msvc version to compile apr/trunk on Windows

2022-01-13 Thread William A Rowe Jr
On Fri, Dec 10, 2021 at 8:47 AM Ivan Zhakov  wrote:
>
> On Thu, 2 Dec 2021 at 22:33, Mladen Turk  wrote:
>>
>> Issues with API beyond Windows SDK 7.1.A
>> (Latest one supporting Windows 7.1sp1)
>>
>> It just means apr-2 won't be able to run on anything below Windows 10.
>> There a lots of code you added recently that uses that new API
>> breaking also possibility to port that back to 1.7.x, 1.6.x branch.
>>
>> Dunno, maybe that's a good thing after all.
>>
> We discussed some time ago that minimum version for APR trunk is  Windows 
> 7/Windows Server 2008 R2:
> https://lists.apache.org/thread.html/de5af5247324db3e0636a654f2c23d3ff3a94a91f9d35adf5402b927@%3Cdev.apr.apache.org%3E
>
> As far I'm aware APR trunk should work fine in Windows 7/Windows Server 2008 
> R2.

Correct, although we should begin ignoring these EOL operating systems
which expired
2 years ago at this point.

Oct 10, 2023 is the end of Extended EOL of Windows Server 2012 and Windows 8.1,
and  by the end of next year, they should also be dropped.

As we call API functions that aren't present before the Windows 10
API, we simply need
to add new dll's such as ws2tcpip.dll to misc/win32/misc.c and api
function prototypes
to include/arch/win32/apr_arch_misc.h.

Right now we should be axing a number of these in apr_arch_misc.h
which have been
present since Server 2012 for the apr 2.0 master branch.

There's a lot of confusion; we should simply define WIN32_WINNT now as
0x0A0A and
be done with it. We simply need to do the work around missing calls.
And will need a
somewhat more dynamic expression for things like APR_HAS_ macros which depend
on the -current- os level.


Re: svn commit: r1896510 - in /apr/apr/branches/1.7.x: ./ file_io/win32/ include/arch/unix/ include/arch/win32/ network_io/os2/ poll/os2/ poll/unix/

2022-01-13 Thread William A Rowe Jr
On Thu, Jan 13, 2022 at 2:37 PM Ruediger Pluem  wrote:
>
> On 1/13/22 7:04 PM, Ivan Zhakov wrote:
> >
> > On Fri, 7 Jan 2022 at 17:33, Yann Ylavic  wrote:
> >>
> >> On Fri, Jan 7, 2022 at 2:50 PM Ivan Zhakov  wrote:
> >>>
> >>> I also have a high-level objection against backporting this change to
> >>> APR 1.7.x: IMHO APR 1.7.x is a stable branch and I think that only
> >>> regression fixes should be backported to the stable branch. r1896510
> >>> is a significant change and as far as I understand it's not a
> >>> regression fix. So I think it would be better to revert r1896510 and
> >>> release it as part of APR 2.0 (or 1.8.x).
> >>
> >> I think that most if not all of the changes to 1.7.x since 1.7.0 are
> >> fixes for bugs that were there before 1.7 already, not regressions
> >> introduced by 1.7.0.
> >
> > Agreed on the bugfix/regressions part. I have misunderstood that
> > r1896510 is a bugfix, perhaps, due to its size, and was thinking that
> > it adds new functionality. But even with that in mind, I still think
> > that the size of the change might be just too large for it to be an
> > appropriate fit for a patch release.
> >
> > Speaking of the change itself, I think that there might be an
> > alternative to making the apr_file_t also handle sockets on Windows.
> > It might be better to specifically change the pollset implementation
> > so that on Windows it would add a socket and use it for wakeup,
> > instead of using the socket disguised as a file.
> >
> > If this alternative approach sounds fine, I could try to implement it.
>
> But this could wait for a 1.7.2, correct? I am asking because there is some 
> desire to get 1.7.1 out of the door soon.
> And yes I would be happy with 1.7.2 that only adds this over 1.7.1 and is 
> released soon after 1.7.2.

Although this doesn't "appear" to introduce an API change, based on the
scope it seems significant enough to merit a 1.8.0 release, following
1.7.1. I've forked a working branch 1.8.x to continue this effort without
interruption. Your call whether it should persist in 1.7.1, it seems that
apr >= 1.8 is an easier decision point for developers looking for these
enhancements.


Possible apr 1.7.1 showstopper from httpd test framework

2022-01-13 Thread William A Rowe Jr
In addition to a broken release of brotli (where enc/dec don't specify
-lbrotlicommon,
even on trunk, for openssl and other consumers to ferret out that binding), and
lots of fun changes to build flags in curl 7.81 minor release (who does that?)
there appears to be one test failure with date formatting in httpd 2.4.x branch
(including release 2.4.52) and apr 1.7.x branch (including release 1.7.0);

t/modules/include.t . 56/98 # Failed test 64 in
t/modules/include.t at line 373

Have not had time to investigate whether this is a change in perl behavior, or
possibly a regression caused by apr datetime handling in 1.7.x itself., but any
release apr-side should hold off just a bit to resolve this question.

And there's the issue of pcre 8.x sources no longer available for download from
ftp.pcre.org. While they still exist, there's already been one
security release that
ignored pcre 8.x considerations, so pcre 10.x obviously can't wait for httpd
release from trunk.


Re: svn commit: r1897021 - /apr/apr/branches/1.8.x/

2022-01-13 Thread William A Rowe Jr
A quick review of nm against libapr-1.so between 1.7.0 and 1.7.x
indicates that 1.7.1
isn't releasable as-is. Any hints?

+T apr_pool_find
+T apr_pool_join
+T apr_pool_lock
+T apr_pool_num_bytes
+T apr__pool_unmanage

Preserving whatever has changed on 1.8.x branch for ABI breaking changes.

On Thu, Jan 13, 2022 at 6:36 PM  wrote:
>
> Author: wrowe
> Date: Fri Jan 14 00:36:06 2022
> New Revision: 1897021
>
> URL: http://svn.apache.org/viewvc?rev=1897021=rev
> Log:
> Branch 1.8.x due to ABI-breaking commits
>
> Begin 1.8.x development from here, with no need to roll anything back
> from this branch. The 1.7.x branch will need breaking backports all
> rolled back.
>
>
> Added:
> apr/apr/branches/1.8.x/   (props changed)
>   - copied from r1897020, apr/apr/branches/1.7.x/
>
> Propchange: apr/apr/branches/1.8.x/
> --
> --- svn:ignore (added)
> +++ svn:ignore Fri Jan 14 00:36:06 2022
> @@ -0,0 +1,52 @@
> +Makefile
> +config.cache
> +config.log
> +config.nice
> +config.status
> +configure
> +libtool
> +apr-config
> +apr-*-config
> +apr-config.out
> +LibD
> +LibNTD
> +LibR
> +LibNTR
> +Debug
> +DebugNT
> +Release
> +ReleaseNT
> +*.ncb
> +*.opt
> +*.plg
> +apr.exp
> +exports.c
> +export_vars.[ch]
> +.libs
> +.deps
> +*.la
> +libapr.rc
> +*.aps
> +*.plg
> +*.dep
> +*.mak
> +*.rc
> +BuildLog.htm
> +*.stc
> +*.stt
> +*.sto
> +*.vcproj
> +*.vcproj.*
> +autom4te.cache
> +ltcf-c.sh
> +build-outputs.mk
> +.make.dirs
> +*.bb
> +*.bbg
> +*.da
> +coverage
> +apr*.pc
> +apr.spec
> +libtool.exe
> +
> +
>
> Propchange: apr/apr/branches/1.8.x/
> --
> --- svn:mergeinfo (added)
> +++ svn:mergeinfo Fri Jan 14 00:36:06 2022
> @@ -0,0 +1,4 @@
> +/apr/apr/branches/1.4.x:1003369,1101301
> +/apr/apr/trunk:733052,739635,741862,741866-741867,741869,741871,745763-745764,746310,747990,748080,748361,748371,748565,74,748902,748988,749810,760443,767895,775683,782838,783398,783958,784633,784773,788588,789050,793192-793193,794118,794485,795267,799497,800627,809745,809854,810472,811455,813063,821306,829490,831641,832904,835607,888669,892028,892159,892435,892909,896382,896653,899905,901088,902077,902090,908427,910419,910597,917819,917837-917838,923311,923320,925965,929796,930508,931973,932585,936323,951771,960665,960671,979891,983618,989450,990435,1003338,100,107,1055657,1072165,1078845,1081462,1081495,1082177,1083038,1083242,1084662,1086695,1088023,1089031,1089129,1089438,1099348,1103310,1183683,1183685-1183686,1183688,1183693,1183698,1213382,1235047,1236970,1237078,1237507,1240472,1340286,1340288,1340470,1341193,1341196,1343233,1343243,1367050,1368819,1370494,1372018,1372022,1372093,1372849,1376957,1384764,1389077,1400200,1402868,1405985,1406690,1420106,1420109,1425
>  
> 356,1428809,1438940,1438957-1438959,1442903,1449568,1456418,1459994,1460179-1460180,1460182,1460184,1460241,1460399,1460405,1462738,1462813,1470186,1470348,1475509,1478905,1478954,1480067,1481186,1481262,1481265,1484271,1487796,1489517,1496407,1502804,1510354,1516261,1523384,1523479,1523484,1523505,1523521,1523604,1523613,1523615,1523844-1523845,1523853,1524014,1524031,1528797,1528809,1529488,1529495,1529515,1529521,1529668,1530786,1530800,1530988,1531554,1531768,1531884,1532022,1533104,1533111,1533979,1534882,1535027,1535157,1536744,1538171,1539374,1539389,1539455,1539603,1541054,1541061,1541486,1541655,1541666,1541744,1542601,1542779,1543033,1543056,1548575,1550907,1551650,1551659,1558905,1559382,1559873,1559975,1561040,1561260,1561265,1561321,1561347,1561356,1561361,1561394,1561555,1571894,1575509,1578420,1587045,1587063,1587543,1587545,1588878,1588937,1589982,1593611,1593614-1593615,1593680,1594684,1594708,1595549,1597797,1597803,1604590,1604596,1604598,1605104,1610854,1611023,1
>  
> 611107,160,167,1611120,1611125,1611184,1611193,1611466,1611515,1611517,1625173,1626564,1634615,1642159,1648830,1664406,1664447,1664451,1664471,1664769-1664770,1664775,1664904,1664911,1664958,1666341,1666411,1666458,111,1667420-1667421,1667423,1667900-1667901,1667903,1667914-1667916,1667962,1669077,1671292,1671329,1671356,1671386,1671389,1671513-1671514,1671957,1672354,1672366,1672495,1672575,1674566,1675644,1675656,1675668,1675967,1675970,1675982,1676013,1683521,1685929,1696140,1696767,1722547,1722557,1726928,1727020,1727160,1727175,1727199,1728957,1732582,1733451,1733594,1733694,1733706,1733708,1733775,1734816,1736552,1738791,1738925,1750374,1755709,1755740,1755746,1755758,1755954,1761279,1762326,1774712,1774973,1775069,1776994,1776998,1788334,1788337,1788929,1789947,1789998,1790045,1790200,1790296,1790302-1790304,1790330-1790331,1790436,1790439,1790444,1790446,1790488,1790521,1790523,1790569,1790632,1791598,1791718,1791728,1792620-1792622,1792625,1792961,1792963,179426
>  
> 

Backport svn log style request

2022-01-13 Thread William A Rowe Jr
We have traditionally described a commit even when performing
a backport. I'd traditionally cut and pasted the actual log entries
and then compressed them some to ignore the trivial stuff that
doesn't particularly apply to the combined patch. Then decorated
with the usual;
Authored by: [commiter/contributor list]
Backports: [rev list]

At some point, it seems authors have drifted from describing
the backport first, and adding an entry for Backports: [list]
to listing the backports, and then describing the patch.

If you have a look at https://github.com/apache/apr/commits/1.7.x
or any other summary, this new style is most unhelpful looking at
any summary of commits. Contrast this with active development
at https://github.com/apache/apr/commits/trunk

Would folks agree with returning to the older logging style, where
the purpose begins the svn log entry so there is something legible in
1-liner terse log summaries?


Re: apr/trunk netware and os2

2022-01-13 Thread William A Rowe Jr
On Tue, Nov 30, 2021 at 4:47 PM Mladen Turk  wrote:
>
> BTW did anyone was able to compile trunk on netware or os2
> There are bunch of those NWGNUmakefiles and netware subdirs

My +1 to drop both from trunk.

> According to Wikipedia NetWare was discontinued in 2009
> and OS/2 in 2001, so if anyone can explain why we have
> those in trunk/apr-2 (that will be hopefully released one day).
>
> IMHO, we should drop all that as well as windows stuff like
> APR_HAS_ANSI_FS and _WIN32_WCE (the latest one beeing my fault)

My +1 to drop WCE.

My +.9 to drop ANSI, but it begs the question of whether anyone truly relies
on an ANSI build for SBCS behavior. But I'm highly confident of our utf-8
implementation and am happy to see ANSI dropped, myself, unless it has
an audience whose needs we hadn't addressed.


Re: svn:ignore a few more paths to help TortoiseSVN's build system

2021-10-27 Thread William A Rowe Jr
While I'm a cmake user, there's no reason not to exclude these
directories, they are clearly not in our hierarchy.

On Wed, Jul 21, 2021 at 8:07 AM Eric Covener  wrote:
>
> Seems reasonable to me even on the release branches, will let it stew
> a bit here.
>
> On Wed, Jul 21, 2021 at 8:18 AM Daniel Sahlberg
>  wrote:
> >
> > Hi,
> >
> > TortoiseSVN is importing APR and APR-util via an svn:externals definition:
> > [[[
> > https://svn.apache.org/repos/asf/apr/apr/tags/1.6.5 apr
> > https://svn.apache.org/repos/asf/apr/apr-util/tags/1.6.1 apr-util
> > ]]]
> >
> > The build system creates a few new directories within the apr and apr-util 
> > directories for temporary build files. These clutter svn status and makes 
> > it more difficult to see if there are any real changes.
> >
> > I would propose to add a few more folders in svn:ignore. (I have already 
> > done similar modifications in the Subversion repository, see r1891003).
> >
> > [[[
> > Index: apr
> > ===
> > --- apr (revision 1891700)
> > +++ apr (working copy)
> >
> > Property changes on: apr
> > ___
> > Modified: svn:ignore
> > ## -14,8 +14,16 ##
> >  LibNTR
> >  Debug
> >  DebugNT
> > +debug_win32
> > +debug_win32_static
> > +debug_x64
> > +debug_x64_static
> >  Release
> >  ReleaseNT
> > +release_win32
> > +release_win32_static
> > +release_x64
> > +release_x64_static
> >  *.ncb
> >  *.opt
> >  *.plg
> > Index: apr-util
> > ===
> > --- apr-util(revision 1891700)
> > +++ apr-util(working copy)
> >
> > Property changes on: apr-util
> > ___
> > Modified: svn:ignore
> > ## -17,7 +17,15 ##
> >  apu-*-config
> >  apu-config.out
> >  Debug
> > +debug_win32
> > +debug_win32_static
> > +debug_x64
> > +debug_x64_static
> >  Release
> > +release_win32
> > +release_win32_static
> > +release_x64
> > +release_x64_static
> >  LibD
> >  LibR
> >  aprutil.opt
> > ]]]
> >
> > An alternative approach would be to ignore debug_* and release_*, this 
> > might help in case there is a future architecture (eg arm).
> >
> > (I realise this will be made on /trunk and not affect the tags that 
> > TortoiseSVN is pulling in until there is a new release, but it will be 
> > better in the future).
> >
> > Kind regards,
> > Daniel Sahlberg
> >
>
>
> --
> Eric Covener
> cove...@gmail.com


Re: apr-util-1.7.0 initial release, or 1.6.x incremental release (or both)?

2021-09-23 Thread William A Rowe Jr
On Mon, Sep 20, 2021 at 1:08 PM Rainer Jung  wrote:
>
> AFAIK if you want to build including crypto support, 1.7.x needs OpenSSL
> 1.0.2+. So if you need to stick to eg. 0.9.8, you are limited to 1.6.x.
> As we know, Apache httpd 2.4.x eg. still supports 0.9.8. So anyone
> building a recent 2.4.x using the totally outdated 0.9.8 must use APU 1.6.x.

Yes, but I don't even know whether it's worth calling out in anything other
than CHANGES, since 0.9.8 is simply DoA from a support or security perspective.
If a distributor is maintaining a 10yo OS distribution, they would in
theory also
be maintaining corresponding packages of openssl, and apr-util. As long as
httpd 2.4.x, subversion, etc support building against apr-1.7.0 (or
apr 2.0) then
there are really no worries.

> Concerning more 1.6.x updates: my understandig of the versioning was,
> that the middle digit provides API stability, ie. no new APIs are
> included in 1.N.x once the first patch release 1.N.0 was released. Based
> on that I would suggest to update 1.6.x very conservativly, mostly real
> bug fixes, no features. I would for instance favor not to include libxml
> support in 1.6.x.

I agree. Anything that would substantially change configure.in is just as likely
to disrupt someones' 1.6.x patch adoption as any ABI change, and needs to
be entirely avoided. Bug fix maintenance only.

> Deciding about having a 1.6.x release could depend on how important we
> think those committed fixes are.

Good point. I don't feel strongly either way, but can see where someone here
is welcome to suggest they prefer a more stable 1.6.x release without new
features, in which case we do both.

> Just my 2c.
>
> Best regards,
>
> Rainer
>
> Am 20.09.2021 um 11:35 schrieb Yann Ylavic:
> > On Fri, Sep 17, 2021 at 9:53 PM William A Rowe Jr  
> > wrote:
> >>
> >> We have much activity on apr-util-1.7.0 but no release actions for
> >> some time now.
> >> There are very minor changes to 1.6.x branch.
> >>
> >> Do we want to release 1.7.0, or only a 1.6.x bump?
> >> Or do we want to do both? Are we ready and are the API changes considered?
> >>
> >> All observations and feedback on apr-util releases encouraged...
> >
> > I tried to align apr-util 1.6.x and 1.7.x w.r.t. "bug fixes" in case
> > we want to release both, but I think that it's not necessarily worth
> > it.
> >
> > Looking at the diffs between the two here is what I found:
> >
> > * Build with Mariadb not in 1.6.x:
> > - http://svn.apache.org/viewvc?rev=1872061=rev
> >
> > * Build with libxml2/expat work in 1.7.x only (I think 1.6.x would
> > want it too?):
> > - http://svn.apache.org/viewvc?rev=1854603=rev
> > - http://svn.apache.org/viewvc?rev=1880287=rev
> >
> > (Also on the xml side still, do we want Windows' xmllite work in 1.7.x? 
> > Ivan?)
> >
> > * The crypto PRNG and Digests are new to 1.7.x (not 1.6.x material per
> > our compat rules).

As long as everyone is happy with their review of this work... I've looked at
the libxml2 work since it landed and am pleased. (Pity that it isn't a
subversion
solution since svn makes direct use of expat itself, so we don't
actually lighten
up the dependency list in that case.)

I'm ok with landing the 1.7.0 package based on observations of apr-2.0 bleed.

> >I'm not sure that 1.6.x is fine with openssl-1.1(.1)+,
> > OPENSSL_init_crypto() is in 1.7.x only (we probably want these changes
> > in both).

I've been building httpd with apr-1.6.x openssl-1.1[.0, and .1] and didn't see
any indication it has an issue. I've also been building bleed against apr-2.0
and observed no problems there.

> > * apr_dbm_driver_t and apr_dbm_get_driver() are new to 1.7.x (not
> > 1.6.x material per our compat rules).
> >
> > * apu_dso_init/load() fixes w.r.t. global pool scope in 1.7.x only (I
> > think we'd want to align here too).

This is a good bugfix to consider, but since it's a notable behavior change,
it's harder for the developer to say 1.6.5+ required as opposed to feature
is correct in 1.7+. But not impossible, so I'd leave it to the original patch
author to decide on backporting to 1.6.x as long as there is no abi change.

> > So if we want to release both, should we look at the above points that
> > can align?
> >
> >
> > Regards;
> > Yann.


apr-util-1.7.0 initial release, or 1.6.x incremental release (or both)?

2021-09-17 Thread William A Rowe Jr
We have much activity on apr-util-1.7.0 but no release actions for
some time now.
There are very minor changes to 1.6.x branch.

Do we want to release 1.7.0, or only a 1.6.x bump?
Or do we want to do both? Are we ready and are the API changes considered?

All observations and feedback on apr-util releases encouraged...

thanks,

Bill


Re: APR trunk testcrypto crash with OpenSSL 1.0.2

2021-09-17 Thread William A Rowe Jr
Did we determine this does not affect apr-util-1.7.x nor
apr-util-1.6.x branches?


Re: CVE-2021-35940: Apache Portable Runtime (APR): Regression of CVE-2017-12613

2021-09-16 Thread William A Rowe Jr
Note the fix referenced below will be picked up in APR 1.7.1

On Mon, Aug 23, 2021 at 5:25 AM Joe Orton  wrote:
>
> Description:
>
> An out-of-bounds array read in the apr_time_exp*() functions was fixed
> in the Apache Portable Runtime 1.6.3 release (CVE-2017-12613).  The fix
> for this issue was not carried forward to the APR 1.7.x branch, and
> hence version 1.7.0 regressed compared to 1.6.3 and is vulnerable to the
> same issue.
>
> The patch below addresses the issue:
> https://downloads.apache.org/apr/patches/apr-1.7.0-CVE-2021-35940.patch
>
> Credit:
>
> The Apache Portable Runtime project would like to thank Iveta Cesalova
> (Red Hat) for reporting this issue.
>
> References:
>
> http://svn.apache.org/viewvc?view=revision=1891198
> http://mail-archives.apache.org/mod_mbox/www-announce/201710.mbox/%3ccacsi251b8ualvm-rrh9fv57-zwi0zhyf3275_jpg1a9vevv...@mail.gmail.com%3E
> https://downloads.apache.org/apr/patches/apr-1.7.0-CVE-2021-35940.patch
>


Re: APR 1.7.1 release?

2021-09-16 Thread William A Rowe Jr
On Fri, Sep 10, 2021 at 3:34 AM Ruediger Pluem  wrote:
>
> On 9/10/21 10:28 AM, Steffen Land wrote:
> > Please be sure that the following two are included in 1.7.1 :
> >
> > PR 63491 regression in 1.7, see 
> > https://www.apachelounge.com/viewtopic.php?p=39558
>
> r1882155 brought this already to 1.7.
>
> > PR 61165 CPU deadlock under load, see  
> > https://github.com/SpiderLabs/ModSecurity/issues/2181
>
> Looks like to me that r1860057 is not backported yet to 1.7.

I agree with the fix and will pick this up shortly.


Re: APR 1.7.1 release?

2021-09-09 Thread William A Rowe Jr
Just as a reminder, with the goal to drop 1.7 apr and 1.7 apr-util
releases in one week,
please observe the practices in other projects and ask for 2 more sets
of eyeballs for
3 validated +1's on patches before backporting to these trees for the
next week. TIA!

I've had some success tweaking the abts framework to accomplish some
win32 fileinfo
validation of my proposed patch, so I should land that for willing
reviewers by CoB
tomorrow. I know we have several associated with the Subversion PMC willing to
lend a review, but I'll be following the same process with these fixes
to cure, and
further solve the apr 1.6.0 original and 1.7.1 release quirks with
mount symlinks.

Bill

On Thu, Sep 2, 2021 at 8:44 PM William A Rowe Jr  wrote:
>
> I'm willing to RM APR and APR-util 1.7 releases.
>
> Would propose we set a date out 2 weeks, anything lingering needs
> to be finalized with the usual oversight no later than the 8th, and
> we tag on the 14th, announce on the 15th when the mirrors have
> caught up. That gives enough days for committers to review the
> last changes to these release branches.
>
> But I'd be happier co-RM'ing this with a newer committer/PMC
> participant who wants to learn the ropes. Any volunteers?
> Other thoughts or observations?
>
> On Tue, Aug 31, 2021 at 3:09 AM Rainer Jung  wrote:
> >
> > Hi there,
> >
> > any chance we find an RM for a APR 1.7.1 release? At least there was the
> > fix for CVE-2021-35940 and CHANGES contains 15 more items (many of them
> > platform specific or build improvements). Last release 1.7.0 was in
> > April 2019.
> >
> > For APR-util I don't know the current state and release needs for the
> > 1.6.x and 1.7.x branches. Last 1.6.x release was in October 2017, 1.7.x
> > has never been released. CHANGES for 1.6.x only contains one
> > apr_dbm_gdbm fix plus a minor libtool use improvement.
> >
> > Apache httpd is planing to start a release cycle soon and it would be
> > nice to have a clean APR 1.7.1 and maybe APR-util also.
> >
> > Thanks and regards,
> >
> > Rainer


Re: APR 1.7.1 release?

2021-09-02 Thread William A Rowe Jr
I'm willing to RM APR and APR-util 1.7 releases.

Would propose we set a date out 2 weeks, anything lingering needs
to be finalized with the usual oversight no later than the 8th, and
we tag on the 14th, announce on the 15th when the mirrors have
caught up. That gives enough days for committers to review the
last changes to these release branches.

But I'd be happier co-RM'ing this with a newer committer/PMC
participant who wants to learn the ropes. Any volunteers?
Other thoughts or observations?

On Tue, Aug 31, 2021 at 3:09 AM Rainer Jung  wrote:
>
> Hi there,
>
> any chance we find an RM for a APR 1.7.1 release? At least there was the
> fix for CVE-2021-35940 and CHANGES contains 15 more items (many of them
> platform specific or build improvements). Last release 1.7.0 was in
> April 2019.
>
> For APR-util I don't know the current state and release needs for the
> 1.6.x and 1.7.x branches. Last 1.6.x release was in October 2017, 1.7.x
> has never been released. CHANGES for 1.6.x only contains one
> apr_dbm_gdbm fix plus a minor libtool use improvement.
>
> Apache httpd is planing to start a release cycle soon and it would be
> nice to have a clean APR 1.7.1 and maybe APR-util also.
>
> Thanks and regards,
>
> Rainer


Re: Subversion reports status fault on substituted drive

2020-12-23 Thread William A Rowe Jr
Sorry, I'd been pounding on a beta deliverable, it's my bad. This coming
week I have cycles to give
back to APR, CMake and other projects neglected during this crunch period.
I hope an end-of-year
(or very early January) release will meet your goals..



On Fri, Dec 11, 2020 at 6:32 AM Johan Corveleyn  wrote:

> On Fri, Nov 27, 2020 at 10:30 AM Johan Corveleyn 
> wrote:
> >
> > On Fri, Nov 27, 2020 at 2:26 AM William A Rowe Jr 
> wrote:
> > >
> > > On Thu, Nov 26, 2020 at 3:35 PM Nick Kew  wrote:
> > >>
> > >>
> > >> > On 24 Nov 2020, at 18:51, William A Rowe Jr 
> wrote:
> > >> >
> > >> > Yes, I'm on it. 1.6.x and prior somewhat works. 1.7.0 does not,
> asking
> > >> > for apr_stat of a root drive. Seeing the request yesterday for a
> new release,
> > >> > I'd like the chance to fix that quirk of the Win32 API and populate
> the stat
> > >> > struct with something resembling correct info about the root mounts.
> > >>
> > >> Is the issue here that we have something that simply won't map onto
> > >> Windows expectations, so there is no solution that will work for Johan
> > >> without re-introducting PR#47630?
> > >
> > >
> > > Close, and no, we won't reintroduce the old defect.
> > >
> > > It's a combination of treating [C:\]"The device" specially because
> there is no
> > > concept on windows of getting the C:\ filesystem, even though it is an
> NTFS
> > > (root) directory object, with contents and context.
> >
> > Just to be sure: the same reasoning goes for subst'ed drives, yes?
> > Say, W:\ pointing to C:\working-copy. That's also a "device" then?
> >
> > >>
> > >> If so, perhaps what we need is an additional argument that'll switch
> > >> between the two behaviours (ignored on non-windows), and to
> > >> deprecate the existing problematic API.
> > >>
> > >> I'm reluctant to jump in here myself 'cos it's been many years since I
> > >> had access to a Windows machine, let alone a dev environment.
> > >> But it's simple enough, I guess I could hack up a "looks OK" patch
> > >> to do that in the absence of any alternative proposal?
> > >
> > >
> > > What we don't want to do is to put two different behaviors on the user.
> > > The fix I have on the bench is to react *when we see the anticipated
> error*
> > > and know that is should react as looking at the device not the
> directory,
> > > and "make stuff up" about the remaining fields to be filled in a
> consistent
> > > manner. What has my attention right now is authoring the tests to prove
> > > up success or failure in this attempt.
> >
> > Can you draw inspiration from the behavior of APR 1.6.5?
> > From a compatibility point of view it would make sense to look at what
> > values 1.6.5 returned in this case, and see if they are the way to go
> > (or even, if we want to go the "compat as much as possible" route,
> > emulate 1.6.5 as much as possible?).
> >
> > Also, in line with what I said above: it would probably be a good idea
> > to also include a "subst" case in the tests ("subst T: C:\test";
> > repeat test on T:\)
> >
> > Thanks for working on this!
>
> Hi,
>
> Any news on this issue?
>
> Just as a FYI: in SVN we're getting close to starting the release
> train on 1.14.1. Would be nice for our Windows users to be able to
> include a 1.7.x APR with this issue fixed ;-).
> (Of course I know I don't get to dictate any project's release
> schedule, not to mention anyone's time spent on an issue -- I know I'm
> not doing the work here ... just consider this some meta-data in case
> it matters :-))
>
> --
> Johan
>


Re: Subversion reports status fault on substituted drive

2020-11-26 Thread William A Rowe Jr
On Thu, Nov 26, 2020 at 3:35 PM Nick Kew  wrote:

>
> > On 24 Nov 2020, at 18:51, William A Rowe Jr  wrote:
> >
> > Yes, I'm on it. 1.6.x and prior somewhat works. 1.7.0 does not, asking
> > for apr_stat of a root drive. Seeing the request yesterday for a new
> release,
> > I'd like the chance to fix that quirk of the Win32 API and populate the
> stat
> > struct with something resembling correct info about the root mounts.
>
> Is the issue here that we have something that simply won't map onto
> Windows expectations, so there is no solution that will work for Johan
> without re-introducting PR#47630?
>

Close, and no, we won't reintroduce the old defect.

It's a combination of treating [C:\]"The device" specially because there is
no
concept on windows of getting the C:\ filesystem, even though it is an NTFS
(root) directory object, with contents and context.




> If so, perhaps what we need is an additional argument that'll switch
> between the two behaviours (ignored on non-windows), and to
> deprecate the existing problematic API.
>
> I'm reluctant to jump in here myself 'cos it's been many years since I
> had access to a Windows machine, let alone a dev environment.
> But it's simple enough, I guess I could hack up a "looks OK" patch
> to do that in the absence of any alternative proposal?
>

What we don't want to do is to put two different behaviors on the user.
The fix I have on the bench is to react *when we see the anticipated error*
and know that is should react as looking at the device not the directory,
and "make stuff up" about the remaining fields to be filled in a consistent
manner. What has my attention right now is authoring the tests to prove
up success or failure in this attempt.


> BTW, please don't Cc: me replies to this thread.  I get it on the APR
> dev list, and I don't need a second copy!
>

:) Anyone is welcome to propose switching to httpd-dev like semantics for
reply-to handling. What I'm curious is whether the svn-dev side of
contributors
have a strong feeling one way or the other, because I know httpd-dev related
contributors all do.


Re: Subversion reports status fault on substituted drive

2020-11-24 Thread William A Rowe Jr
Yes, I'm on it. 1.6.x and prior somewhat works. 1.7.0 does not, asking
for apr_stat of a root drive. Seeing the request yesterday for a new
release,
I'd like the chance to fix that quirk of the Win32 API and populate the stat
struct with something resembling correct info about the root mounts.

That's particularly important to the subversion developers.



On Tue, Nov 24, 2020 at 4:37 AM Johan Corveleyn  wrote:

> On Fri, Jun 12, 2020 at 3:13 PM Johan Corveleyn  wrote:
> >
> > On Sun, May 17, 2020 at 12:22 AM William A Rowe Jr 
> wrote:
> > >
> > > This information is basically all we needed to resolve. The additional
> call
> > > to obtain more fileinfo cannot resolve a root dir. It could, if we
> transform the
> > > path \\?\c:\ to \\.\c: (the device c). Looking at this further
> tomorrow after some
> > > family hangout time tonight.
> > >
> > > Thanks for this level of detail!
> > >
> > > Bill
> >
> > Hi Bill,
> >
> > Any chance this could get fixed in the short / medium term, and
> > perhaps included in a 1.7.x APR release? Just checking ... :-)
> >
> > Apparently some TortoiseSVN users now miss the "Windows Deduplication
> > support" that APR 1.7.0 gave them ...
> > (TSVN 1.13.1 used apr 1.7.0, and TSVN 1.14.0 downgraded apr back to
> 1.6.5)
> >
> >
> https://groups.google.com/forum/?utm_medium=email_source=footer#!msg/tortoisesvn/IgmTDOmDNoM/ZesBeNXRBwAJ
> >
> > Thanks,
> > --
> > Johan
>
> Hi,
>
> I wanted to check again if there has been any progress on this issue.
> It would be nice to get this fixed in a 1.7.x APR release, so
> Subversion on Windows could get (the benefits of) APR 1.7.
>
> To recap, the following call to apr_stat:
>
> status = apr_stat(finfo, "C:/",
> APR_FINFO_TYPE | APR_FINFO_LINK | APR_FINFO_SIZE | APR_FINFO_MTIME,
> pool);
>
> succeeds with APR 1.6.5 on Windows, but fails with ARP 1.7.0 (returns
> 720002). Same goes for any other drive root (for instance with
> subst'ed drives, which was the original report -- sometimes people
> point a subst'ed drive to an SVN working copy root).
>
> Thanks,
> --
> Johan
>


Re: svn commit: r1881476 - /apr/apr/trunk/include/arch/win32/apr_arch_utf8.h

2020-09-08 Thread William A Rowe Jr
On Tue, Sep 8, 2020 at 4:11 AM Ivan Zhakov  wrote:

> On Sat, 5 Sep 2020 at 08:15,  wrote:
>
>> Author: wrowe
>> Date: Sat Sep  5 05:15:52 2020
>> New Revision: 1881476
>>
>> URL: http://svn.apache.org/viewvc?rev=1881476=rev
>>  /**
>> + * @deprecated @see apr_conv_utf8_to_utf16
>> + */
>> +#define apr_conv_utf8_to_ucs2(in, inbytes, out, outwords)
>> apr_conv_utf8_to_utf16(in, inbytes, out, outwords)
>>
> As far as I understand this doesn't fix ABI compatibility: applications
> linked with older APR cannot be used with newer one.
>

You understand correctly.

apr 2.0 has never been released, so trunk is subject to arbitrary changes.
Programs
linked against apr 1.x cannot load libapr-2.so binaries, that release is
ABI-breaking
by design.

Once apr 2.0 has been released, we have a new ABI to follow.

FWIW the ucs2 -> utf16 naming normalization only lives on 2.0 trunk,
and was never
/will never be considered for backport to 1.x..


Re: Subversion reports status fault on substituted drive

2020-05-16 Thread William A Rowe Jr
This information is basically all we needed to resolve. The additional call
to obtain more fileinfo cannot resolve a root dir. It could, if we
transform the
path \\?\c:\ to \\.\c: (the device c). Looking at this further tomorrow
after some
family hangout time tonight.

Thanks for this level of detail!

Bill


On Thu, Apr 23, 2020 at 7:00 PM Johan Corveleyn  wrote:

> On Wed, Apr 15, 2020 at 4:48 PM Johan Corveleyn  wrote:
> >
> > On Wed, Apr 15, 2020 at 3:51 PM William A Rowe Jr 
> wrote:
> > >
> > > Interesting.
> > >
> > > Not being familiar with the subversion code, it would be helpful to
> see what args
> > > are being passed to the offending (offended) apr_stat call, so we can
> investigate
> > > the behavior of APR 1.7.0 (or trunk) further.
> >
> > Understood. I'll try to dig into this a bit in the coming days.
>
> Okay, I finally got around to this.
>
> Subversion indeed performs an apr_stat call with the following flags [1]:
>
> apr_int32_t wanted = APR_FINFO_TYPE | APR_FINFO_LINK
>| APR_FINFO_SIZE | APR_FINFO_MTIME;
>
> The call to apr_stat happens on line 4464 in libsvn_subr/io.c [2]:
>
> status = apr_stat(finfo, fname_apr, wanted, pool);
>
> Apparently this call succeeds with apr 1.6.5 (on Windows), with
> fname_apr="C:/" and wanted as above (I added a screenshot from the VS
> Debugger to illustrate). The returned filetype is APR_DIR, etc ...
>
> But the call fails with apr 1.7.0, returning status == 720002 (see
> second screenshot, FWIW). Is Subversion doing something wrong here?
> Passing incorrect flags or something like that?
>
> [1]
> https://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?revision=1875971=markup#l3149
> [2]
> https://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?revision=1875971=markup#l4464
>
> --
> Johan
>


Re: apr-2 ... cleanup

2020-05-13 Thread William A Rowe Jr
On Wed, May 13, 2020, 15:08 Gregg Smith  wrote:

>
> #1, I think this may be your opinion but not so much mine. No, .mak/defs
> are not in trunk but they never have been and they have been added at
> fork to currently all the 1.0 series. And unless I'm mistaken, 1.7 was
> not that long ago.
>

Historically, httpd (and apr due to common roots) found the makefile churn
during active development to be too disruptive and not worth the iterations
in CVS and later svn. So the projects both put off mak file check-in until
development was largely complete, the API and file tree structure basically
fixed for the lifespan of a new release, and those files almost never
change over the life of a ver.minor epoch.

>


Re: apr-2 ... cleanup

2020-05-13 Thread William A Rowe Jr
I guess I'm very confused.

All modern flavors of Visual Studio *ship* cmake and ninja. The CMake
config lets us toggle options, things that static build files can't do
well. CMake will emit nmake, or ninja, or vcproj/sln. And as noted
elsethread, it should be able to build for Linux as easily as Windows,
although many more autoconf style tests are needed still.

Please explain why this is problematic 20 years later, now that CMake is a
very well established and well maintained solution?



On Wed, May 13, 2020, 16:49 Gregg Smith  wrote:

> Well, with very little work I'm sure the dsw/dsp will still work in any
> Visual Studio IDE of the day. They are not that difficult to modify by
> hand. The mak/dep files have to be generated since they are not in
> trunk. I wish they were in trunk even though they are more difficult to
> hand modify, but they still can be.
>
> Prebuild events I admit are quite tough to hand job into both mak/dsp if
> needed.
>
> Regards
>
> On 5/13/2020 1:48 PM, Mladen Turk wrote:
> > On 13/05/2020 22:08, Gregg Smith wrote:
> >>
> >> #1, I think this may be your opinion but not so much mine. No,
> >> .mak/defs are not in trunk but they never have been and they have been
> >> added at fork to currently all the 1.0 series. And unless I'm
> >> mistaken, 1.7 was not that long ago.
> >>
> >
> > Well, I'll try to make cmake build for winXX in apr/trunk fully working,
> > since it's already there and usable at least for apr-1.[6,7]
> >
> > But if someone have a will to manually edit and create those .dsp files
> > and then generate .mak files from them ... fine.
> >
> >
> > Regards
>


Re: apr-2 ... cleanup

2020-05-13 Thread William A Rowe Jr
On Wed, May 13, 2020 at 3:41 AM Mladen Turk  wrote:

> Hi,
>
> Related mostly to Windows port
>
> 1. Remove all those .dsp, .dsw .mak files from APR trunk
> None of them works for years.
> Replace all that with cmake
> 2. Remove all _WIN32_WCE, APR_NOT_IN_WCE
> Just a bunch of code for Windows CE that
> never worked, and no chance it will compile with
> current Windows code
>

+1


> 3. Drop all that APR_HAS_ANSI_FS and APR_HAS_UNICODE_FS
> Code in trunk need at least _WIN32_WINNT=0x0601 API
>

+1 - actually I'm favoring 0A00 since there is no flavor of supported
windows
not based on the server 2016/windows 10 API level.


> 4. Drop all references to apr-iconv project
> Unusable for years
>

+1. I've always wanted to return to the citrus project sources as a better
maintained
alternative which is BSD licensed, but never find the cycles. to do so


> General
>
> 1. Remove all APU_XXX references
> Since the apr-util is apr
> remove all APU_XXX defines and API
>

Almost +1. We might still keep some of the most common APU identity macros,
although they are now one-in-the-same. There's no real reason we must
change
them, they are still clearly in the namespace.


> 2. Netware and OS/2
> Both platforms are dead for years
> IMHO bunch of relic code no one use
>

+.75


> 3. https://svn.apache.org/viewvc/apr/apr/trunk/include/apu.h?view=markup
> More then 10 years ago ...
>
> I'm willing to solve all that since it's mainly
> cleanup of the copy/paste code that is dragging on for 10+ years
>
> It might even lead to apr-2.0.0-alpha1 :D


Sounding good to me, I'll chip in some help, but my next 3 weeks are still
pretty crazy.


Re: Subversion reports status fault on substituted drive

2020-05-08 Thread William A Rowe Jr
On Fri, May 8, 2020 at 4:50 AM Johan Corveleyn  wrote:

> On Fri, Apr 24, 2020 at 1:59 AM Johan Corveleyn  wrote:
> >
> > On Wed, Apr 15, 2020 at 4:48 PM Johan Corveleyn 
> wrote:
> > >
> > > On Wed, Apr 15, 2020 at 3:51 PM William A Rowe Jr 
> wrote:
> > > >
> > > > Interesting.
> > > >
> > > > Not being familiar with the subversion code, it would be helpful to
> see what args
> > > > are being passed to the offending (offended) apr_stat call, so we
> can investigate
> > > > the behavior of APR 1.7.0 (or trunk) further.
> > >
> > > Understood. I'll try to dig into this a bit in the coming days.
> >
> > Okay, I finally got around to this.
> >
> > Subversion indeed performs an apr_stat call with the following flags [1]:
> >
> > apr_int32_t wanted = APR_FINFO_TYPE | APR_FINFO_LINK
> >| APR_FINFO_SIZE | APR_FINFO_MTIME;
> >
> > The call to apr_stat happens on line 4464 in libsvn_subr/io.c [2]:
> >
> > status = apr_stat(finfo, fname_apr, wanted, pool);
> >
> > Apparently this call succeeds with apr 1.6.5 (on Windows), with
> > fname_apr="C:/" and wanted as above (I added a screenshot from the VS
> > Debugger to illustrate). The returned filetype is APR_DIR, etc ...
> >
> > But the call fails with apr 1.7.0, returning status == 720002 (see
> > second screenshot, FWIW). Is Subversion doing something wrong here?
> > Passing incorrect flags or something like that?
> >
> > [1]
> https://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?revision=1875971=markup#l3149
> > [2]
> https://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?revision=1875971=markup#l4464
>
> I can't debug further into APR itself at this time, I'm afraid. Is
> there anything else I can do? I'm not sure what the return code 720002
> means as a result of apr_stat().
>
> Is anyone able to reproduce this issue with APR 1.7.0 with a
> test-program merely calling apr_stat() on the root of a drive, with
> the flags
>
> APR_FINFO_TYPE | APR_FINFO_LINK | APR_FINFO_SIZE | APR_FINFO_MTIME
>
> ?
>
> At this point it's not even clear to me whether this problem is
> Windows-specific, or can also be reproduced on Linux / Unix, if you
> call it on "/".
>
> If it helps I can file an issue in Bugzilla, if you guys want me to.


I'll pursue this further on Monday and see if we can offer some workaround
for drive-root stat calls in apr itself.


Re: Subversion reports status fault on substituted drive

2020-04-15 Thread William A Rowe Jr
Interesting.

Not being familiar with the subversion code, it would be helpful to see
what args
are being passed to the offending (offended) apr_stat call, so we can
investigate
the behavior of APR 1.7.0 (or trunk) further.

I had to tweak the logic of the test I presented above to ask a
getfileinfo-based
query. Typically, it isn't possible to open a root or directory with
apr_file_open,
but there is some hidden logic which could be extended to the public API for
that purpose, if subversion is trying to rely on apr_file_info_get instead
of
apr_stat. I had to introduce that logic into the test in order to compare
results
between the two different code paths. Opening roots are particularly quirky
because roots can be confusingly accessed as devices or directory entities.



On Wed, Apr 15, 2020 at 3:45 AM Johan Corveleyn  wrote:

> On Fri, Apr 10, 2020 at 4:19 PM Johan Corveleyn  wrote:
> >
> > On Fri, Apr 10, 2020 at 3:45 PM William A Rowe Jr 
> wrote:
> > > On Thu, Apr 9, 2020 at 2:48 PM Johan Corveleyn 
> wrote:
> > >> On Thu, Apr 9, 2020 at 6:31 PM William A Rowe Jr 
> wrote:
> > >> >
> > >> > That's very likely, and this can be researched further to dig up
> exactly how
> > >> > subst is handled. (As you mention, it hasn't been common since the
> introduction
> > >> > of proper junction and directory/file symlinks.)
> > >> >
> > >> > It appears that the root directory is always problematic for an svn
> checkout
> > >> > (actually, most checkouts were problematic on svn when junctions or
> symlinks
> > >> > were in play on windows.) Looks like some additional special
> handling is going
> > >> > to be required even with queries on c:\ (a real, not a substituted
> volume root.)
> > >>
> > >> Thank you for looking into this.
> > >>
> > >> You're absolutely right: with TSVN 1.13 (APR 1.7.0) I get the same
> > >> erroneous behavior when 'svn -st'-ing a checkout to C:\ (real drive)
> > >>
> > >> [[[
> > >> C:\>svn st -q
> > >> !   .
> > >> ]]]
> > >
> > >
> > > Here's another experiment that drives me batty...
> > >
> > > svn co https://svn.apache.org/repos/asf/httpd/httpd/trunk httpd-2.x
> > > svn co https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4
> httpd-2.4
> > > svn co https://svn.apache.org/repos/asf/httpd/docs/trunk httpd-docs
> > > ln -s httpd-docs httpd-2.4/docs/manual/build
> > > ln -s httpd-docs httpd-2.x/docs/manual/build
> > > (flip the args using mklink /d or mklink /j on windows)
> >
> > That's a bit special: you're creating embedded working copies, which
> > is already special to begin with (even if they were all normal,
> > separate checkouts instead of symlinks).
> > There is code that tries to crawl up the tree to find the working copy
> > root. Perhaps that is going wrong in this case, depending on your CWD.
> > Not sure.
> >
> > > You'll see svn is fluxored until you change directory. A similar
> ugliness
> > > occurs if you attempt to check out a number of versions of httpd-test,
> > > and attempt to point all of the X apache-test checkouts into a single
> > > group.
> > >
> > > What I've determined is that apr_stat and apr_getfileinfo differ in
> only
> > > one aspect, that protections on a root directory are 700 or 777 based
> > > on the way we dress up fprot bits from windows ACLs, or not.
> > >
> > >> > Can you successfully `svn co {repos} /` on linux? Hoping to
> understand the
> > >> > scope of the issue.
> > >>
> > >> Sorry, no Linux at my disposal where I'm root. But I suppose the test
> > >> above with the checkout to C:\ already confirms your hypothesis.
> > >
> > >
> > > I'm not sure what svn is doing with the '.' root entity, but it likely
> made
> > > some absurd assumptions about symlinks.
> >
> > Yes, that's very well possible. It's a difficult area I think (also:
> > there is the distinction between a symlink to a working copy root,
> > which just needs to be "read through" for svn commands to act upon,
> > and symlinks that are part your versioned data, which need to be
> > versioned and re-created as symlinks in other working copies (cf. the
> > "svn:special" property) -- something which currently is not supported
> > on Windows (junctions), they are just converted into a text file with
> > the link reference in it ... a long stand

Re: Subversion reports status fault on substituted drive

2020-04-10 Thread William A Rowe Jr
On Thu, Apr 9, 2020 at 2:48 PM Johan Corveleyn  wrote:

> On Thu, Apr 9, 2020 at 6:31 PM William A Rowe Jr 
> wrote:
> >
> > That's very likely, and this can be researched further to dig up exactly
> how
> > subst is handled. (As you mention, it hasn't been common since the
> introduction
> > of proper junction and directory/file symlinks.)
> >
> > It appears that the root directory is always problematic for an svn
> checkout
> > (actually, most checkouts were problematic on svn when junctions or
> symlinks
> > were in play on windows.) Looks like some additional special handling is
> going
> > to be required even with queries on c:\ (a real, not a substituted
> volume root.)
>
> Thank you for looking into this.
>
> You're absolutely right: with TSVN 1.13 (APR 1.7.0) I get the same
> erroneous behavior when 'svn -st'-ing a checkout to C:\ (real drive)
>
> [[[
> C:\>svn st -q
> !   .
> ]]]
>

Here's another experiment that drives me batty...

svn co https://svn.apache.org/repos/asf/httpd/httpd/trunk httpd-2.x
svn co https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4 httpd-2.4
svn co https://svn.apache.org/repos/asf/httpd/docs/trunk httpd-docs
ln -s httpd-docs httpd-2.4/docs/manual/build
ln -s httpd-docs httpd-2.x/docs/manual/build
(flip the args using mklink /d or mklink /j on windows)

You'll see svn is fluxored until you change directory. A similar ugliness
occurs if you attempt to check out a number of versions of httpd-test,
and attempt to point all of the X apache-test checkouts into a single
group.

What I've determined is that apr_stat and apr_getfileinfo differ in only
one aspect, that protections on a root directory are 700 or 777 based
on the way we dress up fprot bits from windows ACLs, or not.

> Can you successfully `svn co {repos} /` on linux? Hoping to understand the
> > scope of the issue.
>
> Sorry, no Linux at my disposal where I'm root. But I suppose the test
> above with the checkout to C:\ already confirms your hypothesis.
>

I'm not sure what svn is doing with the '.' root entity, but it likely made
some absurd assumptions about symlinks.


Re: Subversion reports status fault on substituted drive

2020-04-09 Thread William A Rowe Jr
I'm pursuing something like the attached patch. Only one test fails, which
I presume
devolves back to the /. root comparison (so the data directory comparison
is good.)
The stat/getfileinfo comparison doesn't alert if the info can't be
retrieved from one or
the other call, which isn't good (the availability bits should be
identical.)

testfileinfo: -Line 105: apr_stat and apr_getfileinfo differ in
protection
FAILED 1 of 7
Failed TestsTotal   FailFailed %
===
testfileinfo7  1 14.29%

Out of cycles to dig deeper today, but just wanted to leave an update that
something
interesting is going to have to happen here to delve into the "root device"
vs a simple
"root directory"..


On Thu, Apr 9, 2020 at 11:31 AM William A Rowe Jr 
wrote:

> On Thu, Apr 9, 2020 at 8:01 AM Johan Corveleyn  wrote:
>
>> On Thu, Apr 9, 2020 at 1:20 PM Nick Kew  wrote:
>> > >
>> > > The below report about a problem with Subversion working on 'subst'ed
>> > > drives (Windows), seemed to be related to a change in APR 1.7.0
>> > > (https://svn.apache.org/r1855950),
>> >
>> > That fixed a bug PR#47630 that had been extensively discussed in
>> > bugzilla and shows as having 22 watchers - that's a lot of interest!
>> >
>> > Does that discussion not throw any light on your issue?  For example,
>> > Michael Schlenker's comments there refer to that bug affecting SVN.
>>
>> Thanks, I've read through that issue in Bugzilla now.
>>
>> It definitely seems like a useful improvement for proper support of
>> (different types of) reparse points on Windows. I'm not saying this
>> bugfix needs to be reverted. And indeed, Michael Schlenker's comments
>> point to a specific problem affecting SVN (in combination with Windows
>> Data Deduplication, which creates reparse points with tag
>> IO_REPARSE_TAG_DEDUP). So in general this is a step forward, and will
>> probably benefit SVN for use on these specific types of Windows
>> systems.
>>
>> However, I think this also broke something (or at least inadvertently
>> changed something) related to the usage of "SUBST" (as in 'SUBST G:\
>> D:\Development\TestWC'). The usage of "subst" is not mentioned
>> anywhere in PR#47630, so perhaps the compatibility with / impact on
>> subst was overlooked? I'm not an expert at all, but from what I could
>> find with a quick internet search, "subst" is very old (from MSDOS
>> times), and it seems it's not the same as a junction (? not sure about
>> the relation between subst and reparse points in general). In any
>> case, it's not clear to me that the fix for PR#47630 intended to
>> change something for "subst".
>>
>
> That's very likely, and this can be researched further to dig up exactly
> how
> subst is handled. (As you mention, it hasn't been common since the
> introduction
> of proper junction and directory/file symlinks.)
>
> It appears that the root directory is always problematic for an svn
> checkout
> (actually, most checkouts were problematic on svn when junctions or
> symlinks
> were in play on windows.) Looks like some additional special handling is
> going
> to be required even with queries on c:\ (a real, not a substituted volume
> root.)
>
> Can you successfully `svn co {repos} /` on linux? Hoping to understand the
> scope of the issue.
>
>
>


apr-root-stat-test.patch
Description: Binary data


Re: Subversion reports status fault on substituted drive

2020-04-09 Thread William A Rowe Jr
On Thu, Apr 9, 2020 at 8:01 AM Johan Corveleyn  wrote:

> On Thu, Apr 9, 2020 at 1:20 PM Nick Kew  wrote:
> > >
> > > The below report about a problem with Subversion working on 'subst'ed
> > > drives (Windows), seemed to be related to a change in APR 1.7.0
> > > (https://svn.apache.org/r1855950),
> >
> > That fixed a bug PR#47630 that had been extensively discussed in
> > bugzilla and shows as having 22 watchers - that's a lot of interest!
> >
> > Does that discussion not throw any light on your issue?  For example,
> > Michael Schlenker's comments there refer to that bug affecting SVN.
>
> Thanks, I've read through that issue in Bugzilla now.
>
> It definitely seems like a useful improvement for proper support of
> (different types of) reparse points on Windows. I'm not saying this
> bugfix needs to be reverted. And indeed, Michael Schlenker's comments
> point to a specific problem affecting SVN (in combination with Windows
> Data Deduplication, which creates reparse points with tag
> IO_REPARSE_TAG_DEDUP). So in general this is a step forward, and will
> probably benefit SVN for use on these specific types of Windows
> systems.
>
> However, I think this also broke something (or at least inadvertently
> changed something) related to the usage of "SUBST" (as in 'SUBST G:\
> D:\Development\TestWC'). The usage of "subst" is not mentioned
> anywhere in PR#47630, so perhaps the compatibility with / impact on
> subst was overlooked? I'm not an expert at all, but from what I could
> find with a quick internet search, "subst" is very old (from MSDOS
> times), and it seems it's not the same as a junction (? not sure about
> the relation between subst and reparse points in general). In any
> case, it's not clear to me that the fix for PR#47630 intended to
> change something for "subst".
>

That's very likely, and this can be researched further to dig up exactly how
subst is handled. (As you mention, it hasn't been common since the
introduction
of proper junction and directory/file symlinks.)

It appears that the root directory is always problematic for an svn checkout
(actually, most checkouts were problematic on svn when junctions or symlinks
were in play on windows.) Looks like some additional special handling is
going
to be required even with queries on c:\ (a real, not a substituted volume
root.)

Can you successfully `svn co {repos} /` on linux? Hoping to understand the
scope of the issue.


Re: svn commit: r1872893 - /apr/apr/trunk/file_io/win32/filestat.c

2020-02-05 Thread William A Rowe Jr
So this is an open question... Is this backportable as a bug fix?

Both are errors as the reference to a specific file, but only one condition
represents a fatal error for a grep operation.

Should we wait for 1.8 or is this worth 'adjusting' in 1.7.1 asap?

I already have conflicting thoughts so wanted to raise it as a question.


On Thu, Jan 16, 2020, 13:20  wrote:

> Author: wrowe
> Date: Thu Jan 16 19:20:29 2020
> New Revision: 1872893
>
> URL: http://svn.apache.org/viewvc?rev=1872893=rev
> Log:
> Be more forceful in identifying EBADPATH
>
> Even when the path is first identified as a wildcard path.
>
> Modified:
> apr/apr/trunk/file_io/win32/filestat.c
>
> Modified: apr/apr/trunk/file_io/win32/filestat.c
> URL:
> http://svn.apache.org/viewvc/apr/apr/trunk/file_io/win32/filestat.c?rev=1872893=1872892=1872893=diff
>
> ==
> --- apr/apr/trunk/file_io/win32/filestat.c (original)
> +++ apr/apr/trunk/file_io/win32/filestat.c Thu Jan 16 19:20:29 2020
> @@ -32,6 +32,8 @@
>   */
>  static apr_status_t test_safe_name(const char *name)
>  {
> +apr_status_t rv = APR_SUCCESS;
> +
>  /* Only accept ':' in the second position of the filename,
>   * as the drive letter delimiter:
>   */
> @@ -41,13 +43,13 @@ static apr_status_t test_safe_name(const
>  while (*name) {
>  if (!IS_FNCHAR(*name) && (*name != '\\') && (*name != '/')) {
>  if (*name == '?' || *name == '*')
> -return APR_EPATHWILD;
> +rv = APR_EPATHWILD;
>  else
>  return APR_EBADPATH;
>  }
>  ++name;
>  }
> -return APR_SUCCESS;
> +return rv;
>  }
>
>  static apr_status_t free_localheap(void *heap) {
>
>
>


Re: svn commit: r1871082 - in /apr/apr/trunk: include/arch/win32/apr_arch_misc.h misc/win32/misc.c misc/win32/start.c

2019-12-12 Thread William A Rowe Jr
Awesome :)

Thanks!

On Mon, Dec 9, 2019, 04:22  wrote:

> Author: ivan
> Date: Mon Dec  9 10:22:08 2019
> New Revision: 1871082
>
> URL: http://svn.apache.org/viewvc?rev=1871082=rev
> Log:
> win32: Try to avoid loading shell32.dll whenever possible:
> 1. shell32.dll has about 19 dependencies.
> 2. shell32.dll depends on user32/gdi32.dll and loading them makes the
> process
>a "GUI" process:
>- this slows down process initialization and shutdown [1]
>- doesn't allow enabling mitigation to block win32k.sys
>  for process using APR.
> 3. shell32.dll is not available on Windows Nano Server
>
> The only used function from shell32.dll is CommandLineToArgW. It is used to
> parse command-line arguments in apr_app_initialize().
>
> The solution is twofold:
> 1. Delay loading shell32.dll.
> 2. Prefer to use api-ms-win-downlevel-shell32-l1-1-0.dll API Set instead of
>shell32.dll (Windows 8+)
>
> * include/arch/win32/apr_arch_misc.h
>   (apr_dlltoken_e): Add DLL_API_MS_WIN_DOWNLEVEL_SHELL32_L1_1_0.
>   (CommandLineToArgvW): Add late late dll func for CommandLineToArgvW.
>
> * misc/win32/misc.c
>   (load_dll_callback): Support API Sets.
>
> * misc/win32/start.c
>   (apr_app_initialize): Use apr_winapi_CommandLineToArgvW() instead of
>CommandLineToArgvW().
>
> [1]
> https://randomascii.wordpress.com/2018/12/03/a-not-called-function-can-cause-a-5x-slowdown/
>
> Modified:
> apr/apr/trunk/include/arch/win32/apr_arch_misc.h
> apr/apr/trunk/misc/win32/misc.c
> apr/apr/trunk/misc/win32/start.c
>
> Modified: apr/apr/trunk/include/arch/win32/apr_arch_misc.h
> URL:
> http://svn.apache.org/viewvc/apr/apr/trunk/include/arch/win32/apr_arch_misc.h?rev=1871082=1871081=1871082=diff
>
> ==
> --- apr/apr/trunk/include/arch/win32/apr_arch_misc.h (original)
> +++ apr/apr/trunk/include/arch/win32/apr_arch_misc.h Mon Dec  9 10:22:08
> 2019
> @@ -188,7 +188,10 @@ typedef enum {
>  DLL_SHSTDAPI = 4,  /* shell32  From ShellAPI.h  */
>  DLL_NTDLL = 5, /* ntdllFrom our real kernel */
>  DLL_IPHLPAPI = 6,  /* Iphlpapi From Iphlpapi.h  */
> -DLL_defined = 7/* must define as last idx_ + 1  */
> +DLL_API_MS_WIN_DOWNLEVEL_SHELL32_L1_1_0 = 7,
> +   /* api-ms-win-downlevel-shell32-l1-1-0.dll,
> +  fallback to shell32.dll   */
> +DLL_defined = 8/* must define as last idx_ + 1  */
>  } apr_dlltoken_e;
>
>  FARPROC apr_load_dll_func(apr_dlltoken_e fnLib, char *fnName, int
> ordinal);
> @@ -312,6 +315,11 @@ APR_DECLARE_LATE_DLL_FUNC(DLL_IPHLPAPI,
>  (InterfaceIndex, InterfaceName));
>  #define if_indextoname apr_winapi_if_indextoname
>
> +APR_DECLARE_LATE_DLL_FUNC(DLL_SHSTDAPI, LPWSTR *, STDAPICALLTYPE,
> CommandLineToArgvW, 0, (
> +LPCWSTR lpCmdLine,
> +int *pNumArgs),
> +(lpCmdLine, pNumArgs));
> +
>  #endif /* !defined(_WIN32_WCE) */
>
>  #endif  /* ! MISC_H */
>
> Modified: apr/apr/trunk/misc/win32/misc.c
> URL:
> http://svn.apache.org/viewvc/apr/apr/trunk/misc/win32/misc.c?rev=1871082=1871081=1871082=diff
>
> ==
> --- apr/apr/trunk/misc/win32/misc.c (original)
> +++ apr/apr/trunk/misc/win32/misc.c Mon Dec  9 10:22:08 2019
> @@ -136,18 +136,21 @@ apr_status_t apr_get_oslevel(apr_oslevel
>
>  typedef struct win32_late_dll_t {
>  INIT_ONCE control;
> +const apr_wchar_t *apiset_name;
>  const apr_wchar_t *dll_name;
>  HMODULE dll_handle;
>  } win32_late_dll_t;
>
>  static win32_late_dll_t late_dll[DLL_defined] = {
> -{INIT_ONCE_STATIC_INIT, L"kernel32", NULL},
> -{INIT_ONCE_STATIC_INIT, L"advapi32", NULL},
> -{INIT_ONCE_STATIC_INIT, L"mswsock", NULL},
> -{INIT_ONCE_STATIC_INIT, L"ws2_32", NULL},
> -{INIT_ONCE_STATIC_INIT, L"shell32", NULL},
> -{INIT_ONCE_STATIC_INIT, L"ntdll.dll", NULL},
> -{INIT_ONCE_STATIC_INIT, L"Iphplapi", NULL}
> +{INIT_ONCE_STATIC_INIT, NULL, L"kernel32", NULL},
> +{INIT_ONCE_STATIC_INIT, NULL, L"advapi32", NULL},
> +{INIT_ONCE_STATIC_INIT, NULL, L"mswsock", NULL},
> +{INIT_ONCE_STATIC_INIT, NULL, L"ws2_32", NULL},
> +{INIT_ONCE_STATIC_INIT, NULL, L"shell32", NULL},
> +{INIT_ONCE_STATIC_INIT, NULL, L"ntdll.dll", NULL},
> +{INIT_ONCE_STATIC_INIT, NULL, L"Iphplapi", NULL},
> +{INIT_ONCE_STATIC_INIT, L"api-ms-win-downlevel-shell32-l1-1-0.dll",
> + L"shell32", NULL}
>  };
>
>  static BOOL WINAPI load_dll_callback(PINIT_ONCE InitOnce,
> @@ -156,7 +159,15 @@ static BOOL WINAPI load_dll_callback(PIN
>  {
>  win32_late_dll_t *dll = Parameter;
>
> -dll->dll_handle = LoadLibraryW(dll->dll_name);
> +/* Try api set dll first if defined. */
> +if (dll->apiset_name) {
> +dll->dll_handle = LoadLibraryExW(dll->apiset_name, NULL,
> + LOAD_LIBRARY_SEARCH_SYSTEM32);
> +}
> +
> +

Re: svn commit: r1868502 - /apr/apr/trunk/atomic/win32/apr_atomic64.c

2019-11-01 Thread William A Rowe Jr
On Fri, Nov 1, 2019 at 1:45 AM Ivan Zhakov  wrote:

> On Wed, 30 Oct 2019 at 13:18, Yann Ylavic  wrote:
> >
> > On Wed, Oct 30, 2019 at 4:37 AM William A Rowe Jr 
> wrote:
> > >
> > > On Wed, Oct 16, 2019 at 5:11 AM  wrote:
> > >>
> > >> Author: ivan
> > >> Date: Wed Oct 16 10:10:59 2019
> > >> New Revision: 1868502
> > >>
> > >> URL: http://svn.apache.org/viewvc?rev=1868502=rev
> > >> Log:
> > >> * atomic/win32/apr_atomic64.c
> > >>   (apr_atomic_read64): Use direct memory read when compiled for
> x86_x64, since
> > >>64-bit reads are atomic in 64-bit Windows [1].
> > >>
> > >> +/*
> https://docs.microsoft.com/en-us/windows/win32/sync/interlocked-variable-access
> > >> + * "Simple reads and writes to properly aligned 64-bit variables
> are atomic
> > >> + * on 64-bit Windows."*/
> > >> +return *mem;
> > >
> > > Where are we[1] ensuring *mem is aligned on an 8 byte boundary?
> >
> > Since mem is apr_uint64_t, unless the caller is playing nasty casts we
> > should be good, I think.
>

I was envisioning more packed struct alignments...


> +1.
>
> Also InterlockedCompareExchange64() as all other interlocked function
> requires aligned data [1]. So APR already assumes that data is
> properly aligned.
>
> [1]
> https://docs.microsoft.com/en-us/windows/win32/api/winnt/nf-winnt-interlockedcompareexchange64


Agreed. It would be good to leave a breadcrumb in the doxygen that we
require the pointers are using correct word alignment, and even the patch
I hinted at wouldn't be improved given the ICEx docs.


Re: svn commit: r1868502 - /apr/apr/trunk/atomic/win32/apr_atomic64.c

2019-10-29 Thread William A Rowe Jr
On Wed, Oct 16, 2019 at 5:11 AM  wrote:

> Author: ivan
> Date: Wed Oct 16 10:10:59 2019
> New Revision: 1868502
>
> URL: http://svn.apache.org/viewvc?rev=1868502=rev
> Log:
> * atomic/win32/apr_atomic64.c
>   (apr_atomic_read64): Use direct memory read when compiled for x86_x64,
> since
>64-bit reads are atomic in 64-bit Windows [1].
>
> +/*
> https://docs.microsoft.com/en-us/windows/win32/sync/interlocked-variable-access
> + * "Simple reads and writes to properly aligned 64-bit variables are
> atomic
> + * on 64-bit Windows."*/
> +return *mem;
>

Where are we[1] ensuring *mem is aligned on an 8 byte boundary? It should
be
straightforward to guard that (mem & 7) requires the legacy behavior.

[1] Of course sparc and similar just throw sigbus, but this is intel.


Re: apr_stat() sometimes returning APR_INCOMPLETE (70008) on Windows 10

2019-08-02 Thread William A Rowe Jr
Looking at the Apache Perl dev@ traffic, this is obviously popping up more
and more often. It is no longer just a virus scanner issue, but a
restriction to some common WinNT security models in terms of inspecting
file permissions.

While modules should just be asking for APR_FSTAT_MIN scope most of the
time, or ready to accept that APR_EINCOMPLETE had still provided sufficient
answers, here we should probably start coping with less-than-complete Unix
stat lookups on win32, because the new model won't let us ask for the group
and world permissions.

This can be fixed any way we like in apr-2... Suggestions on acceptable
tweaks in our apr-1 compatibility model?






On Tue, Jul 16, 2019, 08:10 Steve Hay  wrote:

> I'm in the process of preparing a new mod_perl release and have run
> into a few test failures on Windows 10 which are caused by apr_stat()
> sometimes returning APR_INCOMPLETE (70008).
>
> The call stack from mod_perl into libapr-1.dll is here:
>
> > libapr-1.dll!more_finfo(apr_finfo_t * finfo, const void * ufile, int
> wanted, int whatfile) Line 403 C
>   libapr-1.dll!apr_file_info_get(apr_finfo_t * finfo, int wanted,
> apr_file_t * thefile) Line 561 C
>   libapr-1.dll!resolve_ident(apr_finfo_t * finfo, const char * fname,
> int wanted, apr_pool_t * pool) Line 161 C
>   libapr-1.dll!apr_stat(apr_finfo_t * finfo, const char * fname, int
> wanted, apr_pool_t * pool) Line 618 C
>   Finfo.dll!mpxs_APR__Finfo_stat(interpreter * my_perl, const char *
> fname, int wanted, sv * p_sv) Line 25 C
>   Finfo.dll!XS_APR__Finfo_stat(interpreter * my_perl, cv * cv) Line 43 C
>
> The return at the end of more_finfo() is
>
> return ((wanted & ~finfo->valid) ? APR_INCOMPLETE : APR_SUCCESS);
>
> in which I have wanted = 7536640 and finfo->valid = 1306992, so
> (wanted & ~finfo->valid) = 6291456, so it returns APR_INCOMPLETE
> (70008).
>
> The file in question is just a regular plain text file. In fact, it's
> httpd's own httpd.conf file, which is generated by the mod_perl test
> suite.
>
> I can reproduce the problem using a short perl program:
>
> use strict;
> use warnings FATAL => 'all';
>
> use APR::Finfo ();
> use APR::Pool ();
> use APR::Const -compile => qw(FINFO_NORM);
>
> my $file = "D:\\Dev\\Temp\\mp2\\mod_perl-2.0.11-rc1\\t\\conf\\httpd.conf";
>
> my $pool = APR::Pool->new();
> my $finfo = APR::Finfo::stat($file, APR::Const::FINFO_NORM, $pool);
>
> Weirdly, if I simply make a copy of the httpd.conf file and point the
> above program at that copy instead of the original then it works fine.
>
> I'm only getting this on Windows 10. If I run the same build of
> everything on Windows 7 then everything is fine. I've built everything
> from scratch with VS2019. I get the same behaviour with VS2015.
>
> I'm using apr-1.7.0 / apr-util-1.6.1. Is it worth trying the latest
> dev versions?
>
> Does anyone have any ideas what could be going wrong here?
>
> I tried doing a build of APR with -DAPR_BUILD_TESTAPR=on but the build
> fails:
>
> [ 54%] Linking C executable testall.exe
> LINK Pass 1: command
>
> "C:\PROGRA~2\MICROS~1\201916~1.1\PROFES~1\VC\Tools\MSVC\1421~1.277\bin\Hostx64\x64\link.exe
> /nologo @CMakeFiles\testall.dir\objects1.rsp /out:testall.exe
> /implib:testall.lib /pdb:D:\Dev\Temp\test\apr-1.7.0\testall.pdb
> /version:0.0 /machine:x64 /debug /INCREMENTAL /subsystem:console
> libapr-1.lib ws2_32.lib mswsock.lib rpcrt4.lib kernel32.lib user32.lib
> gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib
> comdlg32.lib advapi32.lib /MANIFEST
> /MANIFESTFILE:CMakeFiles\testall.dir/intermediate.manifest
> CMakeFiles\testall.dir/manifest.res" failed (exit code 1120) with the
> following output:
> abts.c.obj : error LNK2001: unresolved external symbol testencode
> testatomic.c.obj : error LNK2019: unresolved external symbol
> __imp_apr_atomic_read64 referenced in function busyloop_read64
> testatomic.c.obj : error LNK2019: unresolved external symbol
> __imp_apr_atomic_set64 referenced in function busyloop_set64
> testatomic.c.obj : error LNK2019: unresolved external symbol
> __imp_apr_atomic_add64 referenced in function busyloop_add64
> testatomic.c.obj : error LNK2019: unresolved external symbol
> __imp_apr_atomic_sub64 referenced in function busyloop_sub64
> testatomic.c.obj : error LNK2019: unresolved external symbol
> __imp_apr_atomic_inc64 referenced in function busyloop_inc64
> testatomic.c.obj : error LNK2019: unresolved external symbol
> __imp_apr_atomic_dec64 referenced in function busyloop_dec64
> testatomic.c.obj : error LNK2019: unresolved external symbol
> __imp_apr_atomic_cas64 referenced in function busyloop_cas64
> testatomic.c.obj : error LNK2019: unresolved external symbol
> __imp_apr_atomic_xchg64 referenced in function busyloop_xchg64
> testall.exe : fatal error LNK1120: 9 unresolved externals
> NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual
> Studio\2019
> 16.1\Professional\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin\cmake.exe"'
> : return code 

Re: Proposal: apr-tools project

2019-07-27 Thread William A Rowe Jr
On Thu, Jul 25, 2019 at 8:26 AM Graham Leggett  wrote:

> On 25 Jul 2019, at 14:26, Nick Kew  wrote:
>
> > Would that be purely in C, or could it be a mix, perhaps including
> scripting languages?
>
> Purely in C.
>
> Exposing APR to other languages through something like swig also sounds
> useful, but that’s not my goal here.
>

+1 - if we correct apr-2 entirely, it is already structured as its own
proper c++ implementation. We got awfully close at apr-1. Other bindings,
swig or otherwise, are useful but not what you were describing, and
probably have a different audience of champions/maintainers to drive it.
Since it the same implementation for different programming consumers it
would be a good fit right here under this PMC given enough volunteers. Very
much like the Apache Logging Project (log4j, log4cxx etc.)


> > First thought: harmless, but how useful is it?  Do you have evidence of
> a userbase who
> > are insufficiently served by existing tools in a similar space?
>
> Yes, me. :)
>

:) I've spent the last month living in mingw64/msys64, all new to me since
I was a unxutils fan, but that port is very lethargic, while mingw paths
are a little absurd in the transitions between PS/cmd and faux-linux. And
most of my real life time on Windows is now in a Windows Services for Linux
Ubuntu port (similar path quirks although it is shockingly comprehensive.)

Having true native ports for many of these things would be awesome...

>From time to time I need to safely manipulate an URL in a shell script, or
> I need to unescape something in one type of encoding and reescape it in
> another, and deploying a sledgehammer scripting language to do that little
> thing becomes tedious.
>
> As for the dbd tool, a while back I had a need to read data out of mysql
> and write it to postgresql (or other way around, don’t remember) and the
> native tools made this one major headache. You needed to be on the same
> machine running as the same user as the database. For crying out loud, why
> is this hard.
>

+1, this was something rbb and I bantered about. In the midst of apache-2
that really wasn't something we had even more cycles to move on. I'd be
happy to port my witch.exe (deviant of bin/which) into such an effort,
since it allows alt PATH searches (such as search for a .h file on the
INCLUDE pathlist.) With the sed implementation donated under AL 2.0 to
httpd, that would be another easy port. An APR awk port supporting utf-8 on
Windows (natively Unicode) would be awesome and straightforward.

>> Second question is structure. Part of APR? Alongside APR?
> >
> > Either within APR or your external effort: IMHO it would be excessively
> bureaucratic
> > to put it within the ASF but separate from APR.
>
> What I meant was part of the apr package, or alongside the apr package (as
> apr-util is alongside the apr package).
>

It doesn't make much sense as part of the apr build. A [group of]
package[s] with the hard dependency on apr is more sensible.

Whether the apr project wants to own this, or as with our many java
brethren at the ASF, it is a self-managing project is another question.
Obviously there is some PMC overlap, but this is well beyond the original
remit of the apr project and will attract a different audience of
developers and consumers.

I'd think that the APT Project (Apache Portable Tools?) would be a good
community to kick off, and it should be independent and self organizing. It
should follow its own versioning and release schema, since what is best
practice for an API might not map 1:1 for a toolchain.

Fastest track, kick off a labs demo. Then one or two paths as a community
unfolds around the code, either through the incubator (great for new and
less familiar contributors) or sponsored by the APR project. Either way,
several mentors and build a group of committees and it should be very
successful.

I mentioned Windows up top, but I've had just as many headaches with small
variances in toolchains between Linux, AIX, HPUX, BSD(s) and Solaris. This
is a great proposal IMO.


Resolutions to APR_POOL_DEBUGging headaches?

2019-07-23 Thread William A Rowe Jr
I'm sorry I haven't had the summertime cycles to dig into this, but am
wondering on the cusp of an httpd 2.4.40, we've learned a bunch about
possible apr pool and especially debug traps that meta

What code remains to be considered for inclusion in an apr 1.7.1?

What is apr 1.8 waiting for, if we think we can slightly twist things?

Do we agree you can't jump between APR_POOL_DEBUG and retain ABI to the
non-debug build?

Have we eliminated the cause for APR 1.x folks to ship a release based on
APR_POOL_DEBUG?

What proposals are we still working to commit to apr trunk, for deep
inspection within snapshot/dev tooling builds?

Which other questions did I overlook?

Maybe a whiteboard (wiki page) for all the proposals under consideration,
or assemble these under STATUS on trunk?


Re: apr_pool_join() and APR_POOL_DEBUG

2019-07-17 Thread William A Rowe Jr
On Tue, Jul 16, 2019, 23:54 Rainer Jung  wrote:

> Am 17.07.2019 um 01:30 schrieb Branko Čibej:
> > On 16.07.2019 23:18, Rainer Jung wrote:
> >> I had some need for using APR_POOL_DEBUG today and ran into a
> >> situation where pool lifetimes needed a hint using apr_pool_join().
> >> That is all documented and fine, except that I was surprised to see,
> >> that apr_pool_join() doesn't work unless the application itself is
> >> also compiled with APR_POOL_DEBUG. I think that's not the intend, one
> >> should be able to replace the apr library against a debug one during
> >> lifetime.
> >>
> >> So IMHO apr_pool_join() in apr has to be a noop function when
> >> APR_POOL_DEBUG is not defined, so that the function call is contained
> >> in the application.
> >>
> >> Currently when the app is compiled without APR_POOL_DEBUG, then in
> >> apr.h we only have
> >>
> >> #if APR_POOL_DEBUG || defined(DOXYGEN)
> >> ...
> >> #else /* APR_POOL_DEBUG or DOXYGEN */
> >>
> >> #ifdef apr_pool_join
> >> #undef apr_pool_join
> >> #endif
> >> #define apr_pool_join(a,b)
> >> ...
> >> #endif /* APR_POOL_DEBUG or DOXYGEN */
> >>
> >> so apr_pool_join() would be an empty macro.
> >>
> >> Other pool debug functions seem to share the same fate:
> >>
> >> - apr_pool_owner_set()
> >> - apr_pool_find()
> >> - apr_pool_num_bytes()
> >> - apr_pool_lock()
> >> - and as mentioned apr_pool_join()
> >>
> >> Any opinion on whether we should introduce empty implementations
> >> instead of empty macros for those all?
> >
> > What should apr_pool_join and the other pool debugging functions do if
> > you're not debugging pools? They should be no-ops. So replacing them
> > with empty macros makes sense.
> >
> > Or are you arguing that two libapr variants, one compiles with
> > APR_POOL_DEBUG and one without, should be ABI- compatible?
>
> As far as I understand it, one should be able to replace the normal apr
> library by a debug apr library at runtime and get pool debugging. There
> should be no need to provide a apr debug compilation of the app itself.
>
> If an app needs calls to apr_pool_join() to guide the debugging, they
> must be in the source code of the app and must not get compiled out.
>
> No-op would only be OK, if the app they are function calls to no-op
> functions, but not if they get compiled out.
>
> So yes, I think it must be ABI compatible and except for the above 5
> functions it is.
>
> Regards,
>
> Rainer
>

But diagnostic macros are rarely ABI compliant. C++ code suffers the same
pain. I don't believe we can or should support ABI between debug and
release models with respect to these toggles, because we want deep
introspection into the structures with no assurance of equivilance across
opaque structures.

What are implemented as functions must remain ABI compliant and we can
implement no-ops for the non-debug runtime.

Stefan Eising identified a number of headaches with pool debugging fairly
recently, I'm all for fixing this, but I'm not altogether concerned that
dropping in the debug binary of apr for code compiled against non-debug apr
should have any effect whatsoever.


Re: apr_stat() sometimes returning APR_INCOMPLETE (70008) on Windows 10

2019-07-16 Thread William A Rowe Jr
On Tue, Jul 16, 2019 at 8:10 AM Steve Hay 
wrote:

> I'm in the process of preparing a new mod_perl release and have run
> into a few test failures on Windows 10 which are caused by apr_stat()
> sometimes returning APR_INCOMPLETE (70008).
>
> I'm only getting this on Windows 10. If I run the same build of
> everything on Windows 7 then everything is fine. I've built everything
> from scratch with VS2019. I get the same behaviour with VS2015.
>
> I'm using apr-1.7.0 / apr-util-1.6.1. Is it worth trying the latest
> dev versions?
>

Would you retest with apr-1.6.5? Minor changes to the handling of symbolic
links (junction/reparse points) on Win32 in 1.7.0 may be to blame.

(Yes, I understand that you are not looking at a symlink in this case.)


Re: The case for apr-util-2.0

2019-06-12 Thread William A Rowe Jr
On Thu, Jun 6, 2019 at 4:15 AM Graham Leggett  wrote:

> On 06 Jun 2019, at 10:38, Branko Čibej  wrote:
>
> $ lsb_release -a
> No LSB modules are available.
> Distributor ID: Ubuntu
> Description: Ubuntu 18.04.2 LTS
> Release: 18.04
> Codename: bionic
> $ apt-cache search aprutil
> libaprutil1 - Apache Portable Runtime Utility Library
> libaprutil1-dbd-sqlite3 - Apache Portable Runtime Utility Library -
> SQLite3 Driver
> libaprutil1-dbg - Apache Portable Runtime Utility Library - Debugging
> Symbols
> libaprutil1-dev - Apache Portable Runtime Utility Library - Development
> Headers
> libaprutil1-ldap - Apache Portable Runtime Utility Library - LDAP Driver
> libaprutil1-dbd-mysql - Apache Portable Runtime Utility Library - MySQL
> Driver
> libaprutil1-dbd-odbc - Apache Portable Runtime Utility Library - ODBC
> Driver
> libaprutil1-dbd-pgsql - Apache Portable Runtime Utility Library -
> PostgreSQL Driver
> $ apt-cache show libaprutil1
> Package: libaprutil1
> [...]
> Depends: libapr1 (>= 1.6.2), libc6 (>= 2.25), libdb5.3, libexpat1 (>=
> 2.0.1), libgdbm5 (>= 1.14), libssl1.1 (>= 1.1.0)
> […]
>
>
> That looks like a packaging bug. This is on CentOS7:
>
> [minfrin@chandler ~]$ ldd /usr/lib64/libapr-1.so.0.6.3
> linux-vdso.so.1 =>  (0x7ffee65c1000)
> libuuid.so.1 => /lib64/libuuid.so.1 (0x7f8d24138000)
> librt.so.1 => /lib64/librt.so.1 (0x7f8d23f3)
> libcrypt.so.1 => /lib64/libcrypt.so.1 (0x7f8d23cf9000)
> libpthread.so.0 => /lib64/libpthread.so.0 (0x7f8d23add000)
> libdl.so.2 => /lib64/libdl.so.2 (0x7f8d238d9000)
> libc.so.6 => /lib64/libc.so.6 (0x7f8d2350c000)
> /lib64/ld-linux-x86-64.so.2 (0x7f8d24578000)
> libfreebl3.so => /lib64/libfreebl3.so (0x7f8d23309000)
>
> [minfrin@chandler ~]$ ldd /usr/lib64/libaprutil-1.so.0
> linux-vdso.so.1 =>  (0x7ffd83171000)
> libexpat.so.1 => /lib64/libexpat.so.1 (0x7f2813501000)
> libapr-1.so.0 => /lib64/libapr-1.so.0 (0x7f28132c6000)
> libuuid.so.1 => /lib64/libuuid.so.1 (0x7f28130c1000)
> librt.so.1 => /lib64/librt.so.1 (0x7f2812eb9000)
> libcrypt.so.1 => /lib64/libcrypt.so.1 (0x7f2812c82000)
> libpthread.so.0 => /lib64/libpthread.so.0 (0x7f2812a66000)
> libdl.so.2 => /lib64/libdl.so.2 (0x7f2812862000)
> libc.so.6 => /lib64/libc.so.6 (0x7f2812495000)
> /lib64/ld-linux-x86-64.so.2 (0x7f2813962000)
> libfreebl3.so => /lib64/libfreebl3.so (0x7f2812292000)
>
> I specifically object to libexpat, libssl, libdb and libgdm being on
> this dependency list ... the latter two are just bloat, but the former
> two create all sorts of problems for any project that happens to use
> either of them directly.
>
> +1.
>
> Looking at the archeology, this problem was identified years ago, and each
> successive addition has been properly modularised. What was needed was to
> go back and modularise the earliest additions.
>

 It seems the latest additions are the only problem on apr-2.0;

$ ldd libapr-2.so
linux-vdso.so.1 (0x7fc16a217000)
libuuid.so.1 => /lib64/libuuid.so.1 (0x7fc16a136000)
librt.so.1 => /lib64/librt.so.1 (0x7fc16a12c000)
libcrypt.so.1 => /lib64/libcrypt.so.1 (0x7fc16a0f)
libpthread.so.0 => /lib64/libpthread.so.0 (0x7fc16a0ce000)
libdl.so.2 => /lib64/libdl.so.2 (0x7fc16a0c8000)
libssl.so.1.1 => /opt/openssl111/lib/libssl.so.1.1 (0x7fc16a034000)
libcrypto.so.1.1 => /opt/openssl111/lib/libcrypto.so.1.1
(0x7fc169d48000)
libxml2.so.2 => /lib64/libxml2.so.2 (0x7fc169bde000)
libc.so.6 => /lib64/libc.so.6 (0x7fc169a18000)
/lib64/ld-linux-x86-64.so.2 (0x7fc16a218000)
libz.so.1 => /lib64/libz.so.1 (0x7fc1699fe000)
liblzma.so.5 => /lib64/liblzma.so.5 (0x7fc1699d5000)
libm.so.6 => /lib64/libm.so.6 (0x7fc169851000)

That would be a problem. Please fix, both yourself, and Nick. Also curious
where libz/lzma arrived from, I'm guessing libxml2, because libssl 1.1.1
build no longer includes compression by default.

We have a second problem, someone colored the names in a very inappropriate
way;

[wrowe@hub apr-2]$ ls *.so
apr_crypto_openssl-2.so
apr_crypto_openssl.so
apr_dbd_mysql-2.so
apr_dbd_mysql.so
apr_dbd_odbc-2.so
apr_dbd_odbc.so
apr_dbd_pgsql-2.so
apr_dbd_pgsql.so
apr_dbd_sqlite3-2.so
apr_dbd_sqlite3.so
apr_dbm_db-2.so
apr_dbm_db.so
apr_dbm_gdbm-2.so
apr_dbm_gdbm.so
apr_dbm_ndbm-2.so
apr_dbm_ndbm.so

There is zero excuse for coloring the names by apr -2 epoch which are
enclosed in a directory explicitly named 'apr-2/'. And absolutely absurd to
have two different aliases, period, which would then conflict with their -1
brethren. This needs to be reverted and solved as apr/apr_foo-2.so or as
apr-2/apr_foo.so.

Now for the real fun, with the exception of the borked -lssl/-crypto,
almost everything has successfully been isolated to their respective
sub-components. I'm sure some of you would be interested, this is a
shortlist of some autodetected components on our build on a modern Fedora;

[wrowe@hub apr-2]$ for f in *-2.so; do echo "$f 

Re: svn commit: r1860745 - /apr/apr/trunk/file_io/win32/dir.c

2019-06-12 Thread William A Rowe Jr
On Tue, Jun 11, 2019 at 5:38 PM William A Rowe Jr 
wrote:

> On Tue, Jun 11, 2019 at 1:44 PM Branko Čibej  wrote:
>
>>  We either reserve about 2x buffers for file name transliteration in heap
>> per thread, or we use the thread stack. As long as we trust that our utf-8
>> to ucs-2 logic is rock solid and the allocations and limits are correctly
>> coded, this continues to be a safe approach.
>>
>>
>> Apropos of that, for 2.0 we're about to or have already ditched support
>> for versions of Windows that do not have native UTF-8/UTF-16 conversions
>> (ah, yes ... Windows has finally moved from UCS-2 to UTF-16). Wouldn't this
>> be the right time to switch to using Windows' functions instead of staying
>> with our own? Especially since, with the transition to UTF-16, we have to
>> deal correctly with surrogate pairs, something our current code (IIRC)
>> doesn't do.
>>
>
> A bit of a misnomer, the code is full of references to ucs-2 w/surrogate
> pair
> support, the combo of these is utf-16. The comments can be refreshed to
> today's utf-16 nomenclature.
>

How strongly do we feel about the naming? I have the following patch to
commit if we want to observe current convention in utf-naming and deprecate
many (but not all) ucs references.
Index: file_io/win32/open.c
===
--- file_io/win32/open.c	(revision 1856754)
+++ file_io/win32/open.c	(working copy)
@@ -86,7 +86,7 @@
 }
 }
 
-if ((rv = apr_conv_utf8_to_ucs2(srcstr, , t, ))) {
+if ((rv = apr_conv_utf8_to_utf16(srcstr, , t, ))) {
 return (rv == APR_INCOMPLETE) ? APR_EINVAL : rv;
 }
 if (srcremains) {
@@ -127,7 +127,7 @@
 }
 }
 
-if ((rv = apr_conv_ucs2_to_utf8(srcstr, , t, ))) {
+if ((rv = apr_conv_utf16_to_utf8(srcstr, , t, ))) {
 return rv;
 }
 if (srcremains) {
@@ -168,7 +168,7 @@
 wfile = apr_palloc(pool, (r + n) * sizeof(apr_wchar_t));
 wcscpy(wfile, wpre);
 d = n;
-if (apr_conv_utf8_to_ucs2(file, , wfile + r, )) {
+if (apr_conv_utf8_to_utf16(file, , wfile + r, )) {
 return NULL;
 }
 for (ch = wfile + r; *ch; ++ch) {
Index: include/arch/win32/apr_arch_utf8.h
===
--- include/arch/win32/apr_arch_utf8.h	(revision 1856754)
+++ include/arch/win32/apr_arch_utf8.h	(working copy)
@@ -27,30 +27,32 @@
 
 /**
  * An APR internal function for fast utf-8 octet-encoded Unicode conversion
- * to the ucs-2 wide Unicode format.  This function is used for filename and 
- * other resource conversions for platforms providing native Unicode support.
+ * to the utf-16 Unicode with surrogate pairs format.  This function is used
+ * for filename and other resource conversions for platforms providing native
+ * Unicode support.
  *
  * @tip Only the errors APR_EINVAL and APR_INCOMPLETE may occur, the former
  * when the character code is invalid (in or out of context) and the later
  * when more characters were expected, but insufficient characters remain.
  */
-APR_DECLARE(apr_status_t) apr_conv_utf8_to_ucs2(const char *in, 
-apr_size_t *inbytes,
-apr_wchar_t *out, 
-apr_size_t *outwords);
+APR_DECLARE(apr_status_t) apr_conv_utf8_to_utf16(const char *in, 
+ apr_size_t *inbytes,
+ apr_wchar_t *out, 
+ apr_size_t *outwords);
 
 /**
- * An APR internal function for fast ucs-2 wide Unicode format conversion to 
- * the utf-8 octet-encoded Unicode.  This function is used for filename and 
- * other resource conversions for platforms providing native Unicode support.
+ * An APR internal function for fast utf-16 Unicode including surrogate pairs
+ * format conversion to the utf-8 octet-encoded Unicode.  This function is used
+ * for filename and other resource conversions for platforms providing native
+ * Unicode support.
  *
  * @tip Only the errors APR_EINVAL and APR_INCOMPLETE may occur, the former
- * when the character code is invalid (in or out of context) and the later
- * when more words were expected, but insufficient words remain.
+ * when the surrogate word is invalid (in or out of context) and the later
+ * when another surrogate word is expected, but insufficient words remain.
  */
-APR_DECLARE(apr_status_t) apr_conv_ucs2_to_utf8(const apr_wchar_t *in, 
-apr_size_t *inwords,
-char *out, 
-apr_size_t *outbytes);
+APR_DECLARE(apr_status_t) apr_conv

Re: svn commit: r1860745 - /apr/apr/trunk/file_io/win32/dir.c

2019-06-11 Thread William A Rowe Jr
On Tue, Jun 11, 2019 at 1:44 PM Branko Čibej  wrote:

>  We either reserve about 2x buffers for file name transliteration in heap
> per thread, or we use the thread stack. As long as we trust that our utf-8
> to ucs-2 logic is rock solid and the allocations and limits are correctly
> coded, this continues to be a safe approach.
>
>
> Apropos of that, for 2.0 we're about to or have already ditched support
> for versions of Windows that do not have native UTF-8/UTF-16 conversions
> (ah, yes ... Windows has finally moved from UCS-2 to UTF-16). Wouldn't this
> be the right time to switch to using Windows' functions instead of staying
> with our own? Especially since, with the transition to UTF-16, we have to
> deal correctly with surrogate pairs, something our current code (IIRC)
> doesn't do.
>

A bit of a misnomer, the code is full of references to ucs-2 w/surrogate
pair
support, the combo of these is utf-16. The comments can be refreshed to
today's utf-16 nomenclature.

Today's logic remains correct, and of course does the correct thing,
because
an unpaired utf-8 surrogate value would be very broken and even possibly a
security issue, much as decoding other invalid utf-8 bytestreams proved to
be.

If you want to look at win32 api's, feel free to benchmark; though I doubt
it
would outperform the current implementation.


Re: svn commit: r1860980 - /apr/apr/trunk/build/crypto.m4

2019-06-11 Thread William A Rowe Jr
I'm totally confused, these libs have been installed across three paths?

If not, isn't that an autoconf task to link to the one and only one
preferred target on rebuilds, in spite of later package adds?



On Mon, Jun 10, 2019 at 3:47 PM  wrote:

> Author: minfrin
> Date: Mon Jun 10 20:47:36 2019
> New Revision: 1860980
>
> URL: http://svn.apache.org/viewvc?rev=1860980=rev
> Log:
> Allow NSS to be found on MacPorts installations.
>
> Modified:
> apr/apr/trunk/build/crypto.m4
>
> Modified: apr/apr/trunk/build/crypto.m4
> URL:
> http://svn.apache.org/viewvc/apr/apr/trunk/build/crypto.m4?rev=1860980=1860979=1860980=diff
>
> ==
> --- apr/apr/trunk/build/crypto.m4 (original)
> +++ apr/apr/trunk/build/crypto.m4 Mon Jun 10 20:47:36 2019
> @@ -200,7 +200,7 @@ AC_DEFUN([APU_CHECK_CRYPTO_NSS], [
>  elif test "x$withval" != "x"; then
>
>nss_CPPFLAGS="-I$withval/include/nss -I$withval/include/nss3
> -I$withval/include/nspr -I$withval/include/nspr4 -I$withval/include
> -I$withval/../public"
> -  nss_LDFLAGS="-L$withval/lib "
> +  nss_LDFLAGS="-L$withval/lib -L$withval/lib/nss -L$withval/lib/nspr "
>
>APR_ADDTO(CPPFLAGS, [$nss_CPPFLAGS])
>APR_ADDTO(LDFLAGS, [$nss_LDFLAGS])
>
>
>


Re: svn commit: r1860745 - /apr/apr/trunk/file_io/win32/dir.c

2019-06-11 Thread William A Rowe Jr
On Tue, Jun 11, 2019 at 9:14 AM William A Rowe Jr 
wrote:

> On Tue, Jun 11, 2019 at 4:15 AM Branko Čibej  wrote:
>
>> On 07.06.2019 21:58, William A Rowe Jr wrote:
>> > I think the optimal way is to allocate a pair of apr thread-specific
>> > wchar buffers in each thread's pool on startup, and use those
>> > exclusively per-thread for wchar translations. We could be looking at
>> > 64k/thread exclusively for name translation, but it doesn't seem
>> > unreasonable.
>> >
>> > The alternative is to continue to use stack, we surely don't want to
>> > lock on acquiring or allocating name translation buffers. /shrug
>>
>> Since this is Windows, and there's no embedded Windows environment left
>> that I'm aware of that's still alive, we can continue with using the
>> stack ... but wouldn't it be so much better to alloca() the required
>> size instead of blindly burning through 64k every time? Obviously that
>> means counting the characters first.
>>
>
> I don't think there is any need to do so. A name buffer needs 32k (right
> now
> we limit that to 8k, but if we wanted to satisfy the OS-acceptable pathname
> length, we would blow that out 4x the current arbitrary limit.) For a
> rename,
> make that 2x buffers. But there is no benefit to wasting cycles determining
> the string length ahead of allocation, because the very next call can run
> right past that limit and hit the wall on available stack. Demanding that
> there
> always be a potential 32k runway of available stack doesn't seem excessive.
>
> The point to the stack is that it contracts immediately on return. So we
> aren't
> burning through a 64k buffer - we are ensuring that we have been topped-up
> to 64k remaining. The targets of these calls are all Win32 API
> invocations,
> we are never nesting them inside further big-buffer allocations of our own.
> What might happen in ntdll we have little control over.
>
>  We either reserve about 2x buffers for file name transliteration in heap
> per thread, or we use the thread stack. As long as we trust that our utf-8
> to ucs-2 logic is rock solid and the allocations and limits are correctly
> coded, this continues to be a safe approach. The 32k/64k are immediately
> given back in the current stack-based approach, but would simply be boat
> anchors most of the time if they are set aside in heap.
>
> I'm +/-0 on switching from stack to heap for this particular transformation
> but welcome good suggestions.
>

I failed to call this out, but guessing that we agree that if we preserve
wide
character string results (without discarding them after a single API call),
using the string size or moving towards counted byte string representation
makes a whole lot more sense for longer-term heap allocations.


Re: svn commit: r1860745 - /apr/apr/trunk/file_io/win32/dir.c

2019-06-11 Thread William A Rowe Jr
On Tue, Jun 11, 2019 at 4:15 AM Branko Čibej  wrote:

> On 07.06.2019 21:58, William A Rowe Jr wrote:
> > I think the optimal way is to allocate a pair of apr thread-specific
> > wchar buffers in each thread's pool on startup, and use those
> > exclusively per-thread for wchar translations. We could be looking at
> > 64k/thread exclusively for name translation, but it doesn't seem
> > unreasonable.
> >
> > The alternative is to continue to use stack, we surely don't want to
> > lock on acquiring or allocating name translation buffers. /shrug
>
> Since this is Windows, and there's no embedded Windows environment left
> that I'm aware of that's still alive, we can continue with using the
> stack ... but wouldn't it be so much better to alloca() the required
> size instead of blindly burning through 64k every time? Obviously that
> means counting the characters first.
>

I don't think there is any need to do so. A name buffer needs 32k (right now
we limit that to 8k, but if we wanted to satisfy the OS-acceptable pathname
length, we would blow that out 4x the current arbitrary limit.) For a
rename,
make that 2x buffers. But there is no benefit to wasting cycles determining
the string length ahead of allocation, because the very next call can run
right past that limit and hit the wall on available stack. Demanding that
there
always be a potential 32k runway of available stack doesn't seem excessive.

The point to the stack is that it contracts immediately on return. So we
aren't
burning through a 64k buffer - we are ensuring that we have been topped-up
to 64k remaining. The targets of these calls are all Win32 API invocations,
we are never nesting them inside further big-buffer allocations of our own.
What might happen in ntdll we have little control over.

 We either reserve about 2x buffers for file name transliteration in heap
per thread, or we use the thread stack. As long as we trust that our utf-8
to ucs-2 logic is rock solid and the allocations and limits are correctly
coded, this continues to be a safe approach. The 32k/64k are immediately
given back in the current stack-based approach, but would simply be boat
anchors most of the time if they are set aside in heap.

I'm +/-0 on switching from stack to heap for this particular transformation
but welcome good suggestions.
.


Re: svn commit: r1860745 - /apr/apr/trunk/file_io/win32/dir.c

2019-06-07 Thread William A Rowe Jr
I think the optimal way is to allocate a pair of apr thread-specific wchar
buffers in each thread's pool on startup, and use those exclusively
per-thread for wchar translations. We could be looking at 64k/thread
exclusively for name translation, but it doesn't seem unreasonable.

The alternative is to continue to use stack, we surely don't want to lock
on acquiring or allocating name translation buffers. /shrug





On Fri, Jun 7, 2019 at 6:27 AM Greg Stein  wrote:

> On Fri, Jun 7, 2019 at 6:02 AM  wrote:
> >...
>
>> \@@ -114,116 +93,73 @@ APR_DECLARE(apr_status_t) apr_dir_read(a
>>  {
>>  apr_status_t rv;
>>  char *fname;
>> +apr_wchar_t wdirname[APR_PATH_MAX];
>>
>
> That's a crazy huge stack frame. I recognize we were doing this before
> this commit, but is it advisable? Should it be corrected?
>
> Cheers,
> -g
>
>


Website refresh

2019-05-29 Thread William A Rowe Jr
I've updated the mail-archive links, and updated all that I could
to prefer https://. There are still likely dead https:// links that I didn't
review at all. A second pair of eyes through the current contents
of https://apr.apache.org/ would be helpful.


[results] [vote] Win32 Decision Point

2019-05-22 Thread William A Rowe Jr
As this vote has run 4 weeks, and has 7 votes in favor (including mine),
none opposed, announcing that;

Please drop 8-bit and focus only on utf-8 resource names on Win32.

is now the official policy for APR 2.0 branch. Others are welcome to help
detangle and eliminate the ANSI win32 code paths in these coming weeks.



On Wed, Apr 24, 2019 at 2:31 PM William A Rowe Jr 
wrote:

> Some 17 years later we are at a crossroads, because the win32 code
> is somewhat illegible and harder to maintain due to the ANSI-vs-UTF8,
> Win9x-vs-NT code paths.
>
> NT won. The only remaining question is how many apr consumers are
> leveraging ANSI-specific builds for local code page semantics, vs how
> many are willing to treat all system resources as utf-8 names, and for
> ANSI, willing to live on the 1.x branch in perpetuity? These are builds
> that explicitly toggle ANSI in spite of whatever OS the binary runs on.
>
> So the vote is pretty simple, I propose to strip all ANSI 8-bit logic from
> the apr (2.0) trunk/ and leave only the utf8->wide char logic remaining.
> Committers and community both, please choose one below,
>
> [ ] Please retain the ANSI logic in APR 2.0 on Win32
>
> [ ] Please drop 8-bit and focus only on utf-8 resource names on Win32.
>
> Will leave this question open a full 10 days to get the widest sampling
> of opinions.
>
>
>


Re: Supported Windows versions for APR 2.0 (was Re: [PATCH] Optimize apr_file_info_get(APR_FINFO_SIZE) on Windows)

2019-05-21 Thread William A Rowe Jr
On Tue, May 21, 2019 at 2:32 PM Ivan Zhakov  wrote:

> >
> > More specifically, I would like to:
> >
> > 1. Use native Slim Reader/Writer (SRW) locks [1] for apr_rwlock.
> >
> >Windows SRW locks are extremely performant (100x times) and also the
> >current APR implementation has known bugs [2].
> >
> >Although the basic SRW API is available since Windows Vista, the Try*
> >functions that required for the implementation are only available
> since
> >Windows 7 [3].
> >
> Done in r1859584.
>
> > 2. Use SetThreadErrorMode() [4] instead of SetErrorMode() because
> >SetErrorMode() is not thread safe.
> >
> >This new API is available since Windows 7.
> >
> Done in r1859519.
>
> > 3. Use BCrypt API instead of CryptoAPI as an entropy source.
> >
> >Although the BCrypt API by itself is available starting from Windows
> Vista,
> >an important `BCRYPT_USE_SYSTEM_PREFERRED_RNG` flag can only be
> >used on Windows 7 and later.
> >
> Done in r1859608.
>
> > 4. Use native one-time initialization [5] for apr_thread_once_t.
> >
> >The current Win32 implementation has a problem that it can return
> before
> >the other thread completes initialization.
> >
> >This particular API is available starting from Windows Vista.
> >
> Done in r1859517.


Tremendous progress, thanks for all of your efforts! I just finished a
visual review yesterday.

I don't know whom of us will jump on removing the ANSI logic first, but
that's next on the chopping block with unanimous consent.

Again, thanks!

Bill

 (And yes, I was referring to DisconnectEx() in the earlier dialog.)


Re: svn commit: r1859658 - /apr/site/trunk/doap.rdf

2019-05-21 Thread William A Rowe Jr
On Tue, May 21, 2019 at 11:01 AM  wrote:

> Author: wrowe
> Date: Tue May 21 16:00:46 2019
> New Revision: 1859658
>
> URL: http://svn.apache.org/viewvc?rev=1859658=rev
> Log:
> s/http:/https:/ - note that usefulinc.com is not so useful, cert CN
> mismatch
>
> --- apr/site/trunk/doap.rdf (original)
> +++ apr/site/trunk/doap.rdf Tue May 21 16:00:46 2019
> @@ -1,16 +1,16 @@
>  
>  http://usefulinc.com/ns/doap#;
> -  xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#;
> -  xmlns:asfext="http://projects.apache.org/ns/asfext#; xml:lang="en">
> +  xmlns:rdf="https://www.w3.org/1999/02/22-rdf-syntax-ns#;
> +  xmlns:asfext="https://projects.apache.org/ns/asfext#; xml:lang="en">
> ...


While all the other elts could be mapped to https:, usefulinc.com presents
simonstl.com as the site cert CN, and further has the wrong media encoding
for http://usefulinc.com/ns/doap# (it presents utf-8 while claiming
iso-8859-1).

What would be the suggested way to handle these discrepancies? For a
standard, they don't appear to grok standards.


Re: unsubscribe

2019-05-20 Thread William A Rowe Jr
I trust you remembered to email bugs-unsubscribe@apr.a.o, and this is all
fixed?


On Thu, May 2, 2019 at 12:27 PM Ryan Bloom  wrote:

>
> Ryan Bloom
> rbl...@gmail.com
>


Re: Supported Windows versions for APR 2.0 (was Re: [PATCH] Optimize apr_file_info_get(APR_FINFO_SIZE) on Windows)

2019-05-15 Thread William A Rowe Jr
On Wed, May 15, 2019 at 7:39 AM Eric Covener  wrote:

> > I would like to propose making Windows 7 / Windows Server 2008 R2 the
> minimum
> > supported Windows version for APR 2.0.
> >
> > This means that for APR 2.0 we would drop the support for Windows Vista
> and
> > Windows Server 2008 (not R2), although in my opinion that shouldn't be
> much
> > of a problem, because Windows Vista is no longer supported by Microsoft,
> and
> > the extended support for Windows Server 2008 will end on January 14,
> 2020 [6].
>
> +1, Win 7 and 2008 R2 still seems pretty conservative for an APR 2.0.
>

For APR 2.0 forwards, a hearty +1!


Re: [vote] Win32 Decision Point

2019-04-26 Thread William A Rowe Jr
On Thu, Apr 25, 2019 at 2:59 AM Ivan Zhakov  wrote:

> [X] Please drop 8-bit and focus only on utf-8 resource names on Win32.
>
> The only problem I see that it would harder to backport fixes to
> stable branches. What about release apr 1.8 without ANSI logic?
>

That isn't possible by my reading of our version contract. Yes, backports
would require some additional adjustments in the existing ANSI code,
but the of changes to that code is the future should be approximately nil.

I don't see us shipping a 1.8, if we proceed on course to ship a 2.0 some
time in the near future. Our feature-set is somewhat complete as things
stand now.


[vote] Win32 Decision Point

2019-04-24 Thread William A Rowe Jr
Some 17 years later we are at a crossroads, because the win32 code
is somewhat illegible and harder to maintain due to the ANSI-vs-UTF8,
Win9x-vs-NT code paths.

NT won. The only remaining question is how many apr consumers are
leveraging ANSI-specific builds for local code page semantics, vs how
many are willing to treat all system resources as utf-8 names, and for
ANSI, willing to live on the 1.x branch in perpetuity? These are builds
that explicitly toggle ANSI in spite of whatever OS the binary runs on.

So the vote is pretty simple, I propose to strip all ANSI 8-bit logic from
the apr (2.0) trunk/ and leave only the utf8->wide char logic remaining.
Committers and community both, please choose one below,

[ ] Please retain the ANSI logic in APR 2.0 on Win32

[ ] Please drop 8-bit and focus only on utf-8 resource names on Win32.

Will leave this question open a full 10 days to get the widest sampling
of opinions.


Re: apr 1.7.0 configure fails on macOS with Xcode 10.2.1 SDK MacOSX10.14.sdk

2019-04-24 Thread William A Rowe Jr
Hi Barry,

you are looking at the latest consensus code spanning BSD and OS/X in two
variants.

It would help us if you would share your config.log and entire output of
./configure along with cc -version type data for the applicable compiler.


On Wed, Apr 24, 2019 at 1:04 PM Barry Scott  wrote:

> I use ./configure --prefix=/usr/local/svn and see the error:
> ...
> checking whether int64_t and long long use fmt %lld... no
> configure: error: could not determine the string function for int64_t
>
> Looking in config.log I see this:
>
> configure:24313:
> /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang
> -c -arch x86_64 -isysroot
> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk
> -Wno-deprecated-declarations  -Werror -DDARWIN
> -DSIGPROCMASK_SETS_THREAD_MASK -DDARWIN_10 conftest.c >&5
> In file included from conftest.c:149:
> In file included from
> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/usr/include/sys/types.h:109:
> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/usr/include/sys/_types/_gid_t.h:31:25:
> error: cannot combine with previous 'type-name' declaration specifier
> typedef __darwin_gid_t  gid_t;
> ^
> ./confdefs.h:137:15: note: expanded from macro 'gid_t'
> #define gid_t int
>   ^
> In file included from conftest.c:149:
> In file included from
> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/usr/include/sys/types.h:128:
> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/usr/include/sys/_types/_uid_t.h:31:31:
> error: cannot combine with previous 'type-name' declaration specifier
> typedef __darwin_uid_tuid_t;
>   ^
> ./confdefs.h:136:15: note: expanded from macro 'uid_t'
> #define uid_t int
>   ^
> In file included from conftest.c:149:
> In file included from
> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/usr/include/sys/types.h:167:
> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/usr/include/sys/_types/_ssize_t.h:31:33:
> error: cannot combine with previous 'type-name' declaration specifier
> typedef __darwin_ssize_tssize_t;
> ^
> ./confdefs.h:138:17: note: expanded from macro 'ssize_t'
> #define ssize_t int
> ^
> 3 errors generated.
>
> Clearly configdefs.h must not define ssize_t - is this a know problem?
> Is there a patch I help can test?
>
> Barry
>
>


Re: [Announce] Apache Portable Runtime APR 1.7.0 Released

2019-04-07 Thread William A Rowe Jr
On Sat, Apr 6, 2019 at 12:33 PM Dennis Clarke  wrote:

> On 4/6/19 10:47 AM, William A Rowe Jr wrote:
> > On Fri, Apr 5, 2019 at 4:03 PM Nick Kew  > <mailto:n...@apache.org>> wrote:
> >
> >
> >     > On 5 Apr 2019, at 19:53, William A Rowe Jr  > <mailto:wr...@apache.org>> wrote:
> > >
> > >Apache Portable Runtime APR 1.7.0 Released
> > >
> > >The Apache Software Foundation and the Apache Portable Runtime
> > >Project are proud to announce the General Availability of
> version
> > >1.7.0 of the Apache Portable Runtime library (APR). Version
> 1.6.1
> > >of the APR Utility library (APR-util) and version 1.2.2 of the
> > >APR iconv library (APR-iconv) remain current.
> >
> > Thanks, Bill.
> >
> > Just a quick note.  Some of the announcement (expat, MySQL and
> FreeTDS)
> > relate not to this APR, but to APR-UTIL updates which are not part
> > of this release.
> >
> >
> > I'm aware. There is also the problem of our release tracking, we have
> > nothing
> > but apr versions there. I understood there is a mass-export-import
> > facility which
> > I'll look into, but basically "1.7.0" and prior all need to become
> > "apr-1.7.0" etc,
> > and then we need to import all the historical apr-util-* apr-iconv-*
> sets.
> >
> > Before the next release, we aught to split Announcement-x.x.x into
> distinct
> > files for the apr, apr-util and apr-iconv groups. As of 2.0.0 these all
> > merge up,
> > but in the interim...
> >
> >
> > I know you're keen to get an APR-UTIL update released reasonably
> > soon, and I
> > promise to tackle at least the XML issues I've been working on when
> > I get back home -
> > which I anticipate being second half of next week.  We also have
> > some further
> > MySQL updates from an external contributor, which I hope to find
> > time to review
> > in time for APR-UTIL 1.7.
> >
> >
> > I was hoping that we could find a 6-12 week window for publishing the
> > next release.
>
> At least two months would be good. However Apache foo is a *major* piece
> of the planetary software ecosystem possibly more so than ISC software.
> So a quarterly schedule ( 3 months ) may be more reasonable.
>
> > Since I started thinking about it, I'm wondering whether we should push
> > towards apr-util-1.7.0, or just get to apr-2.0.0 already. Other thoughts
> > or observations?
>
> Neither at this time.
>

It will certainly be at least apr-util 1.7.0, or perhaps apr-2.0.0 which
rolls all of the functionality of the -util library into the apr core. There
have been a number of enhancements over the past year, present
on both branches, which committers are patiently waiting for release.

A cadence, in this case, is of no concern. In terms of maintenance
updates, 1.7.1 releases, I agree a periodic (3 mos or similar) refresh
cycle is helpful, when significant fixes are present and awaiting release.


Re: [Announce] Apache Portable Runtime APR 1.7.0 Released

2019-04-06 Thread William A Rowe Jr
On Fri, Apr 5, 2019 at 4:03 PM Nick Kew  wrote:

>
> > On 5 Apr 2019, at 19:53, William A Rowe Jr  wrote:
> >
> >Apache Portable Runtime APR 1.7.0 Released
> >
> >The Apache Software Foundation and the Apache Portable Runtime
> >Project are proud to announce the General Availability of version
> >1.7.0 of the Apache Portable Runtime library (APR). Version 1.6.1
> >of the APR Utility library (APR-util) and version 1.2.2 of the
> >APR iconv library (APR-iconv) remain current.
>
> Thanks, Bill.
>
> Just a quick note.  Some of the announcement (expat, MySQL and FreeTDS)
> relate not to this APR, but to APR-UTIL updates which are not part of this
> release.
>

I'm aware. There is also the problem of our release tracking, we have
nothing
but apr versions there. I understood there is a mass-export-import facility
which
I'll look into, but basically "1.7.0" and prior all need to become
"apr-1.7.0" etc,
and then we need to import all the historical apr-util-* apr-iconv-* sets.

Before the next release, we aught to split Announcement-x.x.x into distinct
files for the apr, apr-util and apr-iconv groups. As of 2.0.0 these all
merge up,
but in the interim...


> I know you're keen to get an APR-UTIL update released reasonably soon, and
> I
> promise to tackle at least the XML issues I've been working on when I get
> back home -
> which I anticipate being second half of next week.  We also have some
> further
> MySQL updates from an external contributor, which I hope to find time to
> review
> in time for APR-UTIL 1.7.
>

I was hoping that we could find a 6-12 week window for publishing the next
release.

Since I started thinking about it, I'm wondering whether we should push
towards apr-util-1.7.0, or just get to apr-2.0.0 already. Other thoughts or
observations?


[Announce] Apache Portable Runtime APR 1.7.0 Released

2019-04-05 Thread William A Rowe Jr
   Apache Portable Runtime APR 1.7.0 Released

   The Apache Software Foundation and the Apache Portable Runtime
   Project are proud to announce the General Availability of version
   1.7.0 of the Apache Portable Runtime library (APR). Version 1.6.1
   of the APR Utility library (APR-util) and version 1.2.2 of the
   APR iconv library (APR-iconv) remain current.

   There are a number of specific changes in how APR is deployed
   and how APR-util deals with external dependencies in these current
   releases, which may be disruptive to existing build strategies:

- The typical cross-process locking stragegy now defaults to
  a pthreads lock which will not leak on abnormal process termination,
  as opposed to the earlier sysv semaphore locking strategy.
  This should result in fewer orphaned locks in misbehaving
  applications.

- Netware users should be aware that APR mis-defined the entity
  representing an "os mutex", and it was missing a level of
  indirection. The new definition should allow the use of the
  apr_os_proc_mutex_get/_put API's on the Netware platform but
  requires a rebuild against the APR 1.7.0 or later library.

- Expat sources are no longer bundled, this is now an external
  dependency. Install libexpat runtime (usually installed by
  default) and development packages using your system's package
  manager, or from .

- MySQL support is updated as advised by the MySQL developers.
  MySQL versions older than 5.5 should not be used. If you do
  use an old MySQL version, use the thread-safe libmysqlclient_r
  version of the library.

- FreeTDS partial and incomplete support has been dropped.
  Users of MSSQL and SYBASE databases are recommended to use
  the ODBC driver instead.

   APR 1.7.0, APR-util 1.6.1, and APR-iconv 1.2.2 fix a number
   of security vulnerabilities, run-time and build-time issues.
   For details, see;

 http://www.apache.org/dist/apr/CHANGES-APR-1.7
 http://www.apache.org/dist/apr/CHANGES-APR-UTIL-1.6
 http://www.apache.org/dist/apr/CHANGES-APR-ICONV-1.2

   APR, APR-util and APR-iconv are available for download from:

 http://apr.apache.org/download.cgi

   The mission of the Apache Portable Runtime Project is to create
   and maintain software libraries that provide a predictable and
   consistent interface to underlying platform-specific
   implementations. The primary goal is to provide an API to
   which software developers may code and be assured of predictable
   if not identical behavior regardless of the platform on which
   their software is built. We list all known projects using APR
   at http://apr.apache.org/projects.html - so please let us know
   if you find our libraries useful in your own projects!


Re: [result] [vote] Release apr-1.7.0 ?

2019-04-04 Thread William A Rowe Jr
Yes, please consult with Rainer, there are still hours to pull back and
rework a 1.7.1 launch.

Thanks for the very complete details.

Please crosscheck if you are each speaking of opteron or sparc flavors.

On Thu, Apr 4, 2019 at 1:32 PM Dennis Clarke  wrote:

> On 4/4/19 2:10 PM, William A Rowe Jr wrote:
> > First off, thanks all who have contributed to the 1.6.0 -> 1.7.0
> evolution
> > in some large or small way. Secondly, thanks to all who reviewed.
> >
>
> May not matter much at this point but on Solaris 10 sparc I saw this in
> tests :
>
> us=$status; \
> progfailed="$progfailed '$prog mode
> $mode'"; \
> fi; \
> done; \
> else \
> ./$prog -v; \
> status=$?; \
> if test $status != 0; then \
> teststatus=$status; \
> progfailed="$progfailed $prog"; \
> fi; \
> fi; \
> done; \
> if test $teststatus != 0; then \
> echo "Programs failed:$progfailed"; \
> fi; \
> exit $teststatus
> ld.so.1: testlockperf: fatal: relocation error: file testlockperf:
> symbol apr_thread_mutex_timedlock: referenced symbol not found
> /bin/bash: line 2: 27221 Killed  ./$prog -v
> .
> .
> .
>
> However I think Rainer Jung had no such problems. I will go take a look
> at my environment and see what went sideways.
>
> Config was :
>
>
> beta $ pwd
> /usr/local/build/apr-1.7.0_SunOS5.10_sparc64vii+.001
> beta $ ./configure --prefix=/usr/local --enable-threads \
> > --enable-shared --enable-static \
> > --enable-other-child --with-devrandom=/dev/urandom \
> > --enable-posix-shm
> checking build system type... sparc-sun-solaris2.10
> checking host system type... sparc-sun-solaris2.10
> checking target system type... sparc-sun-solaris2.10
> Configuring APR library
> Platform: sparc-sun-solaris2.10
> checking for working mkdir -p... yes
> APR Version: 1.7.0
> checking for chosen layout... apr
> checking for gcc... /opt/developerstudio12.6/bin/cc
> checking whether the C compiler works... yes
> checking for C compiler default output file name... a.out
> checking for suffix of executables...
> checking whether we are cross compiling... no
> checking for suffix of object files... o
> checking whether we are using the GNU C compiler... no
> checking whether /opt/developerstudio12.6/bin/cc accepts -g... yes
> checking for /opt/developerstudio12.6/bin/cc option to accept ISO C89...
> none needed
> checking for a sed that does not truncate output... /usr/xpg4/bin/sed
> Applying APR hints file rules for sparc-sun-solaris2.10
>   adding "-DSOLARIS2=10" to CPPFLAGS
>   adding "-D_REENTRANT" to CPPFLAGS
>   setting ac_cv_func_pthread_mutex_timedlock to "no"
>   setting apr_lock_method to "USE_PROC_PTHREAD_SERIALIZE"
>   setting ac_cv_func_readdir64_r to "no"
> (Default will be unix)
> checking whether make sets $(MAKE)... yes
> checking how to run the C preprocessor... /opt/developerstudio12.6/bin/cc
> -E
> checking for gawk... no
> checking for mawk... no
> checking for nawk... nawk
> checking whether ln -s works... yes
> checking for ranlib... ranlib
> checking for a BSD-compatible install... build/install.sh -c
> checking for rm... rm
> checking for as... as
> checking for cpp... no
> checking for ar... ar
> checking for grep that handles long lines and -e... /usr/xpg4/bin/grep
> checking for egrep... /usr/xpg4/bin/grep -E
> checking for ANSI C header files... yes
> checking for sys/types.h... yes
> checking for sys/stat.h... yes
> checking for stdlib.h... yes
> checking for string.h... yes
> checking for memory.h... yes
> checking for strings.h... yes
> checking for inttypes.h... yes
> checking for stdint.h... yes
> checking for unistd.h... yes
> checking minix/config.h usability... no
> checking minix/config.h presence... no
> checking for minix/config.h... no
> checking whether it is safe to define __EXTENSIONS__... yes
> checking for library containing strerror... none required
> checking whether system uses EBCDIC... no
> performing libtool configuration...
> checking how to print strings... printf
> checking for a sed that does not truncate output... (cached)
> /usr/xpg4/bin/sed
> checking for fgrep... /usr/xpg4/bin/grep -F
> checking for non-GNU ld... /usr/ccs/bin/sparcv9/ld
> checking if the linker (/usr/ccs/bin/sparcv9/ld) is GNU ld... no
> checking for BSD- or MS-compatible name lister (nm)... /usr/xpg4/bin/nm -p
> checking the name lister (/usr/xpg4/bin/nm -p) interface... BSD nm
&

[result] [vote] Release apr-1.7.0 ?

2019-04-04 Thread William A Rowe Jr
First off, thanks all who have contributed to the 1.6.0 -> 1.7.0 evolution
in some large or small way. Secondly, thanks to all who reviewed.

Steffan's concerns are noted, and the resolution does not appear to be
binary breakage, so we should be safe. Shout loudly and quickly if I
misunderstood. It's concerning that we should possibly re-describe
this mechanism as a 64-less-one-bit unsigned API. For the purposes
it was imagined for, I expect it's sufficient.

My own tests are largely on Fedora and I'm +1 on the result, and very
grateful for Gregg for the windows dsp/makefile refresh. I observed
Yann's feedback which I agree with on the change of locking priority
in the announcement, I'm still hoping someone from the Netware
maintainers community will comment on that specific communication
in the draft Announcement before it goes out tomorrow.

So with all this said, the vote passes unanimously, and I'm moving
the files to release/ about 10 minutes late, and will announce when
the mirrors have populated tomorrow. Again, thanks everyone, we
can start a fresh thread on apr-util 1.next, or the entire kitchen sink
of releasing 2.0 on the tail of this small effort.

Cheers,

Bill


On Mon, Apr 1, 2019 at 1:01 PM William A Rowe Jr 
wrote:

> Candidate tarballs are at the usual location;
> https://apr.apache.org/dev/dist/
>
> For the release of apr-1.7.0
>   [  ]  +1 looks great!
>   [  ]  -1 something is broken
>
> This vote will conclude April 4th 2pm EDT, for potential
> announcement Friday.
>
> I could use a hand from Netware folk to explain the potential for
> binary breakage in os locks to end users for our Announcement,
> not that this ever really worked in the first place AIUI.
>


tarball.sha256 file formatting (Was Re: [VOTE] apr-1.6.4 release?)

2019-04-01 Thread William A Rowe Jr
On Mon, Sep 10, 2018 at 3:34 PM Rainer Jung  wrote:

> Am 07.09.2018 um 18:19 schrieb William A Rowe Jr:
> > Please cast your votes on the following release candidate
> > found at http://apr.apache.org/dev/dist/
> >
> > Release apr-1.6.4
> >[  ] +/-1
>
> One niggle: the sha files are not very tool friendly. I'm using
> sha1sum/sha256sum/sha512sum which can check the checksum files but do
> not understand the format used for 1.6.4. The previous version 1.6.3
> used a checksum file format, the tools did understand. That format would be
>
> CHECKSUM *FILENAME
>
> and the "*" (indicating binary) is only there if the file is not a text
> file, eg. for our tarballs and zip files.


Hi Rainer,

I didn't find a follow-up on this, but I trust our tools/release.sh now
gives you
an appropriate .sha256 format file your tools can parse, with the -r
coreutils
flag passed to openssl?


[vote] Release apr-1.7.0 ?

2019-04-01 Thread William A Rowe Jr
Candidate tarballs are at the usual location;
https://apr.apache.org/dev/dist/

For the release of apr-1.7.0
  [  ]  +1 looks great!
  [  ]  -1 something is broken

This vote will conclude April 4th 2pm EDT, for potential
announcement Friday.

I could use a hand from Netware folk to explain the potential for
binary breakage in os locks to end users for our Announcement,
not that this ever really worked in the first place AIUI.


Re: svn commit: r1856274 - in /apr/apr/branches/1.7.x: ./ configure.in file_io/unix/dir.c

2019-03-31 Thread William A Rowe Jr
I'm happy to do that, and to preserve your dir.c fix. Commit coming up,
crossing fingers for a clean Linux maintainer compilation.



On Sun, Mar 31, 2019, 11:44 Yann Ylavic  wrote:

> On Sun, Mar 31, 2019 at 5:29 PM William A Rowe Jr 
> wrote:
> >
> > On Sun, Mar 31, 2019, 09:15 Yann Ylavic  wrote:
> >>
> >> On Sun, Mar 31, 2019 at 3:58 AM William A Rowe Jr 
> wrote:
> >> >
> >> > Let me just throw this out there and get your reaction, Yann, now that
> >> > I'm back in town... The patch I was initially using to accomplish the
> >> > equivalent was as simple as;
> >> []
> >> > Seems much less wordy for a feature we agree should just be dropped
> >> > altogether fairly soon. Thoughts?
> >>
> >> I don't know actually, I think it's not really "fair" to say that
> >> readdir() is thread-safe, you can't really call it from different
> >> threads with the same DIR arg, while you might with readdir_r()
> >> (besides all its caveats). I'm talking about the system functions
> >> here, not apr_dir_read() which is not thread-safe (on the same
> >> apr_dir_t) with either underlying function, as we discussed elsewhere.
> >> But who would do that anyway?! I think we should do[x]cument that just
> in case.
> >>
> >> As for your patch vs mine, it depends on whether it exists systems
> >> (that we still support) on which readdir() is really not thread-safe
> >> even with different passed-in DIRs (e.g. the returned struct dirent is
> >> static somewhere in the implementation, as opposed to some room
> >> reserved in the struct DIR itself like in "modern" implementations),
> >> and these systems provide readdir_r() for the "thread-safe" way.
> >> I suppose such system wouldn't emit a warning for readdir_r(), my
> >> patch addresses them while yours does not.
> >> So the question is, does such system exists and do we want to still
> support it?
> >
> >
> > I think our answer is we never did support this.
> >
> > My thought is that all support this, inasmuch as they support concurrent
> dlopen's and readdir's on the same thread. If they are truly not thread
> safe (as opposed to parallel or reentrant) on different handles that would
> be a very thread-unsafe architecture in the first place, no? A perfect
> example for APR has no threads at all.
> >
> > Since our readdir is threadsafe flag has a well established meaning
> (thread safe for APR's purpose alone) and we can preserve that meaning with
> no harm between 1.0 and 1.x builds (all overrides preserve the desired
> behavior) I'm in favor of the simplest solution.
> >
> > I'm entirely in favor of dropping the macro from 2.0 and never returning
> to dirread_r() after all the explanatory docs and your evaluation of APR.
> If user wants to walk a single directory across parallel threads, they
> clearly need to serialize the actual directory read and use a serialized
> allocator in that odd edge case, much as they must do the same for any sane
> file read.
>
> Make sense, I'm fine with this.
> If you want to proceed by reverting my related changes, please go down
> to r1789258 since this first commit was an (unsatisfactory) try later
> overwritten by r1856189. The comment change from r1789258 might be
> preserved though.
> Thanks for the simplification!
>


Re: svn commit: r1856274 - in /apr/apr/branches/1.7.x: ./ configure.in file_io/unix/dir.c

2019-03-30 Thread William A Rowe Jr
On Tue, Mar 26, 2019 at 3:35 AM  wrote:

> Author: ylavic
> Date: Tue Mar 26 08:35:35 2019
> New Revision: 1856274
>
> URL: http://svn.apache.org/viewvc?rev=1856274=rev
> Log:
> Merge r1789258, r1856189, r1856191, r1856192, r1856196 from trunk:
>
>
> apr_dir_read: Since readdir() is now thread safe on most (if not all)
> unixes
> and readdir_r() is defective and deprecated, use the former by default
> unless
> APR_USE_READDIR_R is defined (no use case currently hence not
> autoconfigured).
>
>
> Follow up to r1789258: configure to detect whether readdir() is
> thread-safe.
>
> On platforms where readdir_r() is available but deprecated, readdir() is to
> be used although it's not in libc_r (e.g. Linux has no libc_r).
>
> In this case we can APR_TRY_COMPILE_NO_WARNING readdir_r() and, if it's
> deprecated, define READDIR_IS_THREAD_SAFE.
>
> With this we don't need user-defined APR_USE_READDIR{,64}_R from r1789258.
>
>
> Follow up to r1856189: sys/types.h possibly needed.
>
>
> Follow up to r1856189: use NAME_MAX from limits.h when available.
>
>
> apr_dir: no need to allocate our dir entry if readdir{,64}_r() is not used.
>
> Modified:
> apr/apr/branches/1.7.x/   (props changed)
> apr/apr/branches/1.7.x/configure.in
> apr/apr/branches/1.7.x/file_io/unix/dir.c
>
> URL:
> http://svn.apache.org/viewvc/apr/apr/branches/1.7.x/configure.in?rev=1856274=1856273=1856274=diff
>
> ==
> --- apr/apr/branches/1.7.x/configure.in (original)
> +++ apr/apr/branches/1.7.x/configure.in Tue Mar 26 08:35:35 2019
> @@ -817,8 +817,31 @@ ac_cv_define_GETSERVBYNAME_IS_THREAD_SAF
>  if test "$threads" = "1"; then
>  echo "APR will use threads"
>  AC_CHECK_LIB(c_r, readdir,
> + apr_readdir_is_thread_safe=yes)
> +if test "x$apr_readdir_is_thread_safe" = "x"; then
> +AC_CHECK_HEADERS(dirent.h)
> +AC_CHECK_FUNCS(readdir_r)
> +APR_IFALLYES(header:dirent.h func:readdir_r,
> + apr_has_readdir_r="1", apr_has_readdir_r="0")
> +if test "$apr_has_readdir_r" = "1"; then
> +dnl readdir_r() may exist but be deprecated, meaning
> +dnl the readdir() itself is thread-safe
> +APR_TRY_COMPILE_NO_WARNING([
> +#include 
> +#include 
> +],
> +[
> +DIR *dir = opendir("/tmp");
> +struct dirent entry, *result;
> +return readdir_r(dir, , ) != 0;
> +], apr_readdir_is_thread_safe=no,
> apr_readdir_is_thread_safe=yes)
> +fi
> +fi
> +if test "$apr_readdir_is_thread_safe" = "yes"; then
> +AC_MSG_NOTICE([APR will use thread-safe readdir()])
>  AC_DEFINE(READDIR_IS_THREAD_SAFE, 1,
> -  [Define if readdir is thread safe]))
> +  [Define if readdir is thread safe])
> +fi
>  if test "x$apr_gethostbyname_is_thread_safe" = "x"; then
>  AC_CHECK_LIB(c_r, gethostbyname,
> apr_gethostbyname_is_thread_safe=yes)
>  fi
>

Let me just throw this out there and get your reaction, Yann, now that
I'm back in town... The patch I was initially using to accomplish the
equivalent was as simple as;

--- configure.in (revision 1839621)
+++ configure.in (working copy)
@@ -835,15 +835,13 @@
 fi
 fi

-ac_cv_define_READDIR_IS_THREAD_SAFE=no
+ac_cv_define_READDIR_IS_THREAD_SAFE=yes
 ac_cv_define_GETHOSTBYNAME_IS_THREAD_SAFE=no
 ac_cv_define_GETHOSTBYADDR_IS_THREAD_SAFE=no
 ac_cv_define_GETSERVBYNAME_IS_THREAD_SAFE=no
 if test "$threads" = "1"; then
 AC_MSG_NOTICE([APR will use threads])
-AC_CHECK_LIB(c_r, readdir,
-AC_DEFINE(READDIR_IS_THREAD_SAFE, 1,
-  [Define if readdir is thread safe]))
+AC_DEFINE(READDIR_IS_THREAD_SAFE, 1, [Modern readdir is thread safe])
 if test "x$apr_gethostbyname_is_thread_safe" = "x"; then
 AC_CHECK_LIB(c_r, gethostbyname,
apr_gethostbyname_is_thread_safe=yes)
 fi

Seems much less wordy for a feature we agree should just be dropped
altogether fairly soon. Thoughts?


Re: [poll] Resolving readdir_r warning (Re: Showstoppers to 1.7.0?)

2019-03-29 Thread William A Rowe Jr
Sounds great, good observations. Absent any other opinions, with that
resolved, I'm happy to tag and roll this afternoon to begin the release
vote.

On Fri, Mar 29, 2019, 04:26 Yann Ylavic  wrote:

> On Fri, Mar 29, 2019 at 5:01 AM William A Rowe Jr 
> wrote:
> >
> > Opinions?
>
> Anyway, our usage of readdir_r() is no more thread-safe than with the
> non-_r() version, because of the (struct dirent *)thedir->entry we
> pass to it (should be either on the stack or allocated for each
> apr_dir_read() call), or more generally because of our design of
> apr_dir_read() which does _not_ allow to call it concurrently with the
> same apr_dir_t (thus pool), like almost all of our designs.
>
> So readdir() vs readdir_r () is a non-issue to begin with IMHO (we use
> readdir_r() probably like readdir() is implemented), let's just
> silence the warning like in r1856274...
>


Re: [poll] Resolving readdir_r warning (Re: Showstoppers to 1.7.0?)

2019-03-28 Thread William A Rowe Jr
Yes, that patch is option 2... and I would be alright with that choice.
Looking for consensus on which way we resolve this.

Opinions?

On Thu, Mar 28, 2019, 10:55 Yann Ylavic  wrote:

> On Thu, Mar 28, 2019 at 4:19 PM William A Rowe Jr 
> wrote:
> >
> > So... What is our preferred solution as of 1.7.0?
> >
> > 1. Drop readdir[64]_r for readdir[64] always.
> > 2. Allow readdir[64]_r with an autoconf toggle, skip detection.
> > 3. Override readdir[64]_r detection with a test of ldd --version >= 2.24
>
> How about http://svn.apache.org/r1856274 ?
>
> That's an autoconf to detect warnings when compiling readdir_r(), any
> warning but the simply main() shouldn't raise unrelated ones
> hopefully..., such that in this case we use readdir()?
>


  1   2   3   4   5   6   7   8   9   10   >