[PATCH 0/3] Additional changes after introducing Minguo calendar and zero-padding in %Ey

[PATCH 0/3] Additional changes after introducing Minguo calendar and zero-padding in %Ey

[Rafal Luzynski]

Questions:

1. Do we want to mention the Minguo calendar in NEWS?
2. Do we want to add tests for the newly added Minguo calendar?

CC: TAMUKI Shoichi as the original author of the test case and
the zero padding in strftime("%Ey"), and Felix Yan as the original
author of the Minguo calendar support in *_TW.

Regards,

Rafal

[PATCH 1/3] NEWS: Mention Minguo calendar support added [BZ #24293]

[Rafal Luzynski]

---
 NEWS | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/NEWS b/NEWS
index f12524d..a0a9ed5 100644
--- a/NEWS
+++ b/NEWS
@@ -18,6 +18,10 @@ Major new features:
 
 * On Linux, the gettid function has been added.
 
+* Minguo (traditional Chinese) calendar support has been added as an
+  alternative calendar for the following locales: cmn_TW, hak_TW, lzh_TW,
+  nan_TW, zh_TW.
+
 Deprecated and removed features, and other changes affecting compatibility:
 
 * The functions clock_gettime, clock_getres, clock_settime,
-- 
2.7.5

Re: [PATCH 1/3] NEWS: Mention Minguo calendar support added [BZ #24293]

[TAMUKI Shoichi]

Hello Rafal-san,

From: Rafal Luzynski <digitalfreak@lingonborough.com>
Subject: [PATCH 1/3] NEWS: Mention Minguo calendar support added [BZ #24293]
Date: Fri, 15 Mar 2019 12:47:48 +0100 (CET)

> diff --git a/NEWS b/NEWS
> index f12524d..a0a9ed5 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -18,6 +18,10 @@ Major new features:
>  
>  * On Linux, the gettid function has been added.
>  
> +* Minguo (traditional Chinese) calendar support has been added as an
> +  alternative calendar for the following locales: cmn_TW, hak_TW, lzh_TW,
> +  nan_TW, zh_TW.
> +
>  Deprecated and removed features, and other changes affecting compatibility:
>  
>  * The functions clock_gettime, clock_getres, clock_settime,

Looks good to me.

Thank you for your contribution.

Regrads,
TAMUKI Shoichi

Re: [PATCH 1/3] NEWS: Mention Minguo calendar support added [BZ #24293]

[Rafal Luzynski]

17.03.2019 11:28 TAMUKI Shoichi <tamuki@linet.gr.jp> wrote:
> 
> Hello Rafal-san,
> 
> From: Rafal Luzynski <digitalfreak@lingonborough.com>
> Subject: [PATCH 1/3] NEWS: Mention Minguo calendar support added [BZ
> #24293]
> Date: Fri, 15 Mar 2019 12:47:48 +0100 (CET)
> 
> > diff --git a/NEWS b/NEWS
> > index f12524d..a0a9ed5 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -18,6 +18,10 @@ Major new features:
> >  
> >  * On Linux, the gettid function has been added.
> >  
> > +* Minguo (traditional Chinese) calendar support has been added as an
> > +  alternative calendar for the following locales: cmn_TW, hak_TW,
> > lzh_TW,
> > +  nan_TW, zh_TW.
> > +
> >  Deprecated and removed features, and other changes affecting
> > compatibility:
> >  
> >  * The functions clock_gettime, clock_getres, clock_settime,
> 
> Looks good to me.

Thank you.  I would like to hear some feedback from more experienced
maintainers, especially those who are English native speakers and are
able to provide proofreading.  CC'ing people who were helpful in the
past.

Regards,

Rafal

[PATCH 2/3] time/tst-strftime2.c: Make the file easier to maintain.

[Rafal Luzynski]

Express the years as full Gregorian years (e.g., 1988 instead of 88)
and months with natural numbers (1-12 rather than 0-11).

Compare actual dates rather than indexes when selecting the era name.

Declare the local variable era as a string character pointer rather than
an array of chars where the actual string is copied which might lead to
potential buffer overflows in future.

	* time/tst-strftime2.c (date_t): Explicitly define the type.
	(dates): Use natural month and year numbers to express a date.
	(is_before): New function to compare dates.
	(mkreftable): Minor improvements to simplify maintenance.
	(do_test): Reflect the changes in dates array.
---
 time/tst-strftime2.c | 62
+++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 44 insertions(+), 18 deletions(-)

diff --git a/time/tst-strftime2.c b/time/tst-strftime2.c
index 3dca2a9..bf5a66d 100644
--- a/time/tst-strftime2.c
+++ b/time/tst-strftime2.c
@@ -19,8 +19,10 @@
    <http://www.gnu.org/licenses/>.  */
 
 #include <array_length.h>
+#include <assert.h>
 #include <locale.h>
 #include <time.h>
+#include <stdbool.h>
 #include <stdio.h>
 #include <string.h>
 
@@ -28,27 +30,44 @@ static const char *locales[] = { "ja_JP.UTF-8",
"lo_LA.UTF-8", "th_TH.UTF-8" };
 
 static const char *formats[] = { "%EY", "%_EY", "%-EY" };
 
-static const struct
+typedef struct
 {
   const int d, m, y;
-} dates[] =
+} date_t;
+
+static const date_t dates[] =
   {
-    { 1, 3, 88 },
-    { 7, 0, 89 },
-    { 8, 0, 89 },
-    { 1, 3, 90 },
-    { 1, 3, 97 },
-    { 1, 3, 98 }
+    { 1, 4, 1988 },
+    { 7, 1, 1989 },
+    { 8, 1, 1989 },
+    { 1, 4, 1990 },
+    { 1, 4, 1997 },
+    { 1, 4, 1998 }
   };
 
 static char ref[array_length (locales)][array_length (formats)]
 	       [array_length (dates)][100];
 
+static bool
+is_before (const date_t *date, const int d, const int m, const int y)
+{
+  if (date->y < y)
+    return true;
+  else if (date->y > y)
+    return false;
+  else if (date->m < m)
+    return true;
+  else if (date->m > m)
+    return false;
+  else
+    return date->d < d;
+}
+
 static void
 mkreftable (void)
 {
   int i, j, k;
-  char era[10];
+  const char *era;
   static const int yrj[] = { 63, 64, 1, 2, 9, 10 };
   static const int yrb[] = { 2531, 2532, 2532, 2533, 2540, 2541 };
 
@@ -56,10 +75,13 @@ mkreftable (void)
     for (j = 0; j < array_length (formats); j++)
       for (k = 0; k < array_length (dates); k++)
 	{
-	  if (i == 0)
+	  if (i == 0)  /* ja_JP  */
 	    {
-	      sprintf (era, "%s", (k < 2) ? "\xe6\x98\xad\xe5\x92\x8c"
-					  : "\xe5\xb9\xb3\xe6\x88\x90");
+	      if (is_before (&dates[k], 8, 1, 1989))
+		era = "\xe6\x98\xad\xe5\x92\x8c";
+	      else
+		era = "\xe5\xb9\xb3\xe6\x88\x90";
+
 	      if (yrj[k] == 1)
 		sprintf (ref[i][j][k], "%s\xe5\x85\x83\xe5\xb9\xb4", era);
 	      else
@@ -72,16 +94,20 @@ mkreftable (void)
 		    sprintf (ref[i][j][k], "%s%d\xe5\xb9\xb4", era, yrj[k]);
 		}
 	    }
-	  else if (i == 1)
+	  else if (i == 1)  /* lo_LA  */
 	    {
-	      sprintf (era, "\xe0\xba\x9e\x2e\xe0\xba\xaa\x2e ");
+	      era = "\xe0\xba\x9e\x2e\xe0\xba\xaa\x2e ";
 	      sprintf (ref[i][j][k], "%s%d", era, yrb[k]);
 	    }
-	  else
+	  else if (i == 2)  /* th_TH  */
 	    {
-	      sprintf (era, "\xe0\xb8\x9e\x2e\xe0\xb8\xa8\x2e ");
+	      era = "\xe0\xb8\x9e\x2e\xe0\xb8\xa8\x2e ";
 	      sprintf (ref[i][j][k], "%s%d", era, yrb[k]);
 	    }
+	  else
+	    {
+	      assert (0);  /* Unreachable.  */
+	    }
 	}
 }
 
@@ -107,8 +133,8 @@ do_test (void)
 	  for (k = 0; k < array_length (dates); k++)
 	    {
 	      ttm.tm_mday = dates[k].d;
-	      ttm.tm_mon  = dates[k].m;
-	      ttm.tm_year = dates[k].y;
+	      ttm.tm_mon  = dates[k].m - 1;
+	      ttm.tm_year = dates[k].y - 1900;
 	      strftime (date, sizeof (date), "%F", &ttm);
 	      r = strftime (buf, sizeof (buf), formats[j], &ttm);
 	      e = strlen (ref[i][j][k]);
-- 
2.7.5

Re: [PATCH 2/3] time/tst-strftime2.c: Make the file easier to maintain.

[TAMUKI Shoichi]

Hello Rafal-san,

From: Rafal Luzynski <digitalfreak@lingonborough.com>
Subject: [PATCH 2/3] time/tst-strftime2.c: Make the file easier to maintain.
Date: Fri, 15 Mar 2019 12:48:34 +0100 (CET)

> Express the years as full Gregorian years (e.g., 1988 instead of 88)
> and months with natural numbers (1-12 rather than 0-11).
> 
> Compare actual dates rather than indexes when selecting the era name.
> 
> Declare the local variable era as a string character pointer rather than
> an array of chars where the actual string is copied which might lead to
> potential buffer overflows in future.

I think these improvements are good.

> 	* time/tst-strftime2.c (date_t): Explicitly define the type.
> 	(dates): Use natural month and year numbers to express a date.
> 	(is_before): New function to compare dates.
> 	(mkreftable): Minor improvements to simplify maintenance.
> 	(do_test): Reflect the changes in dates array.

Looks good to me.

> diff --git a/time/tst-strftime2.c b/time/tst-strftime2.c
> index 3dca2a9..bf5a66d 100644
> --- a/time/tst-strftime2.c
> +++ b/time/tst-strftime2.c
> @@ -19,8 +19,10 @@
>     <http://www.gnu.org/licenses/>.  */
>  
>  #include <array_length.h>
> +#include <assert.h>
>  #include <locale.h>
>  #include <time.h>
> +#include <stdbool.h>
>  #include <stdio.h>
>  #include <string.h>
>  

I care about the order of including header lines.  Because including
stdbool.h and assert.h are used during the reference table creation,
the following order is preferred.

Recommend instead:

| @@ -19,6 +19,8 @@
|     <http://www.gnu.org/licenses/>.  */
|  
|  #include <array_length.h>
| +#include <stdbool.h>
| +#include <assert.h>
|  #include <locale.h>
|  #include <time.h>
|  #include <stdio.h>

> @@ -28,27 +30,44 @@ static const char *locales[] = { "ja_JP.UTF-8", "lo_LA.UTF-8", "th_TH.UTF-8" };
>  
>  static const char *formats[] = { "%EY", "%_EY", "%-EY" };
>  
> -static const struct
> +typedef struct
>  {
>    const int d, m, y;
> -} dates[] =
> +} date_t;
> +
> +static const date_t dates[] =
>    {
> -    { 1, 3, 88 },
> -    { 7, 0, 89 },
> -    { 8, 0, 89 },
> -    { 1, 3, 90 },
> -    { 1, 3, 97 },
> -    { 1, 3, 98 }
> +    { 1, 4, 1988 },
> +    { 7, 1, 1989 },
> +    { 8, 1, 1989 },
> +    { 1, 4, 1990 },
> +    { 1, 4, 1997 },
> +    { 1, 4, 1998 }
>    };

OK.

> +static bool
> +is_before (const date_t *date, const int d, const int m, const int y)
> +{
> +  if (date->y < y)
> +    return true;
> +  else if (date->y > y)
> +    return false;
> +  else if (date->m < m)
> +    return true;
> +  else if (date->m > m)
> +    return false;
> +  else
> +    return date->d < d;
> +}
> +

OK.  Nice idea.

>  static void
>  mkreftable (void)
>  {
>    int i, j, k;
> -  char era[10];
> +  const char *era;
>    static const int yrj[] = { 63, 64, 1, 2, 9, 10 };
>    static const int yrb[] = { 2531, 2532, 2532, 2533, 2540, 2541 };
>  

OK.

> @@ -56,10 +75,13 @@ mkreftable (void)
>      for (j = 0; j < array_length (formats); j++)
>        for (k = 0; k < array_length (dates); k++)
>  	{
> -	  if (i == 0)
> +	  if (i == 0)  /* ja_JP  */
>  	    {
> -	      sprintf (era, "%s", (k < 2) ? "\xe6\x98\xad\xe5\x92\x8c"
> -					  : "\xe5\xb9\xb3\xe6\x88\x90");
> +	      if (is_before (&dates[k], 8, 1, 1989))
> +		era = "\xe6\x98\xad\xe5\x92\x8c";
> +	      else
> +		era = "\xe5\xb9\xb3\xe6\x88\x90";
> +

Please delete the above blank one line.

> @@ -72,16 +94,20 @@ mkreftable (void)
>  		    sprintf (ref[i][j][k], "%s%d\xe5\xb9\xb4", era, yrj[k]);
>  		}
>  	    }
> -	  else if (i == 1)
> +	  else if (i == 1)  /* lo_LA  */
>  	    {
> -	      sprintf (era, "\xe0\xba\x9e\x2e\xe0\xba\xaa\x2e ");
> +	      era = "\xe0\xba\x9e\x2e\xe0\xba\xaa\x2e ";
>  	      sprintf (ref[i][j][k], "%s%d", era, yrb[k]);
>  	    }
> -	  else
> +	  else if (i == 2)  /* th_TH  */
>  	    {
> -	      sprintf (era, "\xe0\xb8\x9e\x2e\xe0\xb8\xa8\x2e ");
> +	      era = "\xe0\xb8\x9e\x2e\xe0\xb8\xa8\x2e ";
>  	      sprintf (ref[i][j][k], "%s%d", era, yrb[k]);
>  	    }
> +	  else
> +	    {
> +	      assert (0);  /* Unreachable.  */
> +	    }
>  	}
>  }
>  

Braces surrounding assert are redundant.

Recommend instead:

| @@ -72,16 +93,18 @@ mkreftable (void)
|  		    sprintf (ref[i][j][k], "%s%d\xe5\xb9\xb4", era, yrj[k]);
|  		}
|  	    }
| -	  else if (i == 1)
| +	  else if (i == 1)  /* lo_LA  */
|  	    {
| -	      sprintf (era, "\xe0\xba\x9e\x2e\xe0\xba\xaa\x2e ");
| +	      era = "\xe0\xba\x9e\x2e\xe0\xba\xaa\x2e ";
|  	      sprintf (ref[i][j][k], "%s%d", era, yrb[k]);
|  	    }
| -	  else
| +	  else if (i == 2)  /* th_TH  */
|  	    {
| -	      sprintf (era, "\xe0\xb8\x9e\x2e\xe0\xb8\xa8\x2e ");
| +	      era = "\xe0\xb8\x9e\x2e\xe0\xb8\xa8\x2e ";
|  	      sprintf (ref[i][j][k], "%s%d", era, yrb[k]);
|  	    }
| +	  else
| +	    assert (0);  /* Unreachable.  */
|  	}
|  }
|  

> @@ -107,8 +133,8 @@ do_test (void)
>  	  for (k = 0; k < array_length (dates); k++)
>  	    {
>  	      ttm.tm_mday = dates[k].d;
> -	      ttm.tm_mon  = dates[k].m;
> -	      ttm.tm_year = dates[k].y;
> +	      ttm.tm_mon  = dates[k].m - 1;
> +	      ttm.tm_year = dates[k].y - 1900;
>  	      strftime (date, sizeof (date), "%F", &ttm);
>  	      r = strftime (buf, sizeof (buf), formats[j], &ttm);
>  	      e = strlen (ref[i][j][k]);

OK.

Thank you for improving the test case.

Regards,
TAMUKI Shoichi

Re: [PATCH 2/3] time/tst-strftime2.c: Make the file easier to maintain.

[Rafal Luzynski]

17.03.2019 11:32 TAMUKI Shoichi <tamuki@linet.gr.jp> wrote:
> 
> Hello Rafal-san,
> 
> From: Rafal Luzynski <digitalfreak@lingonborough.com>
> Subject: [PATCH 2/3] time/tst-strftime2.c: Make the file easier to
> maintain.
> Date: Fri, 15 Mar 2019 12:48:34 +0100 (CET)
> 
> [...]
> > diff --git a/time/tst-strftime2.c b/time/tst-strftime2.c
> > index 3dca2a9..bf5a66d 100644
> > --- a/time/tst-strftime2.c
> > +++ b/time/tst-strftime2.c
> > @@ -19,8 +19,10 @@
> >     <http://www.gnu.org/licenses/>.  */
> >  
> >  #include <array_length.h>
> > +#include <assert.h>
> >  #include <locale.h>
> >  #include <time.h>
> > +#include <stdbool.h>
> >  #include <stdio.h>
> >  #include <string.h>
> >  
> 
> I care about the order of including header lines.  Because including
> stdbool.h and assert.h are used during the reference table creation,
> the following order is preferred.
> 
> Recommend instead:
> 
> | @@ -19,6 +19,8 @@
> |     <http://www.gnu.org/licenses/>.  */
> |  
> |  #include <array_length.h>
> | +#include <stdbool.h>
> | +#include <assert.h>
> |  #include <locale.h>
> |  #include <time.h>
> |  #include <stdio.h>

I don't mind changing this as you suggest but I thought that includes
should be kept alphabetically.  I don't know what is the preferred
convention
in glibc.  Will anybody help here?

> [...]
> > @@ -56,10 +75,13 @@ mkreftable (void)
> >      for (j = 0; j < array_length (formats); j++)
> >        for (k = 0; k < array_length (dates); k++)
> >  	{
> > -	  if (i == 0)
> > +	  if (i == 0)  /* ja_JP  */
> >  	    {
> > -	      sprintf (era, "%s", (k < 2) ? "\xe6\x98\xad\xe5\x92\x8c"
> > -					  : "\xe5\xb9\xb3\xe6\x88\x90");
> > +	      if (is_before (&dates[k], 8, 1, 1989))
> > +		era = "\xe6\x98\xad\xe5\x92\x8c";
> > +	      else
> > +		era = "\xe5\xb9\xb3\xe6\x88\x90";
> > +
> 
> Please delete the above blank one line.

OK

> > @@ -72,16 +94,20 @@ mkreftable (void)
> >  		    sprintf (ref[i][j][k], "%s%d\xe5\xb9\xb4", era, yrj[k]);
> >  		}
> >  	    }
> > -	  else if (i == 1)
> > +	  else if (i == 1)  /* lo_LA  */
> >  	    {
> > -	      sprintf (era, "\xe0\xba\x9e\x2e\xe0\xba\xaa\x2e ");
> > +	      era = "\xe0\xba\x9e\x2e\xe0\xba\xaa\x2e ";
> >  	      sprintf (ref[i][j][k], "%s%d", era, yrb[k]);
> >  	    }
> > -	  else
> > +	  else if (i == 2)  /* th_TH  */
> >  	    {
> > -	      sprintf (era, "\xe0\xb8\x9e\x2e\xe0\xb8\xa8\x2e ");
> > +	      era = "\xe0\xb8\x9e\x2e\xe0\xb8\xa8\x2e ";
> >  	      sprintf (ref[i][j][k], "%s%d", era, yrb[k]);
> >  	    }
> > +	  else
> > +	    {
> > +	      assert (0);  /* Unreachable.  */
> > +	    }
> >  	}
> >  }
> >  
> 
> Braces surrounding assert are redundant.
> 
> Recommend instead:
> 
> | @@ -72,16 +93,18 @@ mkreftable (void)
> |  		    sprintf (ref[i][j][k], "%s%d\xe5\xb9\xb4", era, yrj[k]);
> |  		}
> |  	    }
> | -	  else if (i == 1)
> | +	  else if (i == 1)  /* lo_LA  */
> |  	    {
> | -	      sprintf (era, "\xe0\xba\x9e\x2e\xe0\xba\xaa\x2e ");
> | +	      era = "\xe0\xba\x9e\x2e\xe0\xba\xaa\x2e ";
> |  	      sprintf (ref[i][j][k], "%s%d", era, yrb[k]);
> |  	    }
> | -	  else
> | +	  else if (i == 2)  /* th_TH  */
> |  	    {
> | -	      sprintf (era, "\xe0\xb8\x9e\x2e\xe0\xb8\xa8\x2e ");
> | +	      era = "\xe0\xb8\x9e\x2e\xe0\xb8\xa8\x2e ";
> |  	      sprintf (ref[i][j][k], "%s%d", era, yrb[k]);
> |  	    }
> | +	  else
> | +	    assert (0);  /* Unreachable.  */
> |  	}
> |  }
> |  

Again I don't mind this but there are different conventions and I am
not sure what is preferred in glibc.

To be honest, I prefer to remove these braces.

Thank you for your feedback, and regards,

Rafal

Re: [PATCH 2/3] time/tst-strftime2.c: Make the file easier to maintain.

[TAMUKI Shoichi]

Hello Rafal-san,

From: Rafal Luzynski <digitalfreak@lingonborough.com>
Subject: Re: [PATCH 2/3] time/tst-strftime2.c: Make the file easier to maintain.
Date: Tue, 19 Mar 2019 00:30:57 +0100 (CET)

> > I care about the order of including header lines.  Because including
> > stdbool.h and assert.h are used during the reference table creation,
> > the following order is preferred.
> > 
> > Recommend instead:
> > 
> > | @@ -19,6 +19,8 @@
> > |     <http://www.gnu.org/licenses/>.  */
> > |  
> > |  #include <array_length.h>
> > | +#include <stdbool.h>
> > | +#include <assert.h>
> > |  #include <locale.h>
> > |  #include <time.h>
> > |  #include <stdio.h>
> 
> I don't mind changing this as you suggest but I thought that includes
> should be kept alphabetically.  I don't know what is the preferred convention
> in glibc.  Will anybody help here?

At first glance, the original includes appear to be in alphabetical
order, but it just happens, and actually the position of time.h is
different.  To improve the readability of source code, they are listed
in loose rules in the order of the procedure.

With relatively large source code, and in sufficiently complex cases,
it is more convenient to be kept in alphabetical order, indeed.

> > Braces surrounding assert are redundant.
> > 
> > Recommend instead:
> > 
> > | @@ -72,16 +93,18 @@ mkreftable (void)
> > |  		    sprintf (ref[i][j][k], "%s%d\xe5\xb9\xb4", era, yrj[k]);
> > |  		}
> > |  	    }
> > | -	  else if (i == 1)
> > | +	  else if (i == 1)  /* lo_LA  */
> > |  	    {
> > | -	      sprintf (era, "\xe0\xba\x9e\x2e\xe0\xba\xaa\x2e ");
> > | +	      era = "\xe0\xba\x9e\x2e\xe0\xba\xaa\x2e ";
> > |  	      sprintf (ref[i][j][k], "%s%d", era, yrb[k]);
> > |  	    }
> > | -	  else
> > | +	  else if (i == 2)  /* th_TH  */
> > |  	    {
> > | -	      sprintf (era, "\xe0\xb8\x9e\x2e\xe0\xb8\xa8\x2e ");
> > | +	      era = "\xe0\xb8\x9e\x2e\xe0\xb8\xa8\x2e ";
> > |  	      sprintf (ref[i][j][k], "%s%d", era, yrb[k]);
> > |  	    }
> > | +	  else
> > | +	    assert (0);  /* Unreachable.  */
> > |  	}
> > |  }
> > |  
> 
> Again I don't mind this but there are different conventions and I am
> not sure what is preferred in glibc.
> 
> To be honest, I prefer to remove these braces.

As a convention in glibc, I think that braces are not necessary for
one-line statements.

Regards,
TAMUKI Shoichi

Re: [PATCH 2/3] time/tst-strftime2.c: Make the file easier to maintain.

[TAMUKI Shoichi]

Hello Rafal-san,

I am sorry I can not tell you at once.

From: Rafal Luzynski <digitalfreak@lingonborough.com>
Subject: [PATCH 2/3] time/tst-strftime2.c: Make the file easier to maintain.
Date: Fri, 15 Mar 2019 12:48:34 +0100 (CET)

> [...]
> +static bool
> +is_before (const date_t *date, const int d, const int m, const int y)
> +{
> +  if (date->y < y)
> +    return true;
> +  else if (date->y > y)
> +    return false;
> +  else if (date->m < m)
> +    return true;
> +  else if (date->m > m)
> +    return false;
> +  else
> +    return date->d < d;
> +}
> +

Since dates[] is a global variable, I think that it will be
simplified by using the index of dates[] instead of the address of
dates[] as an argument for the is_before function.

So maybe something like this:

@@ -62,18 +62,18 @@
 	       [array_length (dates)][100];
 
 static bool
-is_before (const date_t *date, const int d, const int m, const int y)
+is_before (const int i, const int d, const int m, const int y)
 {
-  if (date->y < y)
+  if (dates[i].y < y)
     return true;
-  else if (date->y > y)
+  else if (dates[i].y > y)
     return false;
-  else if (date->m < m)
+  else if (dates[i].m < m)
     return true;
-  else if (date->m > m)
+  else if (dates[i].m > m)
     return false;
   else
-    return date->d < d;
+    return dates[i].d < d;
 }
 
 static void
@@ -103,11 +103,11 @@
 	{
 	  if (i == 0)  /* ja_JP  */
 	    {
-	      if (is_before (&dates[k], 30, 7, 1912))
+	      if (is_before (k, 30, 7, 1912))
 		era = "\xe6\x98\x8e\xe6\xb2\xbb";
-	      else if (is_before (&dates[k], 25, 12, 1926))
+	      else if (is_before (k, 25, 12, 1926))
 		era = "\xe5\xa4\xa7\xe6\xad\xa3";
-	      else if (is_before (&dates[k], 8, 1, 1989))
+	      else if (is_before (k, 8, 1, 1989))
 		era = "\xe6\x98\xad\xe5\x92\x8c";
 	      else
 		era = "\xe5\xb9\xb3\xe6\x88\x90";
@@ -125,7 +125,7 @@
 	    }
 	  else if (i >= 3 && i <= 7)  /* {zh,cmn,hak,nan,lzh}_TW  */
 	    {
-	      if (is_before (&dates[k], 1, 1, 1912))
+	      if (is_before (k, 1, 1, 1912))
 		era = "\xe6\xb0\x91\xe5\x89\x8d";
 	      else
 		era = "\xe6\xb0\x91\xe5\x9c\x8b";

From: TAMUKI Shoichi <tamuki@linet.gr.jp>
Subject: Re: [PATCH 3/3] time: Add tests for Minguo calendar [BZ #24293]
Date: Sun, 24 Mar 2019 20:40:07 +0900

> Question:
> 
> In the typedef struct, which indentation style is appropriate?
> 
> | typedef struct
> | {
> |   const int d, m, y;
> | } date_t;
> | 
> 
> or
> 
> | typedef struct
> |   {
> |     const int d, m, y;
> |   } date_t;
> | 
> 
> ?

According to GNU Coding Standards documentation:

| For struct and enum types, likewise put the braces in column one,
| unless the whole contents fits on one line:
| 
| struct foo
| {
|   int a, b;
| }
| 
| or
| 
| struct foo { int a, b; }

So perhaps maybe something like these:

| static const char *locales[] =
| {
|   "ja_JP.UTF-8", "lo_LA.UTF-8", "th_TH.UTF-8",
|   "zh_TW.UTF-8", "cmn_TW.UTF-8", "hak_TW.UTF-8",
|   "nan_TW.UTF-8", "lzh_TW.UTF-8"
| };

| static const date_t dates[] =
| {
|   {  1,  4, 1910 },
|   { 31, 12, 1911 },
|   { 29,  7, 1912 },
|   { 30,  7, 1912 },
|   {  1,  4, 1913 },
|   {  1,  4, 1988 },
|   {  7,  1, 1989 },
|   {  8,  1, 1989 },
|   {  1,  4, 1990 },
|   {  1,  4, 1997 },
|   {  1,  4, 1998 },
|   {  1,  4, 2010 },
|   {  1,  4, 2011 }
| };

| static void
| mkreftable (void)
| {
|   [...]
|   static const int yrj[] =
|   {
|     43, 44, 45, 1, 2,
|     63, 64, 1, 2, 9, 10, 22, 23
|   };
|   static const int yrb[] =
|   {
|     2453, 2454, 2455, 2455, 2456,
|     2531, 2532, 2532, 2533, 2540, 2541, 2553, 2554
|   };
|   static const int yrc[] =
|   {
|     -2, -1, 1, 1, 2,
|     77, 78, 78, 79, 86, 87, 99, 100
|   };
| 
|   [...]

Regards,
TAMUKI Shoichi

[PATCH 3/3] time: Add tests for Minguo calendar [BZ #24293]

[Rafal Luzynski]

	[BZ #24293]
	* time/Makefile (LOCALES): Add cmn_TW.UTF-8 and zh_TW.UTF-8.
	* time/tst-strftime2.c (locales): Likewise.
	(dates): Add 1910-04-01, 1911-12-31, 1912-07-29, 1912-07-30,
	and 1913-04-01.
	(mkreftable): Add rules for the new locales and the new dates.
---
 time/Makefile        |  2 +-
 time/tst-strftime2.c | 50
++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 47 insertions(+), 5 deletions(-)

diff --git a/time/Makefile b/time/Makefile
index 5c6304e..9f70800 100644
--- a/time/Makefile
+++ b/time/Makefile
@@ -50,7 +50,7 @@ include ../Rules
 ifeq ($(run-built-tests),yes)
 LOCALES := de_DE.ISO-8859-1 en_US.ISO-8859-1 ja_JP.EUC-JP fr_FR.UTF-8 \
 	   es_ES.UTF-8 pl_PL.UTF-8 ru_RU.UTF-8 \
-	   ja_JP.UTF-8 lo_LA.UTF-8 th_TH.UTF-8
+	   ja_JP.UTF-8 lo_LA.UTF-8 th_TH.UTF-8 cmn_TW.UTF-8 zh_TW.UTF-8
 include ../gen-locales.mk
 
 $(objpfx)tst-ftime_l.out: $(gen-locales)
diff --git a/time/tst-strftime2.c b/time/tst-strftime2.c
index bf5a66d..fb7f6cd 100644
--- a/time/tst-strftime2.c
+++ b/time/tst-strftime2.c
@@ -26,7 +26,8 @@
 #include <stdio.h>
 #include <string.h>
 
-static const char *locales[] = { "ja_JP.UTF-8", "lo_LA.UTF-8",
"th_TH.UTF-8" };
+static const char *locales[] = { "ja_JP.UTF-8", "lo_LA.UTF-8",
"th_TH.UTF-8",
+				 "cmn_TW.UTF-8", "zh_TW.UTF-8" };
 
 static const char *formats[] = { "%EY", "%_EY", "%-EY" };
 
@@ -37,6 +38,11 @@ typedef struct
 
 static const date_t dates[] =
   {
+    {  1,  4, 1910 },
+    { 31, 12, 1911 },
+    { 29,  7, 1912 },
+    { 30,  7, 1912 },
+    { 1, 4, 1913 },
     { 1, 4, 1988 },
     { 7, 1, 1989 },
     { 8, 1, 1989 },
@@ -68,8 +74,10 @@ mkreftable (void)
 {
   int i, j, k;
   const char *era;
-  static const int yrj[] = { 63, 64, 1, 2, 9, 10 };
-  static const int yrb[] = { 2531, 2532, 2532, 2533, 2540, 2541 };
+  static const int yrj[] = { 43, 44, 45, 1, 2, 63, 64, 1, 2, 9, 10 };
+  static const int yrb[] = { 2453, 2454, 2455, 2455, 2456,
+			     2531, 2532, 2532, 2533, 2540, 2541 };
+  static const int yrc[] = { 2, 1, 1, 1, 2, 77, 78, 78, 79, 86, 87 };
 
   for (i = 0; i < array_length (locales); i++)
     for (j = 0; j < array_length (formats); j++)
@@ -77,7 +85,11 @@ mkreftable (void)
 	{
 	  if (i == 0)  /* ja_JP  */
 	    {
-	      if (is_before (&dates[k], 8, 1, 1989))
+	      if (is_before (&dates[k], 30, 7, 1912))
+		era = "\xe6\x98\x8e\xe6\xb2\xbb";
+	      else if (is_before (&dates[k], 25, 12, 1926))
+		era = "\xe5\xa4\xa7\xe6\xad\xa3";
+	      else if (is_before (&dates[k], 8, 1, 1989))
 		era = "\xe6\x98\xad\xe5\x92\x8c";
 	      else
 		era = "\xe5\xb9\xb3\xe6\x88\x90";
@@ -104,6 +116,36 @@ mkreftable (void)
 	      era = "\xe0\xb8\x9e\x2e\xe0\xb8\xa8\x2e ";
 	      sprintf (ref[i][j][k], "%s%d", era, yrb[k]);
 	    }
+	  else if (i == 3)  /* cmn_TW  */
+	    {
+	      if (is_before (&dates[k], 1, 1, 1912))
+		era = "\xe6\xb0\x91\xe5\x89\x8d";
+	      else
+		era = "\xe6\xb0\x91\xe5\x9c\x8b";
+	      if (dates[k].y == 1912)
+		sprintf (ref[i][j][k], "%s\xe5\x85\x83\xe5\xb9\xb4", era);
+	      else if (j == 0)
+		sprintf (ref[i][j][k], "%s%02d\xe5\xb9\xb4", era, yrc[k]);
+	      else if (j == 1)
+		sprintf (ref[i][j][k], "%s%2d\xe5\xb9\xb4", era, yrc[k]);
+	      else
+		sprintf (ref[i][j][k], "%s%d\xe5\xb9\xb4", era, yrc[k]);
+	    }
+	  else if (i == 4)  /* zh_TW  */
+	    {
+	      if (is_before (&dates[k], 1, 1, 1912))
+		era = "\xe6\xb0\x91\xe5\x89\x8d";
+	      else
+		era = "\xe6\xb0\x91\xe5\x9c\x8b";
+	      if (dates[k].y == 1912)
+		sprintf (ref[i][j][k], "%s\xe5\x85\x83\xe5\xb9\xb4", era);
+	      else if (j == 0)
+		sprintf (ref[i][j][k], "%s%02d\xe5\xb9\xb4", era, yrc[k]);
+	      else if (j == 1)
+		sprintf (ref[i][j][k], "%s%2d\xe5\xb9\xb4", era, yrc[k]);
+	      else
+		sprintf (ref[i][j][k], "%s%d\xe5\xb9\xb4", era, yrc[k]);
+	    }
 	  else
 	    {
 	      assert (0);  /* Unreachable.  */
-- 
2.7.5

Re: [PATCH 3/3] time: Add tests for Minguo calendar [BZ #24293]

[Rafal Luzynski]

Test results:

[ja_JP.UTF-8]
1910-04-01	"%EY"	"明治43年"	OK
1911-12-31	"%EY"	"明治44年"	OK
1912-07-29	"%EY"	"明治45年"	OK
1912-07-30	"%EY"	"大正元年"	OK
1913-04-01	"%EY"	"大正02年"	OK
1988-04-01	"%EY"	"昭和63年"	OK
1989-01-07	"%EY"	"昭和64年"	OK
1989-01-08	"%EY"	"平成元年"	OK
1990-04-01	"%EY"	"平成02年"	OK
1997-04-01	"%EY"	"平成09年"	OK
1998-04-01	"%EY"	"平成10年"	OK

1910-04-01	"%_EY"	"明治43年"	OK
1911-12-31	"%_EY"	"明治44年"	OK
1912-07-29	"%_EY"	"明治45年"	OK
1912-07-30	"%_EY"	"大正元年"	OK
1913-04-01	"%_EY"	"大正 2年"	OK
1988-04-01	"%_EY"	"昭和63年"	OK
1989-01-07	"%_EY"	"昭和64年"	OK
1989-01-08	"%_EY"	"平成元年"	OK
1990-04-01	"%_EY"	"平成 2年"	OK
1997-04-01	"%_EY"	"平成 9年"	OK
1998-04-01	"%_EY"	"平成10年"	OK

1910-04-01	"%-EY"	"明治43年"	OK
1911-12-31	"%-EY"	"明治44年"	OK
1912-07-29	"%-EY"	"明治45年"	OK
1912-07-30	"%-EY"	"大正元年"	OK
1913-04-01	"%-EY"	"大正2年"	OK
1988-04-01	"%-EY"	"昭和63年"	OK
1989-01-07	"%-EY"	"昭和64年"	OK
1989-01-08	"%-EY"	"平成元年"	OK
1990-04-01	"%-EY"	"平成2年"	OK
1997-04-01	"%-EY"	"平成9年"	OK
1998-04-01	"%-EY"	"平成10年"	OK

[lo_LA.UTF-8]
1910-04-01	"%EY"	"ພ.ສ. 2453"	OK
1911-12-31	"%EY"	"ພ.ສ. 2454"	OK
1912-07-29	"%EY"	"ພ.ສ. 2455"	OK
1912-07-30	"%EY"	"ພ.ສ. 2455"	OK
1913-04-01	"%EY"	"ພ.ສ. 2456"	OK
1988-04-01	"%EY"	"ພ.ສ. 2531"	OK
1989-01-07	"%EY"	"ພ.ສ. 2532"	OK
1989-01-08	"%EY"	"ພ.ສ. 2532"	OK
1990-04-01	"%EY"	"ພ.ສ. 2533"	OK
1997-04-01	"%EY"	"ພ.ສ. 2540"	OK
1998-04-01	"%EY"	"ພ.ສ. 2541"	OK

1910-04-01	"%_EY"	"ພ.ສ. 2453"	OK
1911-12-31	"%_EY"	"ພ.ສ. 2454"	OK
1912-07-29	"%_EY"	"ພ.ສ. 2455"	OK
1912-07-30	"%_EY"	"ພ.ສ. 2455"	OK
1913-04-01	"%_EY"	"ພ.ສ. 2456"	OK
1988-04-01	"%_EY"	"ພ.ສ. 2531"	OK
1989-01-07	"%_EY"	"ພ.ສ. 2532"	OK
1989-01-08	"%_EY"	"ພ.ສ. 2532"	OK
1990-04-01	"%_EY"	"ພ.ສ. 2533"	OK
1997-04-01	"%_EY"	"ພ.ສ. 2540"	OK
1998-04-01	"%_EY"	"ພ.ສ. 2541"	OK

1910-04-01	"%-EY"	"ພ.ສ. 2453"	OK
1911-12-31	"%-EY"	"ພ.ສ. 2454"	OK
1912-07-29	"%-EY"	"ພ.ສ. 2455"	OK
1912-07-30	"%-EY"	"ພ.ສ. 2455"	OK
1913-04-01	"%-EY"	"ພ.ສ. 2456"	OK
1988-04-01	"%-EY"	"ພ.ສ. 2531"	OK
1989-01-07	"%-EY"	"ພ.ສ. 2532"	OK
1989-01-08	"%-EY"	"ພ.ສ. 2532"	OK
1990-04-01	"%-EY"	"ພ.ສ. 2533"	OK
1997-04-01	"%-EY"	"ພ.ສ. 2540"	OK
1998-04-01	"%-EY"	"ພ.ສ. 2541"	OK

[th_TH.UTF-8]
1910-04-01	"%EY"	"พ.ศ. 2453"	OK
1911-12-31	"%EY"	"พ.ศ. 2454"	OK
1912-07-29	"%EY"	"พ.ศ. 2455"	OK
1912-07-30	"%EY"	"พ.ศ. 2455"	OK
1913-04-01	"%EY"	"พ.ศ. 2456"	OK
1988-04-01	"%EY"	"พ.ศ. 2531"	OK
1989-01-07	"%EY"	"พ.ศ. 2532"	OK
1989-01-08	"%EY"	"พ.ศ. 2532"	OK
1990-04-01	"%EY"	"พ.ศ. 2533"	OK
1997-04-01	"%EY"	"พ.ศ. 2540"	OK
1998-04-01	"%EY"	"พ.ศ. 2541"	OK

1910-04-01	"%_EY"	"พ.ศ. 2453"	OK
1911-12-31	"%_EY"	"พ.ศ. 2454"	OK
1912-07-29	"%_EY"	"พ.ศ. 2455"	OK
1912-07-30	"%_EY"	"พ.ศ. 2455"	OK
1913-04-01	"%_EY"	"พ.ศ. 2456"	OK
1988-04-01	"%_EY"	"พ.ศ. 2531"	OK
1989-01-07	"%_EY"	"พ.ศ. 2532"	OK
1989-01-08	"%_EY"	"พ.ศ. 2532"	OK
1990-04-01	"%_EY"	"พ.ศ. 2533"	OK
1997-04-01	"%_EY"	"พ.ศ. 2540"	OK
1998-04-01	"%_EY"	"พ.ศ. 2541"	OK

1910-04-01	"%-EY"	"พ.ศ. 2453"	OK
1911-12-31	"%-EY"	"พ.ศ. 2454"	OK
1912-07-29	"%-EY"	"พ.ศ. 2455"	OK
1912-07-30	"%-EY"	"พ.ศ. 2455"	OK
1913-04-01	"%-EY"	"พ.ศ. 2456"	OK
1988-04-01	"%-EY"	"พ.ศ. 2531"	OK
1989-01-07	"%-EY"	"พ.ศ. 2532"	OK
1989-01-08	"%-EY"	"พ.ศ. 2532"	OK
1990-04-01	"%-EY"	"พ.ศ. 2533"	OK
1997-04-01	"%-EY"	"พ.ศ. 2540"	OK
1998-04-01	"%-EY"	"พ.ศ. 2541"	OK

[cmn_TW.UTF-8]
1910-04-01	"%EY"	"民前02年"	OK
1911-12-31	"%EY"	"民前01年"	OK
1912-07-29	"%EY"	"民國元年"	OK
1912-07-30	"%EY"	"民國元年"	OK
1913-04-01	"%EY"	"民國02年"	OK
1988-04-01	"%EY"	"民國77年"	OK
1989-01-07	"%EY"	"民國78年"	OK
1989-01-08	"%EY"	"民國78年"	OK
1990-04-01	"%EY"	"民國79年"	OK
1997-04-01	"%EY"	"民國86年"	OK
1998-04-01	"%EY"	"民國87年"	OK

1910-04-01	"%_EY"	"民前 2年"	OK
1911-12-31	"%_EY"	"民前 1年"	OK
1912-07-29	"%_EY"	"民國元年"	OK
1912-07-30	"%_EY"	"民國元年"	OK
1913-04-01	"%_EY"	"民國 2年"	OK
1988-04-01	"%_EY"	"民國77年"	OK
1989-01-07	"%_EY"	"民國78年"	OK
1989-01-08	"%_EY"	"民國78年"	OK
1990-04-01	"%_EY"	"民國79年"	OK
1997-04-01	"%_EY"	"民國86年"	OK
1998-04-01	"%_EY"	"民國87年"	OK

1910-04-01	"%-EY"	"民前2年"	OK
1911-12-31	"%-EY"	"民前1年"	OK
1912-07-29	"%-EY"	"民國元年"	OK
1912-07-30	"%-EY"	"民國元年"	OK
1913-04-01	"%-EY"	"民國2年"	OK
1988-04-01	"%-EY"	"民國77年"	OK
1989-01-07	"%-EY"	"民國78年"	OK
1989-01-08	"%-EY"	"民國78年"	OK
1990-04-01	"%-EY"	"民國79年"	OK
1997-04-01	"%-EY"	"民國86年"	OK
1998-04-01	"%-EY"	"民國87年"	OK

[zh_TW.UTF-8]
1910-04-01	"%EY"	"民前02年"	OK
1911-12-31	"%EY"	"民前01年"	OK
1912-07-29	"%EY"	"民國元年"	OK
1912-07-30	"%EY"	"民國元年"	OK
1913-04-01	"%EY"	"民國02年"	OK
1988-04-01	"%EY"	"民國77年"	OK
1989-01-07	"%EY"	"民國78年"	OK
1989-01-08	"%EY"	"民國78年"	OK
1990-04-01	"%EY"	"民國79年"	OK
1997-04-01	"%EY"	"民國86年"	OK
1998-04-01	"%EY"	"民國87年"	OK

1910-04-01	"%_EY"	"民前 2年"	OK
1911-12-31	"%_EY"	"民前 1年"	OK
1912-07-29	"%_EY"	"民國元年"	OK
1912-07-30	"%_EY"	"民國元年"	OK
1913-04-01	"%_EY"	"民國 2年"	OK
1988-04-01	"%_EY"	"民國77年"	OK
1989-01-07	"%_EY"	"民國78年"	OK
1989-01-08	"%_EY"	"民國78年"	OK
1990-04-01	"%_EY"	"民國79年"	OK
1997-04-01	"%_EY"	"民國86年"	OK
1998-04-01	"%_EY"	"民國87年"	OK

1910-04-01	"%-EY"	"民前2年"	OK
1911-12-31	"%-EY"	"民前1年"	OK
1912-07-29	"%-EY"	"民國元年"	OK
1912-07-30	"%-EY"	"民國元年"	OK
1913-04-01	"%-EY"	"民國2年"	OK
1988-04-01	"%-EY"	"民國77年"	OK
1989-01-07	"%-EY"	"民國78年"	OK
1989-01-08	"%-EY"	"民國78年"	OK
1990-04-01	"%-EY"	"民國79年"	OK
1997-04-01	"%-EY"	"民國86年"	OK
1998-04-01	"%-EY"	"民國87年"	OK

Re: [PATCH 3/3] time: Add tests for Minguo calendar [BZ #24293]

[TAMUKI Shoichi]

Hello Rafal-san,

From: Rafal Luzynski <digitalfreak@lingonborough.com>
Subject: [PATCH 3/3] time: Add tests for Minguo calendar [BZ #24293]
Date: Fri, 15 Mar 2019 12:49:27 +0100 (CET)

> 	[BZ #24293]
> 	* time/Makefile (LOCALES): Add cmn_TW.UTF-8 and zh_TW.UTF-8.
> 	* time/tst-strftime2.c (locales): Likewise.

Since cmn_TW.UTF8 and zh_TW.UTF-8 are practically the same, I think
that the test of only zh_TW.UTF-8 is sufficient.

> 	(dates): Add 1910-04-01, 1911-12-31, 1912-07-29, 1912-07-30,
> 	and 1913-04-01.

It is recommended to add 2010 and 2011 to see Y1C issue of Minguo
calendar.

> 	(mkreftable): Add rules for the new locales and the new dates.

Looks good to me.

> diff --git a/time/Makefile b/time/Makefile
> index 5c6304e..9f70800 100644
> --- a/time/Makefile
> +++ b/time/Makefile
> @@ -50,7 +50,7 @@ include ../Rules
>  ifeq ($(run-built-tests),yes)
>  LOCALES := de_DE.ISO-8859-1 en_US.ISO-8859-1 ja_JP.EUC-JP fr_FR.UTF-8 \
>  	   es_ES.UTF-8 pl_PL.UTF-8 ru_RU.UTF-8 \
> -	   ja_JP.UTF-8 lo_LA.UTF-8 th_TH.UTF-8
> +	   ja_JP.UTF-8 lo_LA.UTF-8 th_TH.UTF-8 cmn_TW.UTF-8 zh_TW.UTF-8
>  include ../gen-locales.mk
>  
>  $(objpfx)tst-ftime_l.out: $(gen-locales)

I think that the addition of only zh_TW.UTF-8 is good enough.

> diff --git a/time/tst-strftime2.c b/time/tst-strftime2.c
> index bf5a66d..fb7f6cd 100644
> --- a/time/tst-strftime2.c
> +++ b/time/tst-strftime2.c
> @@ -26,7 +26,8 @@
>  #include <stdio.h>
>  #include <string.h>
>  
> -static const char *locales[] = { "ja_JP.UTF-8", "lo_LA.UTF-8",
> "th_TH.UTF-8" };
> +static const char *locales[] = { "ja_JP.UTF-8", "lo_LA.UTF-8",
> "th_TH.UTF-8",
> +				 "cmn_TW.UTF-8", "zh_TW.UTF-8" };
>  
>  static const char *formats[] = { "%EY", "%_EY", "%-EY" };
>  

Ditto.

> @@ -37,6 +38,11 @@ typedef struct
>  
>  static const date_t dates[] =
>    {
> +    {  1,  4, 1910 },
> +    { 31, 12, 1911 },
> +    { 29,  7, 1912 },
> +    { 30,  7, 1912 },
> +    { 1, 4, 1913 },
>      { 1, 4, 1988 },
>      { 7, 1, 1989 },
>      { 8, 1, 1989 },

It is recommended to add 2010 and 2011.  Also, please align existing
lines.

Recommend instead:

| @@ -37,12 +38,19 @@ typedef struct
|  
|  static const date_t dates[] =
|    {
| -    { 1, 4, 1988 },
| -    { 7, 1, 1989 },
| -    { 8, 1, 1989 },
| -    { 1, 4, 1990 },
| -    { 1, 4, 1997 },
| -    { 1, 4, 1998 }
| +    {  1,  4, 1910 },
| +    { 31, 12, 1911 },
| +    { 29,  7, 1912 },
| +    { 30,  7, 1912 },
| +    {  1,  4, 1913 },
| +    {  1,  4, 1988 },
| +    {  7,  1, 1989 },
| +    {  8,  1, 1989 },
| +    {  1,  4, 1990 },
| +    {  1,  4, 1997 },
| +    {  1,  4, 1998 },
| +    {  1,  4, 2010 },
| +    {  1,  4, 2011 }
|    };
|  
|  static char ref[array_length (locales)][array_length (formats)]

> @@ -68,8 +74,10 @@ mkreftable (void)
>  {
>    int i, j, k;
>    const char *era;
> -  static const int yrj[] = { 63, 64, 1, 2, 9, 10 };
> -  static const int yrb[] = { 2531, 2532, 2532, 2533, 2540, 2541 };
> +  static const int yrj[] = { 43, 44, 45, 1, 2, 63, 64, 1, 2, 9, 10 };
> +  static const int yrb[] = { 2453, 2454, 2455, 2455, 2456,
> +			     2531, 2532, 2532, 2533, 2540, 2541 };
> +  static const int yrc[] = { 2, 1, 1, 1, 2, 77, 78, 78, 79, 86, 87 };
>  
>    for (i = 0; i < array_length (locales); i++)
>      for (j = 0; j < array_length (formats); j++)

Please add 2010 and 2011 according to the above.

> @@ -77,7 +85,11 @@ mkreftable (void)
>  	{
>  	  if (i == 0)  /* ja_JP  */
>  	    {
> -	      if (is_before (&dates[k], 8, 1, 1989))
> +	      if (is_before (&dates[k], 30, 7, 1912))
> +		era = "\xe6\x98\x8e\xe6\xb2\xbb";
> +	      else if (is_before (&dates[k], 25, 12, 1926))
> +		era = "\xe5\xa4\xa7\xe6\xad\xa3";
> +	      else if (is_before (&dates[k], 8, 1, 1989))
>  		era = "\xe6\x98\xad\xe5\x92\x8c";
>  	      else
>  		era = "\xe5\xb9\xb3\xe6\x88\x90";

OK.

> @@ -104,6 +116,36 @@ mkreftable (void)
>  	      era = "\xe0\xb8\x9e\x2e\xe0\xb8\xa8\x2e ";
>  	      sprintf (ref[i][j][k], "%s%d", era, yrb[k]);
>  	    }
> +	  else if (i == 3)  /* cmn_TW  */
> +	    {
> +	      if (is_before (&dates[k], 1, 1, 1912))
> +		era = "\xe6\xb0\x91\xe5\x89\x8d";
> +	      else
> +		era = "\xe6\xb0\x91\xe5\x9c\x8b";
> +	      if (dates[k].y == 1912)
> +		sprintf (ref[i][j][k], "%s\xe5\x85\x83\xe5\xb9\xb4", era);
> +	      else if (j == 0)
> +		sprintf (ref[i][j][k], "%s%02d\xe5\xb9\xb4", era, yrc[k]);
> +	      else if (j == 1)
> +		sprintf (ref[i][j][k], "%s%2d\xe5\xb9\xb4", era, yrc[k]);
> +	      else
> +		sprintf (ref[i][j][k], "%s%d\xe5\xb9\xb4", era, yrc[k]);
> +	    }
> +	  else if (i == 4)  /* zh_TW  */
> +	    {
> +	      if (is_before (&dates[k], 1, 1, 1912))
> +		era = "\xe6\xb0\x91\xe5\x89\x8d";
> +	      else
> +		era = "\xe6\xb0\x91\xe5\x9c\x8b";
> +	      if (dates[k].y == 1912)
> +		sprintf (ref[i][j][k], "%s\xe5\x85\x83\xe5\xb9\xb4", era);
> +	      else if (j == 0)
> +		sprintf (ref[i][j][k], "%s%02d\xe5\xb9\xb4", era, yrc[k]);
> +	      else if (j == 1)
> +		sprintf (ref[i][j][k], "%s%2d\xe5\xb9\xb4", era, yrc[k]);
> +	      else
> +		sprintf (ref[i][j][k], "%s%d\xe5\xb9\xb4", era, yrc[k]);
> +	    }
>  	  else
>  	    {
>  	      assert (0);  /* Unreachable.  */

Since cmn_TW.UTF8 and zh_TW.UTF-8 are practically the same, you can
omit the test of cmn_TW.

Thank you for your contribution.

Regards,
TAMUKI Shoichi

Re: [PATCH 3/3] time: Add tests for Minguo calendar [BZ #24293]

[Rafal Luzynski]

17.03.2019 11:35 TAMUKI Shoichi <tamuki@linet.gr.jp> wrote:
> 
> Hello Rafal-san,
> 
> From: Rafal Luzynski <digitalfreak@lingonborough.com>
> Subject: [PATCH 3/3] time: Add tests for Minguo calendar [BZ #24293]
> Date: Fri, 15 Mar 2019 12:49:27 +0100 (CET)
> 
> > 	[BZ #24293]
> > 	* time/Makefile (LOCALES): Add cmn_TW.UTF-8 and zh_TW.UTF-8.
> > 	* time/tst-strftime2.c (locales): Likewise.
> 
> Since cmn_TW.UTF8 and zh_TW.UTF-8 are practically the same, I think
> that the test of only zh_TW.UTF-8 is sufficient.

My intention was to ensure that the patch works not just for zh_TW
but also for other locales.  What is the recommended *_TW which is
sufficiently different from zh_TW?

I admit the content of all *_TW locales is the same here.  If indeed
all these locales are so similar I don't mind to remove cmn_TW
and leave zh_TW only.

> > 	(dates): Add 1910-04-01, 1911-12-31, 1912-07-29, 1912-07-30,
> > 	and 1913-04-01.
> 
> It is recommended to add 2010 and 2011 to see Y1C issue of Minguo
> calendar.

Good idea, thank you.

> [...]
> > @@ -37,6 +38,11 @@ typedef struct
> >  
> >  static const date_t dates[] =
> >    {
> > +    {  1,  4, 1910 },
> > +    { 31, 12, 1911 },
> > +    { 29,  7, 1912 },
> > +    { 30,  7, 1912 },
> > +    { 1, 4, 1913 },
> >      { 1, 4, 1988 },
> >      { 7, 1, 1989 },
> >      { 8, 1, 1989 },
> 
> It is recommended to add 2010 and 2011.  Also, please align existing
> lines.
> 
> Recommend instead:
> 
> | @@ -37,12 +38,19 @@ typedef struct
> |  
> |  static const date_t dates[] =
> |    {
> | -    { 1, 4, 1988 },
> | -    { 7, 1, 1989 },
> | -    { 8, 1, 1989 },
> | -    { 1, 4, 1990 },
> | -    { 1, 4, 1997 },
> | -    { 1, 4, 1998 }
> | +    {  1,  4, 1910 },
> | +    { 31, 12, 1911 },
> | +    { 29,  7, 1912 },
> | +    { 30,  7, 1912 },
> | +    {  1,  4, 1913 },
> | +    {  1,  4, 1988 },
> | +    {  7,  1, 1989 },
> | +    {  8,  1, 1989 },
> | +    {  1,  4, 1990 },
> | +    {  1,  4, 1997 },
> | +    {  1,  4, 1998 },
> | +    {  1,  4, 2010 },
> | +    {  1,  4, 2011 }
> |    };
> |  
> |  static char ref[array_length (locales)][array_length (formats)]

I always hesitate to make formatting changes but since the original
author asks for this I will not hesitate this time.

Again, thank you for your feedback and please answer the questions
which you are able to answer.

Regards,

Rafal

Re: [PATCH 3/3] time: Add tests for Minguo calendar [BZ #24293]

[TAMUKI Shoichi]

Hello Rafal-san,

From: Rafal Luzynski <digitalfreak@lingonborough.com>
Subject: Re: [PATCH 3/3] time: Add tests for Minguo calendar [BZ #24293]
Date: Tue, 19 Mar 2019 00:35:55 +0100 (CET)

> > Since cmn_TW.UTF8 and zh_TW.UTF-8 are practically the same, I think
> > that the test of only zh_TW.UTF-8 is sufficient.
> 
> My intention was to ensure that the patch works not just for zh_TW
> but also for other locales.  What is the recommended *_TW which is
> sufficiently different from zh_TW?

Sorry, you are correct.  These *_TW locales are similar to each other,
but they are definitely different.  Therefore, I have now reconsidered
all *_TW locales (except nan_TW@latin) should be checked.

> I admit the content of all *_TW locales is the same here.  If indeed
> all these locales are so similar I don't mind to remove cmn_TW
> and leave zh_TW only.

The *_TW locales are shown below.  Please take care of this order in
accordance with ISO 639-[13].

zh_TW	Chinese
cmn_TW	Mandarin Chinese
hak_TW	Hakka Chinese
nan_TW	Min Nan Chinese
lzh_TW	Literary Chinese

Since the code for these *_TW locales are exactly same, so we can
unify these as follows:

| 	  else if (i >= 3 && i <= 7)  /* {zh,cmn,hak,nan,lzh}_TW  */
| 	    {
| 	      [...]
| 	    }

Regards,
TAMUKI Shoichi

Re: [PATCH 3/3] time: Add tests for Minguo calendar [BZ #24293]

[TAMUKI Shoichi]

Hello Rafal-san,

From: TAMUKI Shoichi <tamuki@linet.gr.jp>
Subject: Re: [PATCH 3/3] time: Add tests for Minguo calendar [BZ #24293]
Date: Wed, 20 Mar 2019 18:21:51 +0900

> Since the code for these *_TW locales are exactly same, so we can
> unify these as follows:
> 
> | 	  else if (i >= 3 && i <= 7)  /* {zh,cmn,hak,nan,lzh}_TW  */
> | 	    {
> | 	      [...]
> | 	    }

I forgot to say:

From: Rafal Luzynski <digitalfreak@lingonborough.com>
Subject: [PATCH 3/3] time: Add tests for Minguo calendar [BZ #24293]
Date: Fri, 15 Mar 2019 12:49:27 +0100 (CET)

> @@ -104,6 +116,36 @@ mkreftable (void)
>  	      era = "\xe0\xb8\x9e\x2e\xe0\xb8\xa8\x2e ";
>  	      sprintf (ref[i][j][k], "%s%d", era, yrb[k]);
>  	    }
> +	  else if (i == 3)  /* cmn_TW  */
> +	    {
> +	      if (is_before (&dates[k], 1, 1, 1912))
> +		era = "\xe6\xb0\x91\xe5\x89\x8d";
> +	      else
> +		era = "\xe6\xb0\x91\xe5\x9c\x8b";
> +	      if (dates[k].y == 1912)
> +		sprintf (ref[i][j][k], "%s\xe5\x85\x83\xe5\xb9\xb4", era);

In your patch, "dates[k].y == 1912" is used instead of "yrc[k] == 1"
to check the first year of the era.  I think it is not desirable to
condition a specific year (1912) here, as it would be cumbersome to
maintain the code if there is an era change in the future.

Yes, I understand that this is because we want to exclude the first
year of before Minguo.  This can be easily solved by making before
Minguo a negative value.  When referring to the actual year, the
absolute value is calculated.

Furthermore, the code can be made common by preparing a variable that
stores the year suffix string for each locale.

So maybe something like this:

| static void
| mkreftable (void)
| {
|   int i, j, k, yr;
|   const char *era, *sfx;
|   static const int yrj[] =
|     {
|       43, 44, 45, 1, 2,
|       63, 64, 1, 2, 9, 10, 22, 23
|     };
|   static const int yrb[] =
|     {
|       2453, 2454, 2455, 2455, 2456,
|       2531, 2532, 2532, 2533, 2540, 2541, 2553, 2554
|     };
|   static const int yrc[] =
|     {
|       -2, -1, 1, 1, 2,
|       77, 78, 78, 79, 86, 87, 99, 100
|     };
| 
|   for (i = 0; i < array_length (locales); i++)
|     for (j = 0; j < array_length (formats); j++)
|       for (k = 0; k < array_length (dates); k++)
| 	{
| 	  if (i == 0)  /* ja_JP  */
| 	    {
| 	      if (is_before (&dates[k], 30, 7, 1912))
| 		era = "\xe6\x98\x8e\xe6\xb2\xbb";
| 	      else if (is_before (&dates[k], 25, 12, 1926))
| 		era = "\xe5\xa4\xa7\xe6\xad\xa3";
| 	      else if (is_before (&dates[k], 8, 1, 1989))
| 		era = "\xe6\x98\xad\xe5\x92\x8c";
| 	      else
| 		era = "\xe5\xb9\xb3\xe6\x88\x90";
| 	      yr = yrj[k], sfx = "\xe5\xb9\xb4";
| 	    }
| 	  else if (i == 1)  /* lo_LA  */
| 	    {
| 	      era = "\xe0\xba\x9e\x2e\xe0\xba\xaa\x2e ";
| 	      yr = yrb[k], sfx = "";
| 	    }
| 	  else if (i == 2)  /* th_TH  */
| 	    {
| 	      era = "\xe0\xb8\x9e\x2e\xe0\xb8\xa8\x2e ";
| 	      yr = yrb[k], sfx = "";
| 	    }
| 	  else if (i >= 3 && i <= 7)  /* {zh,cmn,hak,nan,lzh}_TW  */
| 	    {
| 	      if (is_before (&dates[k], 1, 1, 1912))
| 		era = "\xe6\xb0\x91\xe5\x89\x8d";
| 	      else
| 		era = "\xe6\xb0\x91\xe5\x9c\x8b";
| 	      yr = yrc[k], sfx = "\xe5\xb9\xb4";
| 	    }
| 	  else
| 	    assert (0);  /* Unreachable.  */
| 	  if (yr == 1)
| 	    sprintf (ref[i][j][k], "%s\xe5\x85\x83%s", era, sfx);
| 	  else if (j == 0)
| 	    sprintf (ref[i][j][k], "%s%02d%s", era, abs (yr), sfx);
| 	  else if (j == 1)
| 	    sprintf (ref[i][j][k], "%s%2d%s", era, abs (yr), sfx);
| 	  else
| 	    sprintf (ref[i][j][k], "%s%d%s", era, abs (yr), sfx);
| 	}
| }
| 

In order to use abs function, we need to add #include <stdlib.h>:

| #include <array_length.h>
| #include <stdbool.h>
| #include <assert.h>
| #include <stdlib.h>
| #include <locale.h>
| #include <time.h>
| #include <stdio.h>
| #include <string.h>
| 
| static const char *locales[] =
|   {
|     "ja_JP.UTF-8", "lo_LA.UTF-8", "th_TH.UTF-8",
|     "zh_TW.UTF-8", "cmn_TW.UTF-8", "hak_TW.UTF-8",
|     "nan_TW.UTF-8", "lzh_TW.UTF-8"
|   };
| 

In the initialization of *locales[], I prefer the indentation style
above.

Question:

In the typedef struct, which indentation style is appropriate?

| typedef struct
| {
|   const int d, m, y;
| } date_t;
| 

or

| typedef struct
|   {
|     const int d, m, y;
|   } date_t;
| 

?

Regards,
TAMUKI Shoichi