From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> |
Cc: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: GCC 7 warnings |
Date: | 2017-04-12 04:12:10 |
Message-ID: | 24498.1491970330@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Pavan Deolasee | 2017-04-12 04:23:09 | Re: Patch: Write Amplification Reduction Method (WARM) |
Previous Message | Peter Eisentraut | 2017-04-12 04:10:26 | Re: logical replication apply to run with sync commit off by default |