Re: Confine vacuum skip logic to lazy_scan_skip

Thomas Munro <thomas.munro@gmail.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