Re: [PATCH] Added FTW.H Methods

> On 17-Aug-2020, at 3:12 PM, Corinna Vinschen <[email protected]> wrote:
> 
> On Aug 13 08:11, Joel Sherrill wrote:
>>> On Thu, Aug 13, 2020 at 3:17 AM Corinna Vinschen via Newlib <
>>> [email protected]> wrote:
>>> 
>>> On Aug 13 03:08, Eshan dhawan via Newlib wrote:
>>>> Signed-off-by: Eshan dhawan <[email protected]>
>>>> ---
>>>> newlib/configure.host         |   2 +-
>>>> newlib/libc/include/ftw.h     |  64 ++++++++++++++
>>>> newlib/libc/posix/Makefile.am |   2 +-
>>>> newlib/libc/posix/ftw.c       |  36 ++++++++
>>>> newlib/libc/posix/nftw.c      | 154 ++++++++++++++++++++++++++++++++++
>>>> 5 files changed, 256 insertions(+), 2 deletions(-)
>>>> create mode 100644 newlib/libc/include/ftw.h
>>>> create mode 100644 newlib/libc/posix/ftw.c
>>>> create mode 100644 newlib/libc/posix/nftw.c
>>> 
>>> Why do we need _FTW_ENABLE_?
>>> 
>>> What do we need the foo64 variants for?  WHile newlib defines
>>> _LARGEFILE64_SOURCE for __linux__ and __RDOS__, it's not used
>>> anywhere else in newlib itself.
>>> 
>>> Also, there's a ftw/nftw implementation in Cygwin already.  Did you
>>> check it for inclusion into newlib, or to drop the Cygwin version in
>>> favor of a new newlib versions?  We could at least share a common
>>> header, but then we will have to use the flag values already used in
>>> the Cygwin implementation for backward compatibility.
>>> 
>> 
>> We went straight to *BSD for implementations and didn't look at Cygwin.
> 
> Cygwin's ftw is OpenBSD.  Which BSD is this ftw taken from?
It’s taken from MUSL Libc 
> 
>> I've asked Eshan to replace the implementation with the Cygwin one.
>> Hopefully that's easy and allows one more piece of shared software.
> 
> It's not *that* easy because OpenBSD's/Cygwin's ftw(3) uses fts(3) under
> the hood.
That’s the same reason We couldn’t use FTW from BSD 
> 
> While sharing code is good, your implementation is more compact, which
> speaks for your code.
> 
> However, I'd like to ask for ftw.h compatibility at least, so perhaps
> Cygwin can use this other ftw, too at one point.
> 
> But even then, there's still the _FTW_ENABLE_ and _LARGEFILE64_SOURCE
> stuff I asked about.
_FTW_ENABLE_ was there because the FTW implementation required pthread_setcancelstate ()
> 
> 
> Thanks,
> Corinna
>