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

[TAMUKI Shoichi <tamuki at linet dot gr dot jp>]

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