compile warnings in CVS

Started by Neil Conwayover 23 years ago7 messages

nconway@klamath.dyndns.org

over 23 years ago

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

over 23 years ago

In reply to: Neil Conway (#1)

Re: compile warnings in CVS

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

Tom Lane

tgl@sss.pgh.pa.us

over 23 years ago

In reply to: Neil Conway (#1)

Re: compile warnings in CVS

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

Bruce Momjian

pgman@candle.pha.pa.us

over 23 years ago

In reply to: Neil Conway (#1)

1 attachment(s)

Re: compile warnings in CVS

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

Attachments:

/bjm/difftext/plainDownload

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.

Bruce Momjian

pgman@candle.pha.pa.us

over 23 years ago

In reply to: Tom Lane (#3)

Re: compile warnings in CVS

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, Pennsylvania 19073

Bruce Momjian

pgman@candle.pha.pa.us

over 23 years ago

In reply to: Neil Conway (#2)

Re: compile warnings in CVS

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, Pennsylvania 19073

Tom Lane

tgl@sss.pgh.pa.us

over 23 years ago

In reply to: Tom Lane (#3)

Re: compile warnings in CVS

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