Re: [PATCHES] code cleanup for tz

[Tom Lane]

Neil Conway [EMAIL PROTECTED] writes:
 This patch makes various cosmetic improvements to the timezone code: 
 remove the use of the register qualifier, make some function declaration 
 syntax a bit more consistent, etc.

I think mostly what you are doing here is causing code drift from the
upstream zic code.  I don't think that's a very good idea, since we
do need to be able to track and apply bug fixes from them from time
to time ...

regards, tom lane

---(end of broadcast)---
TIP 5: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faq

Re: [PATCHES] code cleanup for tz

[Neil Conway]

Tom Lane wrote:

I think mostly what you are doing here is causing code drift from the
upstream zic code.  I don't think that's a very good idea, since we
do need to be able to track and apply bug fixes from them from time
to time ...


Why run pgindent on the timezone code, then? That seems guaranteed to 
cause a lot more merge headaches...


-Neil

---(end of broadcast)---
TIP 8: explain analyze is your friend

Re: [PATCHES] code cleanup for tz

[Neil Conway]

Tom Lane wrote:

Well, it's certainly hopeless to expect patch to fix it :-(.  But the
further the code drifts the harder it gets to compare manually.


Sure, but I don't see how removing a few register qualifiers and so 
forth is going to make the slightest difference to a manual comparison. 
If the code was intentionally being kept as close to upstream as 
possible, I wouldn't have made the changes in the first place -- but 
since there is little prospect of doing a machine-assisted merge after 
pgindent, I don't think this patch makes things appreciably worse.


-Neil

---(end of broadcast)---
TIP 4: Don't 'kill -9' the postmaster

Re: [PATCHES] code cleanup for tz

[Bruce Momjian]

Tom Lane wrote:
 Neil Conway [EMAIL PROTECTED] writes:
  Tom Lane wrote:
  I think mostly what you are doing here is causing code drift from the
  upstream zic code.  I don't think that's a very good idea, since we
  do need to be able to track and apply bug fixes from them from time
  to time ...
 
  Why run pgindent on the timezone code, then? That seems guaranteed to 
  cause a lot more merge headaches...
 
 Well, it's certainly hopeless to expect patch to fix it :-(.  But the
 further the code drifts the harder it gets to compare manually.
 
 This isn't an academic concern; I went through it once already, and I
 expect we'll want to do it again every release cycle when we sync the
 zic database with upstream.

I think the idea is that we will pgindent the new release of the
timezone code and then diff that against our current CVS.

-- 
  Bruce Momjian|  http://candle.pha.pa.us
  pgman@candle.pha.pa.us   |  (610) 359-1001
  +  If your life is a hard drive, |  13 Roberts Road
  +  Christ can be your backup.|  Newtown Square, Pennsylvania 19073

---(end of broadcast)---
TIP 5: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faq

[PATCHES] code cleanup for tz

[Neil Conway]

This patch makes various cosmetic improvements to the timezone code: 
remove the use of the register qualifier, make some function declaration 
syntax a bit more consistent, etc.


I'll apply this later today, barring any objections.

-Neil
Index: src/timezone/ialloc.c
===
RCS file: /var/lib/cvs/pgsql/src/timezone/ialloc.c,v
retrieving revision 1.5
diff -c -r1.5 ialloc.c
*** src/timezone/ialloc.c	21 May 2004 20:59:10 -	1.5
--- src/timezone/ialloc.c	20 Jun 2005 05:25:30 -
***
*** 38,46 
  char *
  icatalloc(char *old, const char *new)
  {
! 	register char *result;
! 	register int oldsize,
! newsize;
  
  	newsize = (new == NULL) ? 0 : strlen(new);
  	if (old == NULL)
--- 38,46 
  char *
  icatalloc(char *old, const char *new)
  {
! 	char *result;
! 	int oldsize,
! 		newsize;
  
  	newsize = (new == NULL) ? 0 : strlen(new);
  	if (old == NULL)
Index: src/timezone/localtime.c
===
RCS file: /var/lib/cvs/pgsql/src/timezone/localtime.c,v
retrieving revision 1.10
diff -c -r1.10 localtime.c
*** src/timezone/localtime.c	19 Apr 2005 03:13:59 -	1.10
--- src/timezone/localtime.c	20 Jun 2005 05:28:27 -
***
*** 81,95 
  static const char *getnum(const char *strp, int *nump, int min, int max);
  static const char *getsecs(const char *strp, long *secsp);
  static const char *getoffset(const char *strp, long *offsetp);
! static const char *getrule(const char *strp, struct rule * rulep);
! static void gmtload(struct state * sp);
! static void gmtsub(const pg_time_t *timep, long offset, struct pg_tm * tmp);
! static void localsub(const pg_time_t *timep, long offset, struct pg_tm * tmp, const pg_tz *tz);
  static void timesub(const pg_time_t *timep, long offset,
! 		const struct state * sp, struct pg_tm * tmp);
  static pg_time_t transtime(pg_time_t janfirst, int year,
! 		   const struct rule * rulep, long offset);
! int	tzparse(const char *name, struct state * sp, int lastditch);
  
  /* GMT timezone */
  static struct state gmtmem;
--- 81,95 
  static const char *getnum(const char *strp, int *nump, int min, int max);
  static const char *getsecs(const char *strp, long *secsp);
  static const char *getoffset(const char *strp, long *offsetp);
! static const char *getrule(const char *strp, struct rule *rulep);
! static void gmtload(struct state *sp);
! static void gmtsub(const pg_time_t *timep, long offset, struct pg_tm *tmp);
! static void localsub(const pg_time_t *timep, long offset, struct pg_tm *tmp, const pg_tz *tz);
  static void timesub(const pg_time_t *timep, long offset,
! 		const struct state *sp, struct pg_tm *tmp);
  static pg_time_t transtime(pg_time_t janfirst, int year,
! 		   const struct rule *rulep, long offset);
! int	tzparse(const char *name, struct state *sp, int lastditch);
  
  /* GMT timezone */
  static struct state gmtmem;
***
*** 113,120 
  static long
  detzcode(const char *codep)
  {
! 	register long result;
! 	register int i;
  
  	result = (codep[0]  0x80) ? ~0L : 0L;
  	for (i = 0; i  4; ++i)
--- 113,120 
  static long
  detzcode(const char *codep)
  {
! 	long result;
! 	int i;
  
  	result = (codep[0]  0x80) ? ~0L : 0L;
  	for (i = 0; i  4; ++i)
***
*** 123,138 
  }
  
  int
! tzload(register const char *name, register struct state * sp)
  {
! 	register const char *p;
! 	register int i;
! 	register int fid;
  
  	if (name == NULL  (name = TZDEFAULT) == NULL)
  		return -1;
  	{
! 		register int doaccess;
  		char		fullname[MAXPGPATH];
  
  		if (name[0] == ':')
--- 123,138 
  }
  
  int
! tzload(const char *name, struct state *sp)
  {
! 	const char *p;
! 	int i;
! 	int fid;
  
  	if (name == NULL  (name = TZDEFAULT) == NULL)
  		return -1;
  	{
! 		int doaccess;
  		char		fullname[MAXPGPATH];
  
  		if (name[0] == ':')
***
*** 209,215 
  		}
  		for (i = 0; i  sp-typecnt; ++i)
  		{
! 			register struct ttinfo *ttisp;
  
  			ttisp = sp-ttis[i];
  			ttisp-tt_gmtoff = detzcode(p);
--- 209,215 
  		}
  		for (i = 0; i  sp-typecnt; ++i)
  		{
! 			struct ttinfo *ttisp;
  
  			ttisp = sp-ttis[i];
  			ttisp-tt_gmtoff = detzcode(p);
***
*** 227,233 
  		sp-chars[i] = '\0';	/* ensure '\0' at end */
  		for (i = 0; i  sp-leapcnt; ++i)
  		{
! 			register struct lsinfo *lsisp;
  
  			lsisp = sp-lsis[i];
  			lsisp-ls_trans = detzcode(p);
--- 227,233 
  		sp-chars[i] = '\0';	/* ensure '\0' at end */
  		for (i = 0; i  sp-leapcnt; ++i)
  		{
! 			struct lsinfo *lsisp;
  
  			lsisp = sp-lsis[i];
  			lsisp-ls_trans = detzcode(p);
***
*** 237,243 
  		}
  		for (i = 0; i  sp-typecnt; ++i)
  		{
! 			register struct ttinfo *ttisp;
  
  			ttisp = sp-ttis[i];
  			if (ttisstdcnt == 0)
--- 237,243 
  		}
  		for (i = 0; i  sp-typecnt; ++i)
  		{
! 			struct ttinfo *ttisp;
  
  			ttisp = sp-ttis[i];
  			if