* [PATCH 0/3] libc: Comply with the libstdc++ test case names.cc @ 2020-10-01 14:17 Torbjörn SVENSSON 2020-10-01 14:17 ` [PATCH 1/3] libc/include/inttypes.h: Remove parameter name Torbjörn SVENSSON ` (2 more replies) 0 siblings, 3 replies; 21+ messages in thread From: Torbjörn SVENSSON @ 2020-10-01 14:17 UTC (permalink / raw) To: newlib As discussed in GCC bug 97088 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97088), parameters in prototypes of library functions should use reserved names, or no name at all. It's somewhat unclear how to handle members in structs, but as the __tzrule_struct is an internal newlib struct, I don't see any point in not trying to comply here too. Without the patches, you will get errors like: In file included from /work/build-native/gcc-final/arm-none-eabi/thumb/v6-m/nofp/libstdc++-v3/include/ctime:42, from /work/build-native/gcc-final/arm-none-eabi/thumb/v6-m/nofp/libstdc++-v3/include/arm-none-eabi/bits/stdc++.h:49, from /work/src/gcc/libstdc++-v3/testsuite/17_intro/names.cc:199: /work/install-native/arm-none-eabi/include/time.h:110: error: expected unqualified-id before ';' token /work/install-native/arm-none-eabi/include/time.h:110: error: expected ')' before ';' token /work/src/gcc/libstdc++-v3/testsuite/17_intro/names.cc:55: note: to match this '(' In file included from /work/build-native/gcc-final/arm-none-eabi/thumb/v6-m/nofp/libstdc++-v3/include/cwchar:44, from /work/build-native/gcc-final/arm-none-eabi/thumb/v6-m/nofp/libstdc++-v3/include/arm-none-eabi/bits/stdc++.h:50, from /work/src/gcc/libstdc++-v3/testsuite/17_intro/names.cc:199: /work/install-native/arm-none-eabi/include/wchar.h:251: error: expected ')' before ';' token /work/install-native/arm-none-eabi/include/wchar.h:252: error: expected ')' before ';' token In file included from /work/build-native/gcc-final/arm-none-eabi/thumb/v6-m/nofp/libstdc++-v3/include/cinttypes:46, from /work/build-native/gcc-final/arm-none-eabi/thumb/v6-m/nofp/libstdc++-v3/include/arm-none-eabi/bits/stdc++.h:56, from /work/src/gcc/libstdc++-v3/testsuite/17_intro/names.cc:199: /work/install-native/arm-none-eabi/include/inttypes.h:323: error: expected ')' before ';' token I'd need someone to help me push the patches since I have no commit access. Torbjörn SVENSSON (3): libc/include/inttypes.h: Remove parameter name libc/include/wchar.h: Remove parameter name libc: Replace one letter member names in __tzrule_struct newlib/libc/include/inttypes.h | 2 +- newlib/libc/include/time.h | 8 ++++---- newlib/libc/include/wchar.h | 4 ++-- newlib/libc/time/tzcalc_limits.c | 14 +++++++------- newlib/libc/time/tzset_r.c | 22 +++++++++++----------- 5 files changed, 25 insertions(+), 25 deletions(-) -- 2.18.0 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/3] libc/include/inttypes.h: Remove parameter name 2020-10-01 14:17 [PATCH 0/3] libc: Comply with the libstdc++ test case names.cc Torbjörn SVENSSON @ 2020-10-01 14:17 ` Torbjörn SVENSSON 2020-10-01 23:22 ` Jeff Johnston 2020-10-01 14:17 ` [PATCH 2/3] libc/include/wchar.h: " Torbjörn SVENSSON 2020-10-01 14:17 ` [PATCH 3/3] libc: Replace one letter member names in __tzrule_struct Torbjörn SVENSSON 2 siblings, 1 reply; 21+ messages in thread From: Torbjörn SVENSSON @ 2020-10-01 14:17 UTC (permalink / raw) To: newlib As discussed in GCC bug 97088 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97088), parameters in prototypes of library functions should use reserved names, or no name at all. This patch removes the 'j' parameter name from extern intmax_t imaxabs(intmax_t); to avoid possible clashes with user code in case someone uses before including Newlib's inttypes.h (or uses some other conflicting definition) Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@st.com> --- newlib/libc/include/inttypes.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/newlib/libc/include/inttypes.h b/newlib/libc/include/inttypes.h index 073215476..570ed0481 100644 --- a/newlib/libc/include/inttypes.h +++ b/newlib/libc/include/inttypes.h @@ -320,7 +320,7 @@ struct _reent; extern "C" { #endif -extern intmax_t imaxabs(intmax_t j); +extern intmax_t imaxabs(intmax_t); extern imaxdiv_t imaxdiv(intmax_t numer, intmax_t denomer); extern intmax_t strtoimax(const char *__restrict, char **__restrict, int); extern intmax_t _strtoimax_r(struct _reent *, const char *__restrict, char **__restrict, int); -- 2.18.0 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/3] libc/include/inttypes.h: Remove parameter name 2020-10-01 14:17 ` [PATCH 1/3] libc/include/inttypes.h: Remove parameter name Torbjörn SVENSSON @ 2020-10-01 23:22 ` Jeff Johnston 2020-10-02 6:52 ` Torbjorn SVENSSON 0 siblings, 1 reply; 21+ messages in thread From: Jeff Johnston @ 2020-10-01 23:22 UTC (permalink / raw) To: Torbjörn SVENSSON; +Cc: Newlib Looks fine. Could you please resend the patch as an attachment? Thanks, -- Jeff J. On Thu, Oct 1, 2020 at 10:19 AM Torbjörn SVENSSON via Newlib < newlib@sourceware.org> wrote: > As discussed in GCC bug 97088 > (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97088), parameters in > prototypes of library functions should use reserved names, or no name > at all. > > This patch removes the 'j' parameter name from > extern intmax_t imaxabs(intmax_t); > > to avoid possible clashes with user code in case someone uses > before including Newlib's inttypes.h (or uses some other conflicting > definition) > > Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@st.com> > --- > newlib/libc/include/inttypes.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/newlib/libc/include/inttypes.h > b/newlib/libc/include/inttypes.h > index 073215476..570ed0481 100644 > --- a/newlib/libc/include/inttypes.h > +++ b/newlib/libc/include/inttypes.h > @@ -320,7 +320,7 @@ struct _reent; > extern "C" { > #endif > > -extern intmax_t imaxabs(intmax_t j); > +extern intmax_t imaxabs(intmax_t); > extern imaxdiv_t imaxdiv(intmax_t numer, intmax_t denomer); > extern intmax_t strtoimax(const char *__restrict, char **__restrict, > int); > extern intmax_t _strtoimax_r(struct _reent *, const char *__restrict, > char **__restrict, int); > -- > 2.18.0 > > ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH 1/3] libc/include/inttypes.h: Remove parameter name 2020-10-01 23:22 ` Jeff Johnston @ 2020-10-02 6:52 ` Torbjorn SVENSSON 2020-10-02 6:53 ` nrupp 2020-10-02 21:01 ` Jeff Johnston 0 siblings, 2 replies; 21+ messages in thread From: Torbjorn SVENSSON @ 2020-10-02 6:52 UTC (permalink / raw) To: Jeff Johnston; +Cc: Newlib [-- Attachment #1: Type: text/plain, Size: 1658 bytes --] Patch attached. From: Jeff Johnston <jjohnstn@redhat.com> Sent: den 2 oktober 2020 01:23 To: Torbjorn SVENSSON <torbjorn.svensson@st.com> Cc: Newlib <newlib@sourceware.org> Subject: Re: [PATCH 1/3] libc/include/inttypes.h: Remove parameter name Looks fine. Could you please resend the patch as an attachment? Thanks, -- Jeff J. On Thu, Oct 1, 2020 at 10:19 AM Torbjörn SVENSSON via Newlib <newlib@sourceware.org<mailto:newlib@sourceware.org>> wrote: As discussed in GCC bug 97088 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97088), parameters in prototypes of library functions should use reserved names, or no name at all. This patch removes the 'j' parameter name from extern intmax_t imaxabs(intmax_t); to avoid possible clashes with user code in case someone uses before including Newlib's inttypes.h (or uses some other conflicting definition) Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@st.com<mailto:torbjorn.svensson@st.com>> --- newlib/libc/include/inttypes.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/newlib/libc/include/inttypes.h b/newlib/libc/include/inttypes.h index 073215476..570ed0481 100644 --- a/newlib/libc/include/inttypes.h +++ b/newlib/libc/include/inttypes.h @@ -320,7 +320,7 @@ struct _reent; extern "C" { #endif -extern intmax_t imaxabs(intmax_t j); +extern intmax_t imaxabs(intmax_t); extern imaxdiv_t imaxdiv(intmax_t numer, intmax_t denomer); extern intmax_t strtoimax(const char *__restrict, char **__restrict, int); extern intmax_t _strtoimax_r(struct _reent *, const char *__restrict, char **__restrict, int); -- 2.18.0 [-- Attachment #2: 0001-libc-include-inttypes.h-Remove-parameter-name.patch --] [-- Type: application/octet-stream, Size: 1461 bytes --] From 70e162623e37558ed5576518c436adbba6f3818c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torbj=C3=B6rn=20SVENSSON?= <torbjorn.svensson@st.com> Date: Thu, 1 Oct 2020 12:44:43 +0200 Subject: [PATCH 1/3] libc/include/inttypes.h: Remove parameter name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As discussed in GCC bug 97088 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97088), parameters in prototypes of library functions should use reserved names, or no name at all. This patch removes the 'j' parameter name from extern intmax_t imaxabs(intmax_t); to avoid possible clashes with user code in case someone uses before including Newlib's inttypes.h (or uses some other conflicting definition) Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@st.com> --- newlib/libc/include/inttypes.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/newlib/libc/include/inttypes.h b/newlib/libc/include/inttypes.h index 073215476..570ed0481 100644 --- a/newlib/libc/include/inttypes.h +++ b/newlib/libc/include/inttypes.h @@ -320,7 +320,7 @@ struct _reent; extern "C" { #endif -extern intmax_t imaxabs(intmax_t j); +extern intmax_t imaxabs(intmax_t); extern imaxdiv_t imaxdiv(intmax_t numer, intmax_t denomer); extern intmax_t strtoimax(const char *__restrict, char **__restrict, int); extern intmax_t _strtoimax_r(struct _reent *, const char *__restrict, char **__restrict, int); -- 2.18.0 ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH 1/3] libc/include/inttypes.h: Remove parameter name 2020-10-02 6:52 ` Torbjorn SVENSSON @ 2020-10-02 6:53 ` nrupp 2020-10-02 21:01 ` Jeff Johnston 1 sibling, 0 replies; 21+ messages in thread From: nrupp @ 2020-10-02 6:53 UTC (permalink / raw) To: Torbjorn SVENSSON via Newlib <html><head><meta http-equiv="Content-Type" content="text/html; charset=UTF-8"></head><body ><div>Ja</div><br><br><br><br><div>-------- Ursprüngliche Nachricht --------</div><div>Von: Torbjorn SVENSSON via Newlib <newlib@sourceware.org> </div><div>Datum: 02.10.20 08:52 (GMT+01:00) </div><div>An: Jeff Johnston <jjohnstn@redhat.com> </div><div>Cc: Newlib <newlib@sourceware.org> </div><div>Betreff: RE: [PATCH 1/3] libc/include/inttypes.h: Remove parameter name </div><div><br></div>null null ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/3] libc/include/inttypes.h: Remove parameter name 2020-10-02 6:52 ` Torbjorn SVENSSON 2020-10-02 6:53 ` nrupp @ 2020-10-02 21:01 ` Jeff Johnston 1 sibling, 0 replies; 21+ messages in thread From: Jeff Johnston @ 2020-10-02 21:01 UTC (permalink / raw) To: Torbjorn SVENSSON; +Cc: Newlib Patch applied. Thanks. -- Jeff J. On Fri, Oct 2, 2020 at 2:52 AM Torbjorn SVENSSON <torbjorn.svensson@st.com> wrote: > Patch attached. > > > > *From:* Jeff Johnston <jjohnstn@redhat.com> > *Sent:* den 2 oktober 2020 01:23 > *To:* Torbjorn SVENSSON <torbjorn.svensson@st.com> > *Cc:* Newlib <newlib@sourceware.org> > *Subject:* Re: [PATCH 1/3] libc/include/inttypes.h: Remove parameter name > > > > Looks fine. Could you please resend the patch as an attachment? > > > > Thanks, > > > > -- Jeff J. > > > > On Thu, Oct 1, 2020 at 10:19 AM Torbjörn SVENSSON via Newlib < > newlib@sourceware.org> wrote: > > As discussed in GCC bug 97088 > (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97088), parameters in > prototypes of library functions should use reserved names, or no name > at all. > > This patch removes the 'j' parameter name from > extern intmax_t imaxabs(intmax_t); > > to avoid possible clashes with user code in case someone uses > before including Newlib's inttypes.h (or uses some other conflicting > definition) > > Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@st.com> > --- > newlib/libc/include/inttypes.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/newlib/libc/include/inttypes.h > b/newlib/libc/include/inttypes.h > index 073215476..570ed0481 100644 > --- a/newlib/libc/include/inttypes.h > +++ b/newlib/libc/include/inttypes.h > @@ -320,7 +320,7 @@ struct _reent; > extern "C" { > #endif > > -extern intmax_t imaxabs(intmax_t j); > +extern intmax_t imaxabs(intmax_t); > extern imaxdiv_t imaxdiv(intmax_t numer, intmax_t denomer); > extern intmax_t strtoimax(const char *__restrict, char **__restrict, > int); > extern intmax_t _strtoimax_r(struct _reent *, const char *__restrict, > char **__restrict, int); > -- > 2.18.0 > > ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/3] libc/include/wchar.h: Remove parameter name 2020-10-01 14:17 [PATCH 0/3] libc: Comply with the libstdc++ test case names.cc Torbjörn SVENSSON 2020-10-01 14:17 ` [PATCH 1/3] libc/include/inttypes.h: Remove parameter name Torbjörn SVENSSON @ 2020-10-01 14:17 ` Torbjörn SVENSSON 2020-10-01 23:24 ` Jeff Johnston 2020-10-01 14:17 ` [PATCH 3/3] libc: Replace one letter member names in __tzrule_struct Torbjörn SVENSSON 2 siblings, 1 reply; 21+ messages in thread From: Torbjörn SVENSSON @ 2020-10-01 14:17 UTC (permalink / raw) To: newlib As discussed in GCC bug 97088 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97088), parameters in prototypes of library functions should use reserved names, or no name at all. This patch removes the 'ptr' parameter name from wint_t _getwchar_r (struct _reent *); wint_t _getwchar_unlocked_r (struct _reent *); to avoid possible clashes with user code in case someone uses before including Newlib's wchar.h (or uses some other conflicting definition) Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@st.com> --- newlib/libc/include/wchar.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/newlib/libc/include/wchar.h b/newlib/libc/include/wchar.h index c04a6510e..0d3e636f9 100644 --- a/newlib/libc/include/wchar.h +++ b/newlib/libc/include/wchar.h @@ -248,8 +248,8 @@ int _fputws_unlocked_r (struct _reent *, const wchar_t *, __FILE *); int _fwide_r (struct _reent *, __FILE *, int); wint_t _getwc_r (struct _reent *, __FILE *); wint_t _getwc_unlocked_r (struct _reent *, __FILE *); -wint_t _getwchar_r (struct _reent *ptr); -wint_t _getwchar_unlocked_r (struct _reent *ptr); +wint_t _getwchar_r (struct _reent *); +wint_t _getwchar_unlocked_r (struct _reent *); wint_t _putwc_r (struct _reent *, wchar_t, __FILE *); wint_t _putwc_unlocked_r (struct _reent *, wchar_t, __FILE *); wint_t _putwchar_r (struct _reent *, wchar_t); -- 2.18.0 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] libc/include/wchar.h: Remove parameter name 2020-10-01 14:17 ` [PATCH 2/3] libc/include/wchar.h: " Torbjörn SVENSSON @ 2020-10-01 23:24 ` Jeff Johnston 2020-10-02 6:53 ` Torbjorn SVENSSON 0 siblings, 1 reply; 21+ messages in thread From: Jeff Johnston @ 2020-10-01 23:24 UTC (permalink / raw) To: Torbjörn SVENSSON; +Cc: Newlib Looks good. Could you please resend the patch as an attachment? Thanks, -- Jeff J. On Thu, Oct 1, 2020 at 10:19 AM Torbjörn SVENSSON via Newlib < newlib@sourceware.org> wrote: > As discussed in GCC bug 97088 > (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97088), parameters in > prototypes of library functions should use reserved names, or no name > at all. > > This patch removes the 'ptr' parameter name from > wint_t _getwchar_r (struct _reent *); > wint_t _getwchar_unlocked_r (struct _reent *); > > to avoid possible clashes with user code in case someone uses > before including Newlib's wchar.h (or uses some other conflicting > definition) > > Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@st.com> > --- > newlib/libc/include/wchar.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/newlib/libc/include/wchar.h b/newlib/libc/include/wchar.h > index c04a6510e..0d3e636f9 100644 > --- a/newlib/libc/include/wchar.h > +++ b/newlib/libc/include/wchar.h > @@ -248,8 +248,8 @@ int _fputws_unlocked_r (struct _reent *, const wchar_t > *, __FILE *); > int _fwide_r (struct _reent *, __FILE *, int); > wint_t _getwc_r (struct _reent *, __FILE *); > wint_t _getwc_unlocked_r (struct _reent *, __FILE *); > -wint_t _getwchar_r (struct _reent *ptr); > -wint_t _getwchar_unlocked_r (struct _reent *ptr); > +wint_t _getwchar_r (struct _reent *); > +wint_t _getwchar_unlocked_r (struct _reent *); > wint_t _putwc_r (struct _reent *, wchar_t, __FILE *); > wint_t _putwc_unlocked_r (struct _reent *, wchar_t, __FILE *); > wint_t _putwchar_r (struct _reent *, wchar_t); > -- > 2.18.0 > > ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH 2/3] libc/include/wchar.h: Remove parameter name 2020-10-01 23:24 ` Jeff Johnston @ 2020-10-02 6:53 ` Torbjorn SVENSSON 2020-10-02 21:02 ` Jeff Johnston 0 siblings, 1 reply; 21+ messages in thread From: Torbjorn SVENSSON @ 2020-10-02 6:53 UTC (permalink / raw) To: Jeff Johnston; +Cc: Newlib [-- Attachment #1: Type: text/plain, Size: 1911 bytes --] Patch attached. From: Jeff Johnston <jjohnstn@redhat.com> Sent: den 2 oktober 2020 01:24 To: Torbjorn SVENSSON <torbjorn.svensson@st.com> Cc: Newlib <newlib@sourceware.org> Subject: Re: [PATCH 2/3] libc/include/wchar.h: Remove parameter name Looks good. Could you please resend the patch as an attachment? Thanks, -- Jeff J. On Thu, Oct 1, 2020 at 10:19 AM Torbjörn SVENSSON via Newlib <newlib@sourceware.org<mailto:newlib@sourceware.org>> wrote: As discussed in GCC bug 97088 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97088), parameters in prototypes of library functions should use reserved names, or no name at all. This patch removes the 'ptr' parameter name from wint_t _getwchar_r (struct _reent *); wint_t _getwchar_unlocked_r (struct _reent *); to avoid possible clashes with user code in case someone uses before including Newlib's wchar.h (or uses some other conflicting definition) Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@st.com<mailto:torbjorn.svensson@st.com>> --- newlib/libc/include/wchar.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/newlib/libc/include/wchar.h b/newlib/libc/include/wchar.h index c04a6510e..0d3e636f9 100644 --- a/newlib/libc/include/wchar.h +++ b/newlib/libc/include/wchar.h @@ -248,8 +248,8 @@ int _fputws_unlocked_r (struct _reent *, const wchar_t *, __FILE *); int _fwide_r (struct _reent *, __FILE *, int); wint_t _getwc_r (struct _reent *, __FILE *); wint_t _getwc_unlocked_r (struct _reent *, __FILE *); -wint_t _getwchar_r (struct _reent *ptr); -wint_t _getwchar_unlocked_r (struct _reent *ptr); +wint_t _getwchar_r (struct _reent *); +wint_t _getwchar_unlocked_r (struct _reent *); wint_t _putwc_r (struct _reent *, wchar_t, __FILE *); wint_t _putwc_unlocked_r (struct _reent *, wchar_t, __FILE *); wint_t _putwchar_r (struct _reent *, wchar_t); -- 2.18.0 [-- Attachment #2: 0002-libc-include-wchar.h-Remove-parameter-name.patch --] [-- Type: application/octet-stream, Size: 1710 bytes --] From a9ef14712398565ad78998d9ecdd5fab09090f78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torbj=C3=B6rn=20SVENSSON?= <torbjorn.svensson@st.com> Date: Thu, 1 Oct 2020 12:46:51 +0200 Subject: [PATCH 2/3] libc/include/wchar.h: Remove parameter name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As discussed in GCC bug 97088 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97088), parameters in prototypes of library functions should use reserved names, or no name at all. This patch removes the 'ptr' parameter name from wint_t _getwchar_r (struct _reent *); wint_t _getwchar_unlocked_r (struct _reent *); to avoid possible clashes with user code in case someone uses before including Newlib's wchar.h (or uses some other conflicting definition) Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@st.com> --- newlib/libc/include/wchar.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/newlib/libc/include/wchar.h b/newlib/libc/include/wchar.h index c04a6510e..0d3e636f9 100644 --- a/newlib/libc/include/wchar.h +++ b/newlib/libc/include/wchar.h @@ -248,8 +248,8 @@ int _fputws_unlocked_r (struct _reent *, const wchar_t *, __FILE *); int _fwide_r (struct _reent *, __FILE *, int); wint_t _getwc_r (struct _reent *, __FILE *); wint_t _getwc_unlocked_r (struct _reent *, __FILE *); -wint_t _getwchar_r (struct _reent *ptr); -wint_t _getwchar_unlocked_r (struct _reent *ptr); +wint_t _getwchar_r (struct _reent *); +wint_t _getwchar_unlocked_r (struct _reent *); wint_t _putwc_r (struct _reent *, wchar_t, __FILE *); wint_t _putwc_unlocked_r (struct _reent *, wchar_t, __FILE *); wint_t _putwchar_r (struct _reent *, wchar_t); -- 2.18.0 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] libc/include/wchar.h: Remove parameter name 2020-10-02 6:53 ` Torbjorn SVENSSON @ 2020-10-02 21:02 ` Jeff Johnston 0 siblings, 0 replies; 21+ messages in thread From: Jeff Johnston @ 2020-10-02 21:02 UTC (permalink / raw) To: Torbjorn SVENSSON; +Cc: Newlib Patch applied. Thanks. -- Jeff J. On Fri, Oct 2, 2020 at 2:53 AM Torbjorn SVENSSON <torbjorn.svensson@st.com> wrote: > Patch attached. > > > > *From:* Jeff Johnston <jjohnstn@redhat.com> > *Sent:* den 2 oktober 2020 01:24 > *To:* Torbjorn SVENSSON <torbjorn.svensson@st.com> > *Cc:* Newlib <newlib@sourceware.org> > *Subject:* Re: [PATCH 2/3] libc/include/wchar.h: Remove parameter name > > > > Looks good. Could you please resend the patch as an attachment? > > > > Thanks, > > > > -- Jeff J. > > > > On Thu, Oct 1, 2020 at 10:19 AM Torbjörn SVENSSON via Newlib < > newlib@sourceware.org> wrote: > > As discussed in GCC bug 97088 > (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97088), parameters in > prototypes of library functions should use reserved names, or no name > at all. > > This patch removes the 'ptr' parameter name from > wint_t _getwchar_r (struct _reent *); > wint_t _getwchar_unlocked_r (struct _reent *); > > to avoid possible clashes with user code in case someone uses > before including Newlib's wchar.h (or uses some other conflicting > definition) > > Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@st.com> > --- > newlib/libc/include/wchar.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/newlib/libc/include/wchar.h b/newlib/libc/include/wchar.h > index c04a6510e..0d3e636f9 100644 > --- a/newlib/libc/include/wchar.h > +++ b/newlib/libc/include/wchar.h > @@ -248,8 +248,8 @@ int _fputws_unlocked_r (struct _reent *, const wchar_t > *, __FILE *); > int _fwide_r (struct _reent *, __FILE *, int); > wint_t _getwc_r (struct _reent *, __FILE *); > wint_t _getwc_unlocked_r (struct _reent *, __FILE *); > -wint_t _getwchar_r (struct _reent *ptr); > -wint_t _getwchar_unlocked_r (struct _reent *ptr); > +wint_t _getwchar_r (struct _reent *); > +wint_t _getwchar_unlocked_r (struct _reent *); > wint_t _putwc_r (struct _reent *, wchar_t, __FILE *); > wint_t _putwc_unlocked_r (struct _reent *, wchar_t, __FILE *); > wint_t _putwchar_r (struct _reent *, wchar_t); > -- > 2.18.0 > > ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 3/3] libc: Replace one letter member names in __tzrule_struct 2020-10-01 14:17 [PATCH 0/3] libc: Comply with the libstdc++ test case names.cc Torbjörn SVENSSON 2020-10-01 14:17 ` [PATCH 1/3] libc/include/inttypes.h: Remove parameter name Torbjörn SVENSSON 2020-10-01 14:17 ` [PATCH 2/3] libc/include/wchar.h: " Torbjörn SVENSSON @ 2020-10-01 14:17 ` Torbjörn SVENSSON 2020-10-01 23:21 ` Jeff Johnston 2 siblings, 1 reply; 21+ messages in thread From: Torbjörn SVENSSON @ 2020-10-01 14:17 UTC (permalink / raw) To: newlib As discussed in GCC bug 97088 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97088), parameters in prototypes of library functions should use reserved names, or no name at all. This patch replaces 'm', 'n', 'd' and 's' members in 'struct __tzrule_struct' to avoid possible clashes with user code in case someone uses before including Newlib's time.h (or uses some other conflicting definition) Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@st.com> --- newlib/libc/include/time.h | 8 ++++---- newlib/libc/time/tzcalc_limits.c | 14 +++++++------- newlib/libc/time/tzset_r.c | 22 +++++++++++----------- 3 files changed, 22 insertions(+), 22 deletions(-) diff --git a/newlib/libc/include/time.h b/newlib/libc/include/time.h index 3031590b4..6a540537f 100644 --- a/newlib/libc/include/time.h +++ b/newlib/libc/include/time.h @@ -105,10 +105,10 @@ void _tzset_r (struct _reent *); typedef struct __tzrule_struct { char ch; - int m; - int n; - int d; - int s; + int month; /* Month of year if ch=M */ + int week; /* Week of month if ch=M */ + int day; /* Day of week if ch=M, day of year if ch=J or ch=D */ + int secs; /* Time of day in seconds */ time_t change; long offset; /* Match type of _timezone. */ } __tzrule_type; diff --git a/newlib/libc/time/tzcalc_limits.c b/newlib/libc/time/tzcalc_limits.c index e0ea6549c..b2163ed3d 100644 --- a/newlib/libc/time/tzcalc_limits.c +++ b/newlib/libc/time/tzcalc_limits.c @@ -34,13 +34,13 @@ __tzcalc_limits (int year) if (tz->__tzrule[i].ch == 'J') { /* The Julian day n (1 <= n <= 365). */ - days = year_days + tz->__tzrule[i].d + - (isleap(year) && tz->__tzrule[i].d >= 60); + days = year_days + tz->__tzrule[i].day + + (isleap(year) && tz->__tzrule[i].day >= 60); /* Convert to yday */ --days; } else if (tz->__tzrule[i].ch == 'D') - days = year_days + tz->__tzrule[i].d; + days = year_days + tz->__tzrule[i].day; else { const int yleap = isleap(year); @@ -49,15 +49,15 @@ __tzcalc_limits (int year) days = year_days; - for (j = 1; j < tz->__tzrule[i].m; ++j) + for (j = 1; j < tz->__tzrule[i].month; ++j) days += ip[j-1]; m_wday = (EPOCH_WDAY + days) % DAYSPERWEEK; - wday_diff = tz->__tzrule[i].d - m_wday; + wday_diff = tz->__tzrule[i].day - m_wday; if (wday_diff < 0) wday_diff += DAYSPERWEEK; - m_day = (tz->__tzrule[i].n - 1) * DAYSPERWEEK + wday_diff; + m_day = (tz->__tzrule[i].week - 1) * DAYSPERWEEK + wday_diff; while (m_day >= ip[j-1]) m_day -= DAYSPERWEEK; @@ -67,7 +67,7 @@ __tzcalc_limits (int year) /* store the change-over time in GMT form by adding offset */ tz->__tzrule[i].change = days * SECSPERDAY + - tz->__tzrule[i].s + tz->__tzrule[i].offset; + tz->__tzrule[i].secs + tz->__tzrule[i].offset; } tz->__tznorth = (tz->__tzrule[0].change < tz->__tzrule[1].change); diff --git a/newlib/libc/time/tzset_r.c b/newlib/libc/time/tzset_r.c index 9e0cf834b..7117b51e6 100644 --- a/newlib/libc/time/tzset_r.c +++ b/newlib/libc/time/tzset_r.c @@ -115,9 +115,9 @@ _tzset_unlocked_r (struct _reent *reent_ptr) return; tz->__tzrule[i].ch = 'M'; - tz->__tzrule[i].m = m; - tz->__tzrule[i].n = w; - tz->__tzrule[i].d = d; + tz->__tzrule[i].month = m; + tz->__tzrule[i].week = w; + tz->__tzrule[i].day = d; tzenv += n; } @@ -142,22 +142,22 @@ _tzset_unlocked_r (struct _reent *reent_ptr) if (i == 0) { tz->__tzrule[0].ch = 'M'; - tz->__tzrule[0].m = 3; - tz->__tzrule[0].n = 2; - tz->__tzrule[0].d = 0; + tz->__tzrule[0].month = 3; + tz->__tzrule[0].week = 2; + tz->__tzrule[0].day = 0; } else { tz->__tzrule[1].ch = 'M'; - tz->__tzrule[1].m = 11; - tz->__tzrule[1].n = 1; - tz->__tzrule[1].d = 0; + tz->__tzrule[1].month = 11; + tz->__tzrule[1].week = 1; + tz->__tzrule[1].day = 0; } } else { tz->__tzrule[i].ch = ch; - tz->__tzrule[i].d = d; + tz->__tzrule[i].day = d; } tzenv = end; @@ -172,7 +172,7 @@ _tzset_unlocked_r (struct _reent *reent_ptr) if (*tzenv == '/') sscanf (tzenv, "/%hu%n:%hu%n:%hu%n", &hh, &n, &mm, &n, &ss, &n); - tz->__tzrule[i].s = ss + SECSPERMIN * mm + SECSPERHOUR * hh; + tz->__tzrule[i].secs = ss + SECSPERMIN * mm + SECSPERHOUR * hh; tzenv += n; } -- 2.18.0 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/3] libc: Replace one letter member names in __tzrule_struct 2020-10-01 14:17 ` [PATCH 3/3] libc: Replace one letter member names in __tzrule_struct Torbjörn SVENSSON @ 2020-10-01 23:21 ` Jeff Johnston 2020-10-02 7:36 ` Torbjorn SVENSSON 0 siblings, 1 reply; 21+ messages in thread From: Jeff Johnston @ 2020-10-01 23:21 UTC (permalink / raw) To: Torbjörn SVENSSON; +Cc: Newlib Hello, while I fully agree there is an issue (the struct was due to my initial check-in in 2005 based on glibc), the change will break API and thus technically requires a major release bump of newlib. I would prefer to wait for something else to require a major bump before making such a change. In such a case, I also believe that the patch should either use double-underscores for the field names (e.g. __month) or hide the struct from regular users of time.h. -- Jeff J. On Thu, Oct 1, 2020 at 10:19 AM Torbjörn SVENSSON via Newlib < newlib@sourceware.org> wrote: > As discussed in GCC bug 97088 > (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97088), parameters in > prototypes of library functions should use reserved names, or no name > at all. > > This patch replaces 'm', 'n', 'd' and 's' members in > 'struct __tzrule_struct' to avoid possible clashes with user code in > case someone uses before including Newlib's time.h (or uses some > other conflicting definition) > > Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@st.com> > --- > newlib/libc/include/time.h | 8 ++++---- > newlib/libc/time/tzcalc_limits.c | 14 +++++++------- > newlib/libc/time/tzset_r.c | 22 +++++++++++----------- > 3 files changed, 22 insertions(+), 22 deletions(-) > > diff --git a/newlib/libc/include/time.h b/newlib/libc/include/time.h > index 3031590b4..6a540537f 100644 > --- a/newlib/libc/include/time.h > +++ b/newlib/libc/include/time.h > @@ -105,10 +105,10 @@ void _tzset_r (struct _reent *); > typedef struct __tzrule_struct > { > char ch; > - int m; > - int n; > - int d; > - int s; > + int month; /* Month of year if ch=M */ > + int week; /* Week of month if ch=M */ > + int day; /* Day of week if ch=M, day of year if ch=J or ch=D */ > + int secs; /* Time of day in seconds */ > time_t change; > long offset; /* Match type of _timezone. */ > } __tzrule_type; > diff --git a/newlib/libc/time/tzcalc_limits.c > b/newlib/libc/time/tzcalc_limits.c > index e0ea6549c..b2163ed3d 100644 > --- a/newlib/libc/time/tzcalc_limits.c > +++ b/newlib/libc/time/tzcalc_limits.c > @@ -34,13 +34,13 @@ __tzcalc_limits (int year) > if (tz->__tzrule[i].ch == 'J') > { > /* The Julian day n (1 <= n <= 365). */ > - days = year_days + tz->__tzrule[i].d + > - (isleap(year) && tz->__tzrule[i].d >= 60); > + days = year_days + tz->__tzrule[i].day + > + (isleap(year) && tz->__tzrule[i].day >= 60); > /* Convert to yday */ > --days; > } > else if (tz->__tzrule[i].ch == 'D') > - days = year_days + tz->__tzrule[i].d; > + days = year_days + tz->__tzrule[i].day; > else > { > const int yleap = isleap(year); > @@ -49,15 +49,15 @@ __tzcalc_limits (int year) > > days = year_days; > > - for (j = 1; j < tz->__tzrule[i].m; ++j) > + for (j = 1; j < tz->__tzrule[i].month; ++j) > days += ip[j-1]; > > m_wday = (EPOCH_WDAY + days) % DAYSPERWEEK; > > - wday_diff = tz->__tzrule[i].d - m_wday; > + wday_diff = tz->__tzrule[i].day - m_wday; > if (wday_diff < 0) > wday_diff += DAYSPERWEEK; > - m_day = (tz->__tzrule[i].n - 1) * DAYSPERWEEK + wday_diff; > + m_day = (tz->__tzrule[i].week - 1) * DAYSPERWEEK + wday_diff; > > while (m_day >= ip[j-1]) > m_day -= DAYSPERWEEK; > @@ -67,7 +67,7 @@ __tzcalc_limits (int year) > > /* store the change-over time in GMT form by adding offset */ > tz->__tzrule[i].change = days * SECSPERDAY + > - tz->__tzrule[i].s + tz->__tzrule[i].offset; > + tz->__tzrule[i].secs + tz->__tzrule[i].offset; > } > > tz->__tznorth = (tz->__tzrule[0].change < tz->__tzrule[1].change); > diff --git a/newlib/libc/time/tzset_r.c b/newlib/libc/time/tzset_r.c > index 9e0cf834b..7117b51e6 100644 > --- a/newlib/libc/time/tzset_r.c > +++ b/newlib/libc/time/tzset_r.c > @@ -115,9 +115,9 @@ _tzset_unlocked_r (struct _reent *reent_ptr) > return; > > tz->__tzrule[i].ch = 'M'; > - tz->__tzrule[i].m = m; > - tz->__tzrule[i].n = w; > - tz->__tzrule[i].d = d; > + tz->__tzrule[i].month = m; > + tz->__tzrule[i].week = w; > + tz->__tzrule[i].day = d; > > tzenv += n; > } > @@ -142,22 +142,22 @@ _tzset_unlocked_r (struct _reent *reent_ptr) > if (i == 0) > { > tz->__tzrule[0].ch = 'M'; > - tz->__tzrule[0].m = 3; > - tz->__tzrule[0].n = 2; > - tz->__tzrule[0].d = 0; > + tz->__tzrule[0].month = 3; > + tz->__tzrule[0].week = 2; > + tz->__tzrule[0].day = 0; > } > else > { > tz->__tzrule[1].ch = 'M'; > - tz->__tzrule[1].m = 11; > - tz->__tzrule[1].n = 1; > - tz->__tzrule[1].d = 0; > + tz->__tzrule[1].month = 11; > + tz->__tzrule[1].week = 1; > + tz->__tzrule[1].day = 0; > } > } > else > { > tz->__tzrule[i].ch = ch; > - tz->__tzrule[i].d = d; > + tz->__tzrule[i].day = d; > } > > tzenv = end; > @@ -172,7 +172,7 @@ _tzset_unlocked_r (struct _reent *reent_ptr) > if (*tzenv == '/') > sscanf (tzenv, "/%hu%n:%hu%n:%hu%n", &hh, &n, &mm, &n, &ss, &n); > > - tz->__tzrule[i].s = ss + SECSPERMIN * mm + SECSPERHOUR * hh; > + tz->__tzrule[i].secs = ss + SECSPERMIN * mm + SECSPERHOUR * hh; > > tzenv += n; > } > -- > 2.18.0 > > ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH 3/3] libc: Replace one letter member names in __tzrule_struct 2020-10-01 23:21 ` Jeff Johnston @ 2020-10-02 7:36 ` Torbjorn SVENSSON 2020-10-02 21:04 ` Jeff Johnston 0 siblings, 1 reply; 21+ messages in thread From: Torbjorn SVENSSON @ 2020-10-02 7:36 UTC (permalink / raw) To: Jeff Johnston; +Cc: Newlib Hello! Thanks for the feedback Jeff. Regarding the alternative to using "__" prefix for the members; would it be better to move line 105-123 from newlib/libc/include/time.h to newlib/libc/time/local.h, thus not exposing it in the API? The lines in question are: typedef struct __tzrule_struct { char ch; int m; int n; int d; int s; time_t change; long offset; /* Match type of _timezone. */ } __tzrule_type; typedef struct __tzinfo_struct { int __tznorth; int __tzyear; __tzrule_type __tzrule[2]; } __tzinfo_type; __tzinfo_type *__gettzinfo (void); Would this change be easier to get accepted? If this approach is better, I guess the single letter member names can be kept too… While I took another look at the change, I also noticed that the struct definition (not usage) is also present in these files: newlib/libc/sys/linux/include/time.h newlib/libc/sys/phoenix/include/time.h I’m not sure in what situations those files are used and when the time.h file that I modified is used, but I guess they should be in sync regardless. Thanks Torbjörn From: Jeff Johnston <jjohnstn@redhat.com> Sent: den 2 oktober 2020 01:21 To: Torbjorn SVENSSON <torbjorn.svensson@st.com> Cc: Newlib <newlib@sourceware.org> Subject: Re: [PATCH 3/3] libc: Replace one letter member names in __tzrule_struct Hello, while I fully agree there is an issue (the struct was due to my initial check-in in 2005 based on glibc), the change will break API and thus technically requires a major release bump of newlib. I would prefer to wait for something else to require a major bump before making such a change. In such a case, I also believe that the patch should either use double-underscores for the field names (e.g. __month) or hide the struct from regular users of time.h. -- Jeff J. On Thu, Oct 1, 2020 at 10:19 AM Torbjörn SVENSSON via Newlib <mailto:newlib@sourceware.org> wrote: As discussed in GCC bug 97088 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97088), parameters in prototypes of library functions should use reserved names, or no name at all. This patch replaces 'm', 'n', 'd' and 's' members in 'struct __tzrule_struct' to avoid possible clashes with user code in case someone uses before including Newlib's time.h (or uses some other conflicting definition) Signed-off-by: Torbjörn SVENSSON <mailto:torbjorn.svensson@st.com> --- newlib/libc/include/time.h | 8 ++++---- newlib/libc/time/tzcalc_limits.c | 14 +++++++------- newlib/libc/time/tzset_r.c | 22 +++++++++++----------- 3 files changed, 22 insertions(+), 22 deletions(-) diff --git a/newlib/libc/include/time.h b/newlib/libc/include/time.h index 3031590b4..6a540537f 100644 --- a/newlib/libc/include/time.h +++ b/newlib/libc/include/time.h @@ -105,10 +105,10 @@ void _tzset_r (struct _reent *); typedef struct __tzrule_struct { char ch; - int m; - int n; - int d; - int s; + int month; /* Month of year if ch=M */ + int week; /* Week of month if ch=M */ + int day; /* Day of week if ch=M, day of year if ch=J or ch=D */ + int secs; /* Time of day in seconds */ time_t change; long offset; /* Match type of _timezone. */ } __tzrule_type; diff --git a/newlib/libc/time/tzcalc_limits.c b/newlib/libc/time/tzcalc_limits.c index e0ea6549c..b2163ed3d 100644 --- a/newlib/libc/time/tzcalc_limits.c +++ b/newlib/libc/time/tzcalc_limits.c @@ -34,13 +34,13 @@ __tzcalc_limits (int year) if (tz->__tzrule[i].ch == 'J') { /* The Julian day n (1 <= n <= 365). */ - days = year_days + tz->__tzrule[i].d + - (isleap(year) && tz->__tzrule[i].d >= 60); + days = year_days + tz->__tzrule[i].day + + (isleap(year) && tz->__tzrule[i].day >= 60); /* Convert to yday */ --days; } else if (tz->__tzrule[i].ch == 'D') - days = year_days + tz->__tzrule[i].d; + days = year_days + tz->__tzrule[i].day; else { const int yleap = isleap(year); @@ -49,15 +49,15 @@ __tzcalc_limits (int year) days = year_days; - for (j = 1; j < tz->__tzrule[i].m; ++j) + for (j = 1; j < tz->__tzrule[i].month; ++j) days += ip[j-1]; m_wday = (EPOCH_WDAY + days) % DAYSPERWEEK; - wday_diff = tz->__tzrule[i].d - m_wday; + wday_diff = tz->__tzrule[i].day - m_wday; if (wday_diff < 0) wday_diff += DAYSPERWEEK; - m_day = (tz->__tzrule[i].n - 1) * DAYSPERWEEK + wday_diff; + m_day = (tz->__tzrule[i].week - 1) * DAYSPERWEEK + wday_diff; while (m_day >= ip[j-1]) m_day -= DAYSPERWEEK; @@ -67,7 +67,7 @@ __tzcalc_limits (int year) /* store the change-over time in GMT form by adding offset */ tz->__tzrule[i].change = days * SECSPERDAY + - tz->__tzrule[i].s + tz->__tzrule[i].offset; + tz->__tzrule[i].secs + tz->__tzrule[i].offset; } tz->__tznorth = (tz->__tzrule[0].change < tz->__tzrule[1].change); diff --git a/newlib/libc/time/tzset_r.c b/newlib/libc/time/tzset_r.c index 9e0cf834b..7117b51e6 100644 --- a/newlib/libc/time/tzset_r.c +++ b/newlib/libc/time/tzset_r.c @@ -115,9 +115,9 @@ _tzset_unlocked_r (struct _reent *reent_ptr) return; tz->__tzrule[i].ch = 'M'; - tz->__tzrule[i].m = m; - tz->__tzrule[i].n = w; - tz->__tzrule[i].d = d; + tz->__tzrule[i].month = m; + tz->__tzrule[i].week = w; + tz->__tzrule[i].day = d; tzenv += n; } @@ -142,22 +142,22 @@ _tzset_unlocked_r (struct _reent *reent_ptr) if (i == 0) { tz->__tzrule[0].ch = 'M'; - tz->__tzrule[0].m = 3; - tz->__tzrule[0].n = 2; - tz->__tzrule[0].d = 0; + tz->__tzrule[0].month = 3; + tz->__tzrule[0].week = 2; + tz->__tzrule[0].day = 0; } else { tz->__tzrule[1].ch = 'M'; - tz->__tzrule[1].m = 11; - tz->__tzrule[1].n = 1; - tz->__tzrule[1].d = 0; + tz->__tzrule[1].month = 11; + tz->__tzrule[1].week = 1; + tz->__tzrule[1].day = 0; } } else { tz->__tzrule[i].ch = ch; - tz->__tzrule[i].d = d; + tz->__tzrule[i].day = d; } tzenv = end; @@ -172,7 +172,7 @@ _tzset_unlocked_r (struct _reent *reent_ptr) if (*tzenv == '/') sscanf (tzenv, "/%hu%n:%hu%n:%hu%n", &hh, &n, &mm, &n, &ss, &n); - tz->__tzrule[i].s = ss + SECSPERMIN * mm + SECSPERHOUR * hh; + tz->__tzrule[i].secs = ss + SECSPERMIN * mm + SECSPERHOUR * hh; tzenv += n; } -- 2.18.0 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/3] libc: Replace one letter member names in __tzrule_struct 2020-10-02 7:36 ` Torbjorn SVENSSON @ 2020-10-02 21:04 ` Jeff Johnston 2020-10-05 12:50 ` [PATCH v2] libc/time: Move internal newlib tz-structs to local.h Torbjörn SVENSSON 0 siblings, 1 reply; 21+ messages in thread From: Jeff Johnston @ 2020-10-02 21:04 UTC (permalink / raw) To: Torbjorn SVENSSON; +Cc: Newlib On Fri, Oct 2, 2020 at 3:36 AM Torbjorn SVENSSON <torbjorn.svensson@st.com> wrote: > Hello! > > Thanks for the feedback Jeff. > > Regarding the alternative to using "__" prefix for the members; would it > be better to move line 105-123 from newlib/libc/include/time.h to > newlib/libc/time/local.h, thus not exposing it in the API? > Yes. It was one of the alternatives I mentioned (hiding the struct from users of time.h). The struct is hidden in glibc and should have been hidden likewise in newlib. The lines in question are: > typedef struct __tzrule_struct > { > char ch; > int m; > int n; > int d; > int s; > time_t change; > long offset; /* Match type of _timezone. */ > } __tzrule_type; > > typedef struct __tzinfo_struct > { > int __tznorth; > int __tzyear; > __tzrule_type __tzrule[2]; > } __tzinfo_type; > > __tzinfo_type *__gettzinfo (void); > > > > Would this change be easier to get accepted? If this approach is better, I > guess the single letter member names can be kept too… > While I took another look at the change, I also noticed that the struct > definition (not usage) is also present in these files: > newlib/libc/sys/linux/include/time.h > newlib/libc/sys/phoenix/include/time.h > > I’m not sure in what situations those files are used and when the time.h > file that I modified is used, but I guess they should be in sync regardless. > > Thanks > Torbjörn > > From: Jeff Johnston <jjohnstn@redhat.com> > Sent: den 2 oktober 2020 01:21 > To: Torbjorn SVENSSON <torbjorn.svensson@st.com> > Cc: Newlib <newlib@sourceware.org> > Subject: Re: [PATCH 3/3] libc: Replace one letter member names in > __tzrule_struct > > Hello, while I fully agree there is an issue (the struct was due to my > initial check-in in 2005 based on glibc), the change will break API and > thus technically requires a major release bump of newlib. I would prefer > to wait for something else > to require a major bump before making such a change. In such a case, I > also believe that the patch should either use double-underscores for the > field names (e.g. __month) or hide the struct from regular users of time.h. > > -- Jeff J. > > On Thu, Oct 1, 2020 at 10:19 AM Torbjörn SVENSSON via Newlib <mailto: > newlib@sourceware.org> wrote: > As discussed in GCC bug 97088 > (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97088), parameters in > prototypes of library functions should use reserved names, or no name > at all. > > This patch replaces 'm', 'n', 'd' and 's' members in > 'struct __tzrule_struct' to avoid possible clashes with user code in > case someone uses before including Newlib's time.h (or uses some > other conflicting definition) > > Signed-off-by: Torbjörn SVENSSON <mailto:torbjorn.svensson@st.com> > --- > newlib/libc/include/time.h | 8 ++++---- > newlib/libc/time/tzcalc_limits.c | 14 +++++++------- > newlib/libc/time/tzset_r.c | 22 +++++++++++----------- > 3 files changed, 22 insertions(+), 22 deletions(-) > > diff --git a/newlib/libc/include/time.h b/newlib/libc/include/time.h > index 3031590b4..6a540537f 100644 > --- a/newlib/libc/include/time.h > +++ b/newlib/libc/include/time.h > @@ -105,10 +105,10 @@ void _tzset_r (struct _reent *); > typedef struct __tzrule_struct > { > char ch; > - int m; > - int n; > - int d; > - int s; > + int month; /* Month of year if ch=M */ > + int week; /* Week of month if ch=M */ > + int day; /* Day of week if ch=M, day of year if ch=J or ch=D */ > + int secs; /* Time of day in seconds */ > time_t change; > long offset; /* Match type of _timezone. */ > } __tzrule_type; > diff --git a/newlib/libc/time/tzcalc_limits.c > b/newlib/libc/time/tzcalc_limits.c > index e0ea6549c..b2163ed3d 100644 > --- a/newlib/libc/time/tzcalc_limits.c > +++ b/newlib/libc/time/tzcalc_limits.c > @@ -34,13 +34,13 @@ __tzcalc_limits (int year) > if (tz->__tzrule[i].ch == 'J') > { > /* The Julian day n (1 <= n <= 365). */ > - days = year_days + tz->__tzrule[i].d + > - (isleap(year) && tz->__tzrule[i].d >= 60); > + days = year_days + tz->__tzrule[i].day + > + (isleap(year) && tz->__tzrule[i].day >= 60); > /* Convert to yday */ > --days; > } > else if (tz->__tzrule[i].ch == 'D') > - days = year_days + tz->__tzrule[i].d; > + days = year_days + tz->__tzrule[i].day; > else > { > const int yleap = isleap(year); > @@ -49,15 +49,15 @@ __tzcalc_limits (int year) > > days = year_days; > > - for (j = 1; j < tz->__tzrule[i].m; ++j) > + for (j = 1; j < tz->__tzrule[i].month; ++j) > days += ip[j-1]; > > m_wday = (EPOCH_WDAY + days) % DAYSPERWEEK; > > - wday_diff = tz->__tzrule[i].d - m_wday; > + wday_diff = tz->__tzrule[i].day - m_wday; > if (wday_diff < 0) > wday_diff += DAYSPERWEEK; > - m_day = (tz->__tzrule[i].n - 1) * DAYSPERWEEK + wday_diff; > + m_day = (tz->__tzrule[i].week - 1) * DAYSPERWEEK + wday_diff; > > while (m_day >= ip[j-1]) > m_day -= DAYSPERWEEK; > @@ -67,7 +67,7 @@ __tzcalc_limits (int year) > > /* store the change-over time in GMT form by adding offset */ > tz->__tzrule[i].change = days * SECSPERDAY + > - tz->__tzrule[i].s + tz->__tzrule[i].offset; > + tz->__tzrule[i].secs + tz->__tzrule[i].offset; > } > > tz->__tznorth = (tz->__tzrule[0].change < tz->__tzrule[1].change); > diff --git a/newlib/libc/time/tzset_r.c b/newlib/libc/time/tzset_r.c > index 9e0cf834b..7117b51e6 100644 > --- a/newlib/libc/time/tzset_r.c > +++ b/newlib/libc/time/tzset_r.c > @@ -115,9 +115,9 @@ _tzset_unlocked_r (struct _reent *reent_ptr) > return; > > tz->__tzrule[i].ch = 'M'; > - tz->__tzrule[i].m = m; > - tz->__tzrule[i].n = w; > - tz->__tzrule[i].d = d; > + tz->__tzrule[i].month = m; > + tz->__tzrule[i].week = w; > + tz->__tzrule[i].day = d; > > tzenv += n; > } > @@ -142,22 +142,22 @@ _tzset_unlocked_r (struct _reent *reent_ptr) > if (i == 0) > { > tz->__tzrule[0].ch = 'M'; > - tz->__tzrule[0].m = 3; > - tz->__tzrule[0].n = 2; > - tz->__tzrule[0].d = 0; > + tz->__tzrule[0].month = 3; > + tz->__tzrule[0].week = 2; > + tz->__tzrule[0].day = 0; > } > else > { > tz->__tzrule[1].ch = 'M'; > - tz->__tzrule[1].m = 11; > - tz->__tzrule[1].n = 1; > - tz->__tzrule[1].d = 0; > + tz->__tzrule[1].month = 11; > + tz->__tzrule[1].week = 1; > + tz->__tzrule[1].day = 0; > } > } > else > { > tz->__tzrule[i].ch = ch; > - tz->__tzrule[i].d = d; > + tz->__tzrule[i].day = d; > } > > tzenv = end; > @@ -172,7 +172,7 @@ _tzset_unlocked_r (struct _reent *reent_ptr) > if (*tzenv == '/') > sscanf (tzenv, "/%hu%n:%hu%n:%hu%n", &hh, &n, &mm, &n, &ss, &n); > > - tz->__tzrule[i].s = ss + SECSPERMIN * mm + SECSPERHOUR * hh; > + tz->__tzrule[i].secs = ss + SECSPERMIN * mm + SECSPERHOUR * hh; > > tzenv += n; > } > -- > 2.18.0 > ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2] libc/time: Move internal newlib tz-structs to local.h 2020-10-02 21:04 ` Jeff Johnston @ 2020-10-05 12:50 ` Torbjörn SVENSSON 2020-10-15 6:52 ` Torbjorn SVENSSON 0 siblings, 1 reply; 21+ messages in thread From: Torbjörn SVENSSON @ 2020-10-05 12:50 UTC (permalink / raw) To: newlib As discussed in GCC bug 97088 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97088), parameters in prototypes of library functions should use reserved names, or no name at all. This patch moves the internal struct __tzrule_struct from the public API to the internal headerfile newlib/libc/time/local.h. Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@st.com> --- newlib/libc/include/time.h | 20 -------------------- newlib/libc/sys/linux/include/time.h | 20 -------------------- newlib/libc/sys/phoenix/include/time.h | 17 ----------------- newlib/libc/time/local.h | 19 +++++++++++++++++++ 4 files changed, 19 insertions(+), 57 deletions(-) diff --git a/newlib/libc/include/time.h b/newlib/libc/include/time.h index 3031590b4..ed6cc70fe 100644 --- a/newlib/libc/include/time.h +++ b/newlib/libc/include/time.h @@ -102,26 +102,6 @@ void tzset (void); #endif void _tzset_r (struct _reent *); -typedef struct __tzrule_struct -{ - char ch; - int m; - int n; - int d; - int s; - time_t change; - long offset; /* Match type of _timezone. */ -} __tzrule_type; - -typedef struct __tzinfo_struct -{ - int __tznorth; - int __tzyear; - __tzrule_type __tzrule[2]; -} __tzinfo_type; - -__tzinfo_type *__gettzinfo (void); - /* getdate functions */ #ifdef HAVE_GETDATE diff --git a/newlib/libc/sys/linux/include/time.h b/newlib/libc/sys/linux/include/time.h index 5e61d2b65..917a35858 100644 --- a/newlib/libc/sys/linux/include/time.h +++ b/newlib/libc/sys/linux/include/time.h @@ -84,26 +84,6 @@ char *strptime (const char *, const char *, struct tm *); void tzset (void); void _tzset_r (struct _reent *); -typedef struct __tzrule_struct -{ - char ch; - int m; - int n; - int d; - int s; - time_t change; - long offset; /* Match type of _timezone. */ -} __tzrule_type; - -typedef struct __tzinfo_struct -{ - int __tznorth; - int __tzyear; - __tzrule_type __tzrule[2]; -} __tzinfo_type; - -__tzinfo_type *__gettzinfo (void); - /* getdate functions */ #ifndef _REENT_ONLY diff --git a/newlib/libc/sys/phoenix/include/time.h b/newlib/libc/sys/phoenix/include/time.h index 3a9449c77..41fb137e4 100644 --- a/newlib/libc/sys/phoenix/include/time.h +++ b/newlib/libc/sys/phoenix/include/time.h @@ -40,23 +40,6 @@ extern char *_tzname[2]; #define tzname _tzname #endif -typedef struct __tzrule_struct { - char ch; - int m; - int n; - int d; - int s; - time_t change; - long offset; -} __tzrule_type; - -typedef struct __tzinfo_struct { - int __tznorth; - int __tzyear; - __tzrule_type __tzrule[2]; -} __tzinfo_type; - -__tzinfo_type *__gettzinfo(); void tzset(); clock_t clock(); diff --git a/newlib/libc/time/local.h b/newlib/libc/time/local.h index dce51cda2..290e1aee5 100644 --- a/newlib/libc/time/local.h +++ b/newlib/libc/time/local.h @@ -38,3 +38,22 @@ void _tzset_unlocked (void); void __tz_lock (void); void __tz_unlock (void); +typedef struct __tzrule_struct +{ + char ch; + int m; /* Month of year if ch=M */ + int n; /* Week of month if ch=M */ + int d; /* Day of week if ch=M, day of year if ch=J or ch=D */ + int s; /* Time of day in seconds */ + time_t change; + long offset; /* Match type of _timezone. */ +} __tzrule_type; + +typedef struct __tzinfo_struct +{ + int __tznorth; + int __tzyear; + __tzrule_type __tzrule[2]; +} __tzinfo_type; + +__tzinfo_type *__gettzinfo (void); -- 2.18.0 ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH v2] libc/time: Move internal newlib tz-structs to local.h 2020-10-05 12:50 ` [PATCH v2] libc/time: Move internal newlib tz-structs to local.h Torbjörn SVENSSON @ 2020-10-15 6:52 ` Torbjorn SVENSSON 2020-10-15 10:21 ` Corinna Vinschen 0 siblings, 1 reply; 21+ messages in thread From: Torbjorn SVENSSON @ 2020-10-15 6:52 UTC (permalink / raw) To: newlib Ping! -----Original Message----- From: Torbjorn SVENSSON <torbjorn.svensson@st.com> Sent: den 5 oktober 2020 14:50 To: newlib@sourceware.org Cc: Torbjorn SVENSSON <torbjorn.svensson@st.com>; jjohnstn@redhat.com Subject: [PATCH v2] libc/time: Move internal newlib tz-structs to local.h As discussed in GCC bug 97088 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97088), parameters in prototypes of library functions should use reserved names, or no name at all. This patch moves the internal struct __tzrule_struct from the public API to the internal headerfile newlib/libc/time/local.h. Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@st.com> --- newlib/libc/include/time.h | 20 -------------------- newlib/libc/sys/linux/include/time.h | 20 -------------------- newlib/libc/sys/phoenix/include/time.h | 17 ----------------- newlib/libc/time/local.h | 19 +++++++++++++++++++ 4 files changed, 19 insertions(+), 57 deletions(-) diff --git a/newlib/libc/include/time.h b/newlib/libc/include/time.h index 3031590b4..ed6cc70fe 100644 --- a/newlib/libc/include/time.h +++ b/newlib/libc/include/time.h @@ -102,26 +102,6 @@ void tzset (void); #endif void _tzset_r (struct _reent *); -typedef struct __tzrule_struct -{ - char ch; - int m; - int n; - int d; - int s; - time_t change; - long offset; /* Match type of _timezone. */ -} __tzrule_type; - -typedef struct __tzinfo_struct -{ - int __tznorth; - int __tzyear; - __tzrule_type __tzrule[2]; -} __tzinfo_type; - -__tzinfo_type *__gettzinfo (void); - /* getdate functions */ #ifdef HAVE_GETDATE diff --git a/newlib/libc/sys/linux/include/time.h b/newlib/libc/sys/linux/include/time.h index 5e61d2b65..917a35858 100644 --- a/newlib/libc/sys/linux/include/time.h +++ b/newlib/libc/sys/linux/include/time.h @@ -84,26 +84,6 @@ char *strptime (const char *, const char *, struct tm *); void tzset (void); void _tzset_r (struct _reent *); -typedef struct __tzrule_struct -{ - char ch; - int m; - int n; - int d; - int s; - time_t change; - long offset; /* Match type of _timezone. */ -} __tzrule_type; - -typedef struct __tzinfo_struct -{ - int __tznorth; - int __tzyear; - __tzrule_type __tzrule[2]; -} __tzinfo_type; - -__tzinfo_type *__gettzinfo (void); - /* getdate functions */ #ifndef _REENT_ONLY diff --git a/newlib/libc/sys/phoenix/include/time.h b/newlib/libc/sys/phoenix/include/time.h index 3a9449c77..41fb137e4 100644 --- a/newlib/libc/sys/phoenix/include/time.h +++ b/newlib/libc/sys/phoenix/include/time.h @@ -40,23 +40,6 @@ extern char *_tzname[2]; #define tzname _tzname #endif -typedef struct __tzrule_struct { - char ch; - int m; - int n; - int d; - int s; - time_t change; - long offset; -} __tzrule_type; - -typedef struct __tzinfo_struct { - int __tznorth; - int __tzyear; - __tzrule_type __tzrule[2]; -} __tzinfo_type; - -__tzinfo_type *__gettzinfo(); void tzset(); clock_t clock(); diff --git a/newlib/libc/time/local.h b/newlib/libc/time/local.h index dce51cda2..290e1aee5 100644 --- a/newlib/libc/time/local.h +++ b/newlib/libc/time/local.h @@ -38,3 +38,22 @@ void _tzset_unlocked (void); void __tz_lock (void); void __tz_unlock (void); +typedef struct __tzrule_struct +{ + char ch; + int m; /* Month of year if ch=M */ + int n; /* Week of month if ch=M */ + int d; /* Day of week if ch=M, day of year if ch=J or ch=D */ + int s; /* Time of day in seconds */ + time_t change; + long offset; /* Match type of _timezone. */ +} __tzrule_type; + +typedef struct __tzinfo_struct +{ + int __tznorth; + int __tzyear; + __tzrule_type __tzrule[2]; +} __tzinfo_type; + +__tzinfo_type *__gettzinfo (void); -- 2.18.0 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] libc/time: Move internal newlib tz-structs to local.h 2020-10-15 6:52 ` Torbjorn SVENSSON @ 2020-10-15 10:21 ` Corinna Vinschen 2020-10-15 16:47 ` Torbjorn SVENSSON 0 siblings, 1 reply; 21+ messages in thread From: Corinna Vinschen @ 2020-10-15 10:21 UTC (permalink / raw) To: Torbjorn SVENSSON; +Cc: newlib [-- Attachment #1: Type: text/plain, Size: 268 bytes --] On Oct 15 06:52, Torbjorn SVENSSON via Newlib wrote: > Ping! Due to vacation I only just checked now. The patch breaks the Cygwin build. Especially __tzinfo_type has to be exposed from a public header in newlib. So what about the attached patch instead? Corinna [-- Attachment #2: 0001-libc-time-Move-internal-newlib-tz-structs-into-own-h.patch --] [-- Type: text/plain, Size: 4901 bytes --] From c6ad49622e42b4b80ba5fbad40a0776ec74e9ec5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torbj=C3=B6rn=20SVENSSON=20via=20Newlib?= <newlib@sourceware.org> Date: Mon, 5 Oct 2020 14:50:13 +0200 Subject: [PATCH] libc/time: Move internal newlib tz-structs into own header MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As discussed in GCC bug 97088 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97088), parameters in prototypes of library functions should use reserved names, or no name at all. This patch moves the internal struct __tzrule_struct to its own internal header sys/_tz_structs.h. This is included from newlib's time code as well as from Cygwin's localtime wrapper. Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@st.com> Signed-off-by: Corinna Vinschen <corinna@vinschen.de> --- newlib/libc/include/sys/_tz_structs.h | 24 ++++++++++++++++++++++++ newlib/libc/include/time.h | 20 -------------------- newlib/libc/sys/linux/include/time.h | 20 -------------------- newlib/libc/sys/phoenix/include/time.h | 17 ----------------- newlib/libc/time/local.h | 1 + winsup/cygwin/tzcode/localtime_wrapper.c | 1 + 6 files changed, 26 insertions(+), 57 deletions(-) create mode 100644 newlib/libc/include/sys/_tz_structs.h diff --git a/newlib/libc/include/sys/_tz_structs.h b/newlib/libc/include/sys/_tz_structs.h new file mode 100644 index 000000000000..9610b06819e1 --- /dev/null +++ b/newlib/libc/include/sys/_tz_structs.h @@ -0,0 +1,24 @@ +#ifndef _SYS__TZ_STRUCTS_H_ +#define _SYS__TZ_STRUCTS_H_ + +typedef struct __tzrule_struct +{ + char ch; + int m; /* Month of year if ch=M */ + int n; /* Week of month if ch=M */ + int d; /* Day of week if ch=M, day of year if ch=J or ch=D */ + int s; /* Time of day in seconds */ + time_t change; + long offset; /* Match type of _timezone. */ +} __tzrule_type; + +typedef struct __tzinfo_struct +{ + int __tznorth; + int __tzyear; + __tzrule_type __tzrule[2]; +} __tzinfo_type; + +__tzinfo_type *__gettzinfo (void); + +#endif /* _SYS__TZ_STRUCTS_H_ */ diff --git a/newlib/libc/include/time.h b/newlib/libc/include/time.h index 3031590b441c..ed6cc70fec94 100644 --- a/newlib/libc/include/time.h +++ b/newlib/libc/include/time.h @@ -102,26 +102,6 @@ void tzset (void); #endif void _tzset_r (struct _reent *); -typedef struct __tzrule_struct -{ - char ch; - int m; - int n; - int d; - int s; - time_t change; - long offset; /* Match type of _timezone. */ -} __tzrule_type; - -typedef struct __tzinfo_struct -{ - int __tznorth; - int __tzyear; - __tzrule_type __tzrule[2]; -} __tzinfo_type; - -__tzinfo_type *__gettzinfo (void); - /* getdate functions */ #ifdef HAVE_GETDATE diff --git a/newlib/libc/sys/linux/include/time.h b/newlib/libc/sys/linux/include/time.h index 5e61d2b65057..917a35858285 100644 --- a/newlib/libc/sys/linux/include/time.h +++ b/newlib/libc/sys/linux/include/time.h @@ -84,26 +84,6 @@ char *strptime (const char *, const char *, struct tm *); void tzset (void); void _tzset_r (struct _reent *); -typedef struct __tzrule_struct -{ - char ch; - int m; - int n; - int d; - int s; - time_t change; - long offset; /* Match type of _timezone. */ -} __tzrule_type; - -typedef struct __tzinfo_struct -{ - int __tznorth; - int __tzyear; - __tzrule_type __tzrule[2]; -} __tzinfo_type; - -__tzinfo_type *__gettzinfo (void); - /* getdate functions */ #ifndef _REENT_ONLY diff --git a/newlib/libc/sys/phoenix/include/time.h b/newlib/libc/sys/phoenix/include/time.h index 3a9449c77540..41fb137e4391 100644 --- a/newlib/libc/sys/phoenix/include/time.h +++ b/newlib/libc/sys/phoenix/include/time.h @@ -40,23 +40,6 @@ extern char *_tzname[2]; #define tzname _tzname #endif -typedef struct __tzrule_struct { - char ch; - int m; - int n; - int d; - int s; - time_t change; - long offset; -} __tzrule_type; - -typedef struct __tzinfo_struct { - int __tznorth; - int __tzyear; - __tzrule_type __tzrule[2]; -} __tzinfo_type; - -__tzinfo_type *__gettzinfo(); void tzset(); clock_t clock(); diff --git a/newlib/libc/time/local.h b/newlib/libc/time/local.h index dce51cda29bf..bfe06e62230d 100644 --- a/newlib/libc/time/local.h +++ b/newlib/libc/time/local.h @@ -1,6 +1,7 @@ /* local header used by libc/time routines */ #include <_ansi.h> #include <time.h> +#include <sys/_tz_structs.h> #define SECSPERMIN 60L #define MINSPERHOUR 60L diff --git a/winsup/cygwin/tzcode/localtime_wrapper.c b/winsup/cygwin/tzcode/localtime_wrapper.c index 3ac8f8cb00ea..4e784480b0f4 100644 --- a/winsup/cygwin/tzcode/localtime_wrapper.c +++ b/winsup/cygwin/tzcode/localtime_wrapper.c @@ -11,6 +11,7 @@ details. */ #include "tz_posixrules.h" #include <cygwin/version.h> #include <stdlib.h> +#include <sys/_tz_structs.h> static NO_COPY SRWLOCK tzset_guard = SRWLOCK_INIT; -- 2.26.2 ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH v2] libc/time: Move internal newlib tz-structs to local.h 2020-10-15 10:21 ` Corinna Vinschen @ 2020-10-15 16:47 ` Torbjorn SVENSSON 2020-10-15 17:09 ` Jeff Johnston 2020-10-15 17:11 ` Corinna Vinschen 0 siblings, 2 replies; 21+ messages in thread From: Torbjorn SVENSSON @ 2020-10-15 16:47 UTC (permalink / raw) To: newlib Hello Corinna, Thanks for the feedback and I hope that you had a nice vacation! I haven't run tests with your patch applied, but just reading it should be fine. I see no reason why "sys/_tz_structs.h" would be included from "bits/stdc++.h", and it is this particular include chain that is causing the problems in the libstdc ++ test suite, so if it works for Cygwin, I'd say go for it! Kind regards, Torbjörn -----Original Message----- From: Corinna Vinschen <vinschen@redhat.com> Sent: den 15 oktober 2020 12:22 To: Torbjorn SVENSSON <torbjorn.svensson@st.com> Cc: newlib@sourceware.org Subject: Re: [PATCH v2] libc/time: Move internal newlib tz-structs to local.h On Oct 15 06:52, Torbjorn SVENSSON via Newlib wrote: > Ping! Due to vacation I only just checked now. The patch breaks the Cygwin build. Especially __tzinfo_type has to be exposed from a public header in newlib. So what about the attached patch instead? Corinna ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] libc/time: Move internal newlib tz-structs to local.h 2020-10-15 16:47 ` Torbjorn SVENSSON @ 2020-10-15 17:09 ` Jeff Johnston 2020-10-15 17:15 ` Corinna Vinschen 2020-10-15 17:11 ` Corinna Vinschen 1 sibling, 1 reply; 21+ messages in thread From: Jeff Johnston @ 2020-10-15 17:09 UTC (permalink / raw) To: Newlib I also like this solution, but again as I mentioned earlier, it technically requires a major release bump due to the removal of API from time.h and would be best to wait until we have something else warranting a major release bump. That said, what do you think Corinna? -- Jeff J. On Thu, Oct 15, 2020 at 12:47 PM Torbjorn SVENSSON via Newlib < newlib@sourceware.org> wrote: > Hello Corinna, > > Thanks for the feedback and I hope that you had a nice vacation! > > I haven't run tests with your patch applied, but just reading it should be > fine. > I see no reason why "sys/_tz_structs.h" would be included from > "bits/stdc++.h", and it is this particular include chain that is causing > the problems in the libstdc ++ test suite, so if it works for Cygwin, I'd > say go for it! > > Kind regards, > Torbjörn > > -----Original Message----- > From: Corinna Vinschen <vinschen@redhat.com> > Sent: den 15 oktober 2020 12:22 > To: Torbjorn SVENSSON <torbjorn.svensson@st.com> > Cc: newlib@sourceware.org > Subject: Re: [PATCH v2] libc/time: Move internal newlib tz-structs to > local.h > > On Oct 15 06:52, Torbjorn SVENSSON via Newlib wrote: > > Ping! > > Due to vacation I only just checked now. The patch breaks the Cygwin > build. Especially __tzinfo_type has to be exposed from a public header > in newlib. > > So what about the attached patch instead? > > > Corinna > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] libc/time: Move internal newlib tz-structs to local.h 2020-10-15 17:09 ` Jeff Johnston @ 2020-10-15 17:15 ` Corinna Vinschen 0 siblings, 0 replies; 21+ messages in thread From: Corinna Vinschen @ 2020-10-15 17:15 UTC (permalink / raw) To: Jeff Johnston; +Cc: newlib On Oct 15 13:09, Jeff Johnston via Newlib wrote: > I also like this solution, but again as I mentioned earlier, it technically > requires a major release bump due to the removal of API from time.h and > would be best > to wait until we have something else warranting a major release bump. > > That said, what do you think Corinna? Oops, time warp! Sorry, but I already pushed this. From my POV, these *are* internal structs and functions, so, technically, they are *not* official API and consequentially nobody has the right to expect them in an official header. For me the only problem is that the members are not underscored and may produce problems. Corinna > > -- Jeff J. > > On Thu, Oct 15, 2020 at 12:47 PM Torbjorn SVENSSON via Newlib < > newlib@sourceware.org> wrote: > > > Hello Corinna, > > > > Thanks for the feedback and I hope that you had a nice vacation! > > > > I haven't run tests with your patch applied, but just reading it should be > > fine. > > I see no reason why "sys/_tz_structs.h" would be included from > > "bits/stdc++.h", and it is this particular include chain that is causing > > the problems in the libstdc ++ test suite, so if it works for Cygwin, I'd > > say go for it! > > > > Kind regards, > > Torbjörn > > > > -----Original Message----- > > From: Corinna Vinschen <vinschen@redhat.com> > > Sent: den 15 oktober 2020 12:22 > > To: Torbjorn SVENSSON <torbjorn.svensson@st.com> > > Cc: newlib@sourceware.org > > Subject: Re: [PATCH v2] libc/time: Move internal newlib tz-structs to > > local.h > > > > On Oct 15 06:52, Torbjorn SVENSSON via Newlib wrote: > > > Ping! > > > > Due to vacation I only just checked now. The patch breaks the Cygwin > > build. Especially __tzinfo_type has to be exposed from a public header > > in newlib. > > > > So what about the attached patch instead? > > > > > > Corinna > > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] libc/time: Move internal newlib tz-structs to local.h 2020-10-15 16:47 ` Torbjorn SVENSSON 2020-10-15 17:09 ` Jeff Johnston @ 2020-10-15 17:11 ` Corinna Vinschen 1 sibling, 0 replies; 21+ messages in thread From: Corinna Vinschen @ 2020-10-15 17:11 UTC (permalink / raw) To: Torbjorn SVENSSON, Jeff Johnston; +Cc: newlib On Oct 15 16:47, Torbjorn SVENSSON via Newlib wrote: > Hello Corinna, > > Thanks for the feedback and I hope that you had a nice vacation! > > I haven't run tests with your patch applied, but just reading it should be fine. > I see no reason why "sys/_tz_structs.h" would be included from > "bits/stdc++.h", and it is this particular include chain that is > causing the problems in the libstdc ++ test suite, so if it works for > Cygwin, I'd say go for it! Thanks for reviewing. I'll push the patch in a minute. I still think we should rename the struct members as well, though. There's no good reason that we have a user of these structures outside newlib/Cygwin. But still, *iff* this file is included for whatever dubious purpose, it might result in problems. We have two ways to fix this: - Either guard the definitions additionally with a preprocessor expression like this: #if defined (__INSIDE_CYGWIN__) || defined (_COMPILING_NEWLIB) [...] #endif - or fix the names of the struct members and the newlib/Cygwin code using them. Thoughts? Jeff? Corinna ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2020-10-15 17:15 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-10-01 14:17 [PATCH 0/3] libc: Comply with the libstdc++ test case names.cc Torbjörn SVENSSON 2020-10-01 14:17 ` [PATCH 1/3] libc/include/inttypes.h: Remove parameter name Torbjörn SVENSSON 2020-10-01 23:22 ` Jeff Johnston 2020-10-02 6:52 ` Torbjorn SVENSSON 2020-10-02 6:53 ` nrupp 2020-10-02 21:01 ` Jeff Johnston 2020-10-01 14:17 ` [PATCH 2/3] libc/include/wchar.h: " Torbjörn SVENSSON 2020-10-01 23:24 ` Jeff Johnston 2020-10-02 6:53 ` Torbjorn SVENSSON 2020-10-02 21:02 ` Jeff Johnston 2020-10-01 14:17 ` [PATCH 3/3] libc: Replace one letter member names in __tzrule_struct Torbjörn SVENSSON 2020-10-01 23:21 ` Jeff Johnston 2020-10-02 7:36 ` Torbjorn SVENSSON 2020-10-02 21:04 ` Jeff Johnston 2020-10-05 12:50 ` [PATCH v2] libc/time: Move internal newlib tz-structs to local.h Torbjörn SVENSSON 2020-10-15 6:52 ` Torbjorn SVENSSON 2020-10-15 10:21 ` Corinna Vinschen 2020-10-15 16:47 ` Torbjorn SVENSSON 2020-10-15 17:09 ` Jeff Johnston 2020-10-15 17:15 ` Corinna Vinschen 2020-10-15 17:11 ` Corinna Vinschen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).