folder:lk/lk date:yesterday..
Hi,
Coverity flagged a couple of issues that seem to worth addressing by
changing the code instead of ignoring them:
01) heap_xlog_update() looks to coverity as if it could trigger a NULL
pointer dereference. That's because it thinks that oldtup.t_data is
NULL if XLR_BKP_BLOCK(0) while reconstructing incremental
tuples. That fortunately can't happen as incremental updates are
only performed if the tuples are on the same page.
Add an Assert().
02) Be a bit more consistent in what type to use for a size
variable. Inconsequential, but more consistent.
03) Don't leak memory after connection aborts in pg_recvlogical.
04) Use a sensible parameter for memset() when randomizing memory in
reorderbuffer. Inconsequential.
Could somebody please pick these up?
I have to say, I am not particularly happy with the complexity of the
control flow in heap_xlog_update() :(.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2014-04-30 17:39:27 +0200, Andres Freund wrote:
Hi,
Coverity flagged a couple of issues that seem to worth addressing by
changing the code instead of ignoring them:01) heap_xlog_update() looks to coverity as if it could trigger a NULL
pointer dereference. That's because it thinks that oldtup.t_data is
NULL if XLR_BKP_BLOCK(0) while reconstructing incremental
tuples. That fortunately can't happen as incremental updates are
only performed if the tuples are on the same page.
Add an Assert().
02) Be a bit more consistent in what type to use for a size
variable. Inconsequential, but more consistent.
03) Don't leak memory after connection aborts in pg_recvlogical.
04) Use a sensible parameter for memset() when randomizing memory in
reorderbuffer. Inconsequential.Could somebody please pick these up?
That might be easier with actual patches...
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
0001-Assert-that-pre-post-fix-updated-tuples-are-on-the-s.patchtext/x-patch; charset=us-asciiDownload+2-1
0002-Use-Size-instead-of-uint32-to-measure-.-size-in-snap.patchtext/x-patch; charset=us-asciiDownload+1-2
0003-Don-t-leak-memory-after-connection-aborts-in-pg_recv.patchtext/x-patch; charset=us-asciiDownload+5-4
0004-Pass-sensible-value-to-memset-when-randomizing-reord.patchtext/x-patch; charset=us-asciiDownload+1-2
On 04/30/2014 06:39 PM, Andres Freund wrote:
Hi,
Coverity flagged a couple of issues that seem to worth addressing by
changing the code instead of ignoring them:01) heap_xlog_update() looks to coverity as if it could trigger a NULL
pointer dereference. That's because it thinks that oldtup.t_data is
NULL if XLR_BKP_BLOCK(0) while reconstructing incremental
tuples. That fortunately can't happen as incremental updates are
only performed if the tuples are on the same page.
Add an Assert().
02) Be a bit more consistent in what type to use for a size
variable. Inconsequential, but more consistent.
03) Don't leak memory after connection aborts in pg_recvlogical.
04) Use a sensible parameter for memset() when randomizing memory in
reorderbuffer. Inconsequential.Could somebody please pick these up?
Committed, thanks.
I have to say, I am not particularly happy with the complexity of the
control flow in heap_xlog_update() :(.
Agreed :-(. It might be good to split it into two functions, one for
same-page updates and another for others. And have two different WAL
record structs for the cases, too.
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers