public inbox for newlib@sourceware.org
 help / color / mirror / Atom feed
* [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

* [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

* [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 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 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 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 &lt;newlib@sourceware.org&gt; </div><div>Datum: 02.10.20  08:52  (GMT+01:00) </div><div>An: Jeff Johnston &lt;jjohnstn@redhat.com&gt; </div><div>Cc: Newlib &lt;newlib@sourceware.org&gt; </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 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 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 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

* 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

* 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 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

* 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

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).