tst-strftime2: Define the number of elements in each array

Message ID 201902060329.AA04230@tamuki.linet.gr.jp
State New
Headers show
Series
  • tst-strftime2: Define the number of elements in each array
Related show

Commit Message

TAMUKI Shoichi Feb. 6, 2019, 3:29 a.m.
Define the number of elements in each array (locales, formats, dates)
as nlocales, nformats, ndates, respectively, so that the array for
reference is declared using them in stead of magic numbers.

ChangeLog:

	* time/tst-strftime2.c: Define the number of elements in each array
	(locales, formats, dates) as nlocales, nformats, ndates, respectively,
	so that the array for reference is declared using them in stead of
	magic numbers.
---
 time/tst-strftime2.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

Comments

Florian Weimer Feb. 6, 2019, 10:10 a.m. | #1
* TAMUKI Shoichi:

> Define the number of elements in each array (locales, formats, dates)
> as nlocales, nformats, ndates, respectively, so that the array for
> reference is declared using them in stead of magic numbers.
>
> ChangeLog:
>
> 	* time/tst-strftime2.c: Define the number of elements in each array
> 	(locales, formats, dates) as nlocales, nformats, ndates, respectively,
> 	so that the array for reference is declared using them in stead of
> 	magic numbers.
> ---
>  time/tst-strftime2.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/time/tst-strftime2.c b/time/tst-strftime2.c
> index 57d2144..6c2d359 100644
> --- a/time/tst-strftime2.c
> +++ b/time/tst-strftime2.c
> @@ -25,8 +25,10 @@
>  #include <string.h>
>  
>  static const char *locales[] = { "ja_JP.UTF-8", "lo_LA.UTF-8", "th_TH.UTF-8" };
> +#define nlocales array_length (locales)
>  
>  static const char *formats[] = { "%EY", "%_EY", "%-EY" };
> +#define nformats array_length (formats)
>  
>  static const struct
>  {
> @@ -40,8 +42,9 @@ static const struct
>      { 1, 3, 97 },
>      { 1, 3, 98 }
>    };
> +#define ndates array_length (dates)
>  
> -static char ref[3][3][6][100];
> +static char ref[nlocales][nformats][ndates][100];

I'd suggest using array_length only in this place.  It's true that
array_length (locales) is more to type than nlocales, but I think it's
clearer to readers because they do not have to look up the definition of
nlocales.

Thanks,
Florian
TAMUKI Shoichi Feb. 7, 2019, 2:18 a.m. | #2
Hello Florian-san,

Thank you for your suggestion.

From: Florian Weimer <fweimer@redhat.com>
Subject: Re: [PATCH] tst-strftime2: Define the number of elements in each array
Date: Wed, 06 Feb 2019 11:10:05 +0100

> > Define the number of elements in each array (locales, formats, dates)
> > as nlocales, nformats, ndates, respectively, so that the array for
> > reference is declared using them in stead of magic numbers.
> >
> > ChangeLog:
> >
> > 	* time/tst-strftime2.c: Define the number of elements in each array
> > 	(locales, formats, dates) as nlocales, nformats, ndates, respectively,
> > 	so that the array for reference is declared using them in stead of
> > 	magic numbers.
> > ---
> >  time/tst-strftime2.c | 17 ++++++++++-------
> >  1 file changed, 10 insertions(+), 7 deletions(-)
> >
> > diff --git a/time/tst-strftime2.c b/time/tst-strftime2.c
> > index 57d2144..6c2d359 100644
> > --- a/time/tst-strftime2.c
> > +++ b/time/tst-strftime2.c
> > @@ -25,8 +25,10 @@
> >  #include <string.h>
> >  
> >  static const char *locales[] = { "ja_JP.UTF-8", "lo_LA.UTF-8", "th_TH.UTF-8" };
> > +#define nlocales array_length (locales)
> >  
> >  static const char *formats[] = { "%EY", "%_EY", "%-EY" };
> > +#define nformats array_length (formats)
> >  
> >  static const struct
> >  {
> > @@ -40,8 +42,9 @@ static const struct
> >      { 1, 3, 97 },
> >      { 1, 3, 98 }
> >    };
> > +#define ndates array_length (dates)
> >  
> > -static char ref[3][3][6][100];
> > +static char ref[nlocales][nformats][ndates][100];
> 
> I'd suggest using array_length only in this place.  It's true that
> array_length (locales) is more to type than nlocales, but I think it's
> clearer to readers because they do not have to look up the definition of
> nlocales.

That is reasonable.  I will fix it.

In regards to include/array_length.h, if it is OK, could you please
push to master.

Regards,
TAMUKI Shoichi
Florian Weimer Feb. 7, 2019, 8:47 a.m. | #3
* TAMUKI Shoichi:

> In regards to include/array_length.h, if it is OK, could you please
> push to master.

Sure, I have pushed it.

Thanks,
Florian

Patch

diff --git a/time/tst-strftime2.c b/time/tst-strftime2.c
index 57d2144..6c2d359 100644
--- a/time/tst-strftime2.c
+++ b/time/tst-strftime2.c
@@ -25,8 +25,10 @@ 
 #include <string.h>
 
 static const char *locales[] = { "ja_JP.UTF-8", "lo_LA.UTF-8", "th_TH.UTF-8" };
+#define nlocales array_length (locales)
 
 static const char *formats[] = { "%EY", "%_EY", "%-EY" };
+#define nformats array_length (formats)
 
 static const struct
 {
@@ -40,8 +42,9 @@  static const struct
     { 1, 3, 97 },
     { 1, 3, 98 }
   };
+#define ndates array_length (dates)
 
-static char ref[3][3][6][100];
+static char ref[nlocales][nformats][ndates][100];
 
 static void
 mkreftable (void)
@@ -51,9 +54,9 @@  mkreftable (void)
   static const int yrj[] = { 63, 64, 1, 2, 9, 10 };
   static const int yrb[] = { 2531, 2532, 2532, 2533, 2540, 2541 };
 
-  for (i = 0; i < array_length (locales); i++)
-    for (j = 0; j < array_length (formats); j++)
-      for (k = 0; k < array_length (dates); k++)
+  for (i = 0; i < nlocales; i++)
+    for (j = 0; j < nformats; j++)
+      for (k = 0; k < ndates; k++)
 	{
 	  if (i == 0)
 	    {
@@ -93,7 +96,7 @@  do_test (void)
   size_t r, e;
 
   mkreftable ();
-  for (i = 0; i < array_length (locales); i++)
+  for (i = 0; i < nlocales; i++)
     {
       if (setlocale (LC_ALL, locales[i]) == NULL)
 	{
@@ -101,9 +104,9 @@  do_test (void)
 	  continue;
 	}
       printf ("[%s]\n", locales[i]);
-      for (j = 0; j < array_length (formats); j++)
+      for (j = 0; j < nformats; j++)
 	{
-	  for (k = 0; k < array_length (dates); k++)
+	  for (k = 0; k < ndates; k++)
 	    {
 	      ttm.tm_mday = dates[k].d;
 	      ttm.tm_mon  = dates[k].m;