Обсуждение: compile warnings in CVS
I get the following compiling the current CVS code with gcc 3.1: ... fe-connect.c: In function `connectDBComplete': fe-connect.c:1081: warning: suggest parentheses around && within || fe-connect.c:1086: warning: implicit declaration of function `gettimeofday' ... pg_controldata.c: In function `main': pg_controldata.c:91: warning: `%c' yields only last 2 digits of year in some locales pg_controldata.c:93: warning: `%c' yields only last 2 digits of year in some locales Cheers, Neil -- Neil Conway <neilconway@rogers.com> PGP Key ID: DB3C29FC
Neil Conway <nconway@klamath.dyndns.org> writes: > I get the following compiling the current CVS code with gcc 3.1: I also get 4 regression test failures, due to Gavin's improvements to the parser error messages. AFAICT no actual problems, the expected error message strings just needed to be updated. Cheers, Neil -- Neil Conway <neilconway@rogers.com> PGP Key ID: DB3C29FC
Neil Conway <nconway@klamath.dyndns.org> writes:
> pg_controldata.c: In function `main':
> pg_controldata.c:91: warning: `%c' yields only last 2 digits of year in some locales
> pg_controldata.c:93: warning: `%c' yields only last 2 digits of year in some locales
Yeah. I was willing to ignore that while pg_controldata was in contrib,
but it's much more annoying when it's in the main tree. Anyone know if
gcc has a --not-quite-so-nannyish warnings mode?
IMHO %c is a perfectly reasonable format choice --- the strftime man
page defines it as %c Locale's appropriate date and time representation.
While we could go over to some %Y-%M-etc-etc notation, that doesn't
strike me as a step forward. pg_controldata's output should be
conveniently human-readable IMHO, and that means following local
conventions.
Another alternative ischar *fmt = "%c";...strftime(..., fmt, ...);
which I think will probably defeat gcc's check (haven't tried it
though).
Does anyone want to argue that %c is actually a bad choice? I think
gcc's just being unreasonable here, but maybe I'm missing something
(and no, Y2K arguments won't change my mind).
regards, tom lane
OK, I have fixed the first two with the following patch. The second pair Tom has commented on. --------------------------------------------------------------------------- Neil Conway wrote: > I get the following compiling the current CVS code with gcc 3.1: > > ... > fe-connect.c: In function `connectDBComplete': > fe-connect.c:1081: warning: suggest parentheses around && within || > fe-connect.c:1086: warning: implicit declaration of function `gettimeofday' > ... > pg_controldata.c: In function `main': > pg_controldata.c:91: warning: `%c' yields only last 2 digits of year in some locales > pg_controldata.c:93: warning: `%c' yields only last 2 digits of year in some locales > > Cheers, > > Neil > > -- > Neil Conway <neilconway@rogers.com> > PGP Key ID: DB3C29FC > > > ---------------------------(end of broadcast)--------------------------- > TIP 4: Don't 'kill -9' the postmaster > -- 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 Index: src/interfaces/libpq/fe-connect.c =================================================================== RCS file: /cvsroot/pgsql-server/src/interfaces/libpq/fe-connect.c,v retrieving revision 1.192 diff -c -r1.192 fe-connect.c *** src/interfaces/libpq/fe-connect.c 17 Aug 2002 12:33:17 -0000 1.192 --- src/interfaces/libpq/fe-connect.c 18 Aug 2002 00:04:07 -0000 *************** *** 19,24 **** --- 19,25 ---- #include <fcntl.h> #include <errno.h> #include <ctype.h> + #include <time.h> #include "libpq-fe.h" #include "libpq-int.h" *************** *** 1078,1095 **** } ! while (NULL == rp || remains.tv_sec > 0 || remains.tv_sec == 0 && remains.tv_usec > 0) { /* * If connecting timeout is set, get current time. */ ! if ( NULL != rp && -1 == gettimeofday(&start_time, NULL)) { conn->status = CONNECTION_BAD; return 0; } ! /* * Wait, if necessary. Note that the initial state (just after * PQconnectStart) is to wait for the socket to select for * writing. --- 1079,1096 ---- } ! while (rp == NULL || remains.tv_sec > 0 || (remains.tv_sec == 0 && remains.tv_usec > 0)) { /* * If connecting timeout is set, get current time. */ ! if (rp != NULL && gettimeofday(&start_time, NULL) == -1) { conn->status = CONNECTION_BAD; return 0; } ! /* * Wait, if necessary. Note that the initial state (just after * PQconnectStart) is to wait for the socket to select for * writing.
Yes, very nanny-ish. Not sure how to turn it off. --------------------------------------------------------------------------- Tom Lane wrote: > Neil Conway <nconway@klamath.dyndns.org> writes: > > pg_controldata.c: In function `main': > > pg_controldata.c:91: warning: `%c' yields only last 2 digits of year in some locales > > pg_controldata.c:93: warning: `%c' yields only last 2 digits of year in some locales > > Yeah. I was willing to ignore that while pg_controldata was in contrib, > but it's much more annoying when it's in the main tree. Anyone know if > gcc has a --not-quite-so-nannyish warnings mode? > > IMHO %c is a perfectly reasonable format choice --- the strftime man > page defines it as > %c Locale's appropriate date and time representation. > While we could go over to some %Y-%M-etc-etc notation, that doesn't > strike me as a step forward. pg_controldata's output should be > conveniently human-readable IMHO, and that means following local > conventions. > > Another alternative is > char *fmt = "%c"; > ... > strftime(..., fmt, ...); > > which I think will probably defeat gcc's check (haven't tried it > though). > > Does anyone want to argue that %c is actually a bad choice? I think > gcc's just being unreasonable here, but maybe I'm missing something > (and no, Y2K arguments won't change my mind). > > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 3: if posting/reading through Usenet, please send an appropriate > subscribe-nomail command to majordomo@postgresql.org so that your > message can get through to the mailing list cleanly > -- 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, Pennsylvania19073
I have applied patches to the regression test to fix this. Thanks. --------------------------------------------------------------------------- Neil Conway wrote: > Neil Conway <nconway@klamath.dyndns.org> writes: > > I get the following compiling the current CVS code with gcc 3.1: > > I also get 4 regression test failures, due to Gavin's improvements to > the parser error messages. AFAICT no actual problems, the expected > error message strings just needed to be updated. > > Cheers, > > Neil > > -- > Neil Conway <neilconway@rogers.com> > PGP Key ID: DB3C29FC > > > ---------------------------(end of broadcast)--------------------------- > TIP 3: if posting/reading through Usenet, please send an appropriate > subscribe-nomail command to majordomo@postgresql.org so that your > message can get through to the mailing list cleanly > -- 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, Pennsylvania19073
I said:
> Another alternative is
> char *fmt = "%c";
> ...
> strftime(..., fmt, ...);
> which I think will probably defeat gcc's check (haven't tried it
> though).
I tried this, and it did shut up the warning in my local copy of gcc.
So I committed it.
> Does anyone want to argue that %c is actually a bad choice?
This is still open to debate if anyone wants to make the case...
regards, tom lane