Обсуждение: 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
thatoldtup.t_data is   NULL if XLR_BKP_BLOCK(0) while reconstructing incremental   tuples. That fortunately can't
happenas 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

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

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