On Sun, Feb 16, 2025 at 1:12 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > > 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".
Is this easy to do? Like is there a list of things from coverity to ignore?
> 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.
Done and pushed. Thanks!
Per Coverity.
CID 1592454: (#1 of 1): Explicit null dereferenced (FORWARD_NULL)
