From: Tom Lane <tgl@sss.pgh.pa.us>
To: Jeff Davis <jdavis@postgresql.org>
Cc: pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Logical Tape Set: lazily allocate read buffer.
Date: Sun, 16 Feb 2020 10:51:46 -0500
Message-ID: <20351.1581868306@sss.pgh.pa.us> (raw)
In-Reply-To: <E1j2JUY-0001ik-Cc@gemulon.postgresql.org>
References: <E1j2JUY-0001ik-Cc@gemulon.postgresql.org>
Jeff Davis <jdavis@postgresql.org> writes:
> Logical Tape Set: lazily allocate read buffer.
Coverity is not very pleased with this patch: it's spewing warnings like
1112 if (lt->buffer == NULL)
1113 ltsInitReadBuffer(lts, lt);
1114
1115 if (blocknum != lt->curBlockNumber)
1116 {
>>> CID 1458440: Null pointer dereferences (FORWARD_NULL)
>>> Passing null pointer "lt->buffer" to "ltsReadBlock", which dereferences it.
1117 ltsReadBlock(lts, blocknum, (void *) lt->buffer);
1118 lt->curBlockNumber = blocknum;
Evidently, it doesn't think that ltsInitReadBuffer() is guaranteed to
make lt->buffer not null, which is not so surprising considering
that that's coded this way:
if (lt->firstBlockNumber != -1L)
{
Assert(lt->buffer_size > 0);
lt->buffer = palloc(lt->buffer_size);
}
Is there a reason for that to be coded in such an obscure and fragile
fashion, rather than having the test be say "if (lt->buffer == NULL)"?
If we really need a connection to firstBlockNumber, I'd suggest
something like
- if (lt->firstBlockNumber != -1L)
+ if (lt->buffer == NULL)
{
+ Assert(lt->firstBlockNumber != -1L);
Assert(lt->buffer_size > 0);
lt->buffer = palloc(lt->buffer_size);
}
but I'm not sure the extra Assert has any value.
regards, tom lane
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-committers@postgresql.org
Cc: tgl@sss.pgh.pa.us, jdavis@postgresql.org, pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Logical Tape Set: lazily allocate read buffer.
In-Reply-To: <20351.1581868306@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox