Re: [HACKERS] GCC 7 warnings - Mailing list pgsql-hackers

From Tom Lane
Subject Re: [HACKERS] GCC 7 warnings
Date
Msg-id 24498.1491970330@sss.pgh.pa.us
Whole thread Raw
In response to Re: [HACKERS] GCC 7 warnings  (Peter Eisentraut <peter.eisentraut@2ndquadrant.com>)
Responses Re: [HACKERS] GCC 7 warnings  (Peter Eisentraut <peter.eisentraut@2ndquadrant.com>)
List pgsql-hackers
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> Attached is a more refined patch that I propose for PG10 now.  Compared
> to the previous rushed version, this one uses some more precise
> arithmetic to size some of the buffers.

Generally +1 for switching the snprintf calls to use sizeof() rather
than repeating the declared sizes of the arrays.

The change in setup_formatted_log_time seems a bit weird:

-    char        msbuf[8];
+    char        msbuf[10];

The associated call is
sprintf(msbuf, ".%03d", (int) (saved_timeval.tv_usec / 1000));

Now a human can see that saved_timeval.tv_usec must be 0..999999, so
that the %d format item must always emit exactly 3 characters, which
means that really 5 bytes would be enough.  I wouldn't expect a
compiler to know that, but if it's making a generic assumption about
the worst-case width of %d, shouldn't it conclude that we might need
as many as 13 bytes for the buffer?  Why does msbuf[10] satisfy it
if msbuf[8] doesn't?

IOW, if we're going to touch this at all, I'd be inclined to go with
msbuf[16] or so, as being more likely to satisfy compilers that have
decided to try to warn about this.  And maybe we should use snprintf,
just for luck.
        regards, tom lane



pgsql-hackers by date:

Previous
From: Peter Eisentraut
Date:
Subject: Re: [HACKERS] logical replication apply to run with sync commit offby default
Next
From: Pavan Deolasee
Date:
Subject: Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM)