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

login
register
mail settings
Submitter TAMUKI Shoichi
Date Feb. 6, 2019, 3:29 a.m.
Message ID <201902060329.AA04230@tamuki.linet.gr.jp>
Download mbox | patch
Permalink /patch/31317/
State New
Headers show

Comments

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(-)
Florian Weimer - Feb. 6, 2019, 10:10 a.m.
* 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.
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.
* 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;