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

[William A Rowe Jr <wr...@rowe-clan.net>]

So to this question, glibc 2.24 is the first release to loudly deprecate
readdir_r.
https://www.sourceware.org/ml/libc-alpha/2016-08/msg00212.html

However, the docs summary mentioned below calls out reasons why readdir_r
was never a good thing.

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


I am leaning towards option 1 at this point, even as I was about to
implement 3.



On Sun, Mar 24, 2019, 18:13 William A Rowe Jr <wr...@rowe-clan.net> wrote:

> AFAICT the very last 'nit' is our bad habit of using the deprecated
> readdir_r()/readdir64_t() functions in modern gcc. Comparing ./configure
> and make reminds me I had a kludge to finish in my working 1.7 tree. See
> https://www.gnu.org/software/libc/manual/html_node/Reading_002fClosing-Directory.html
> for a lengthy explanation.
>

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

[Yann Ylavic <yl...@gmail.com>]

On Thu, Mar 28, 2019 at 4:55 PM Yann Ylavic <yl...@gmail.com> wrote:
>
> On Thu, Mar 28, 2019 at 4:19 PM William A Rowe Jr <wr...@rowe-clan.net> 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()?

Forgot to mention, it does not address readdir64_r(), what system is that?

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

[William A Rowe Jr <wr...@rowe-clan.net>]

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 <yl...@gmail.com> wrote:

> On Fri, Mar 29, 2019 at 5:01 AM William A Rowe Jr <wr...@rowe-clan.net>
> 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?)

[Yann Ylavic <yl...@gmail.com>]

On Fri, Mar 29, 2019 at 5:01 AM William A Rowe Jr <wr...@rowe-clan.net> 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?)

[William A Rowe Jr <wr...@rowe-clan.net>]

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 <yl...@gmail.com> wrote:

> On Thu, Mar 28, 2019 at 4:19 PM William A Rowe Jr <wr...@rowe-clan.net>
> 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()?
>

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

[Yann Ylavic <yl...@gmail.com>]

On Thu, Mar 28, 2019 at 4:19 PM William A Rowe Jr <wr...@rowe-clan.net> 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()?