Re: Confine vacuum skip logic to lazy_scan_skip

Thomas Munro <thomas(dot)munro(at)gmail(dot)com> writes:
> Thanks! It's green again.

The security team's Coverity instance complained about this patch:

*** CID 1642971: Null pointer dereferences (FORWARD_NULL)
/srv/coverity/git/pgsql-git/postgresql/src/backend/access/heap/vacuumlazy.c: 1295 in lazy_scan_heap()
1289 buf = read_stream_next_buffer(stream, &per_buffer_data);
1290
1291 /* The relation is exhausted. */
1292 if (!BufferIsValid(buf))
1293 break;
1294
>>> CID 1642971: Null pointer dereferences (FORWARD_NULL)
>>> Dereferencing null pointer "per_buffer_data".
1295 blk_info = *((uint8 *) per_buffer_data);
1296 CheckBufferIsPinnedOnce(buf);
1297 page = BufferGetPage(buf);
1298 blkno = BufferGetBlockNumber(buf);
1299
1300 vacrel->scanned_pages++;

Basically, Coverity doesn't understand that a successful call to
read_stream_next_buffer must set per_buffer_data here. I don't
think there's much chance of teaching it that, so we'll just
have to dismiss this item as "intentional, not a bug". However,
I do have a suggestion: I think the "per_buffer_data" variable
should be declared inside the "while (true)" loop not outside.
That way there is no chance of a value being carried across
iterations, so that if for some reason read_stream_next_buffer
failed to do what we expect and did not set per_buffer_data,
we'd be certain to get a null-pointer core dump rather than
accessing data from a previous buffer.

regards, tom lane