TransactionIdGetCommitTsData and its dereferenced pointers
Hi all,
TransactionIdGetCommitTsData@commit_ts.c does the following:
if (ts)
*ts = entry.time;
[...]
return *ts != 0;
This is a bad idea, because if TransactionIdGetCommitTsData is called
with ts == NULL this would simply crash. It seems to me that it makes
little sense to have ts == NULL either way because this function is
intended to return a timestamp in any case, hence I think that we
should simply Assert(ts == NULL) and remove those checks.
Petr, Alvaro, perhaps you intended something like the patch attached,
or perhaps something like that? This would be useful to not ERROR
should an OOM happen when allocating a timestamp pointer, though I
doubt that this is the first intention:
return ts != NULL ? *ts != 0 : false;
Regards,
--
Michael
Attachments:
20150812_committs_dereferenced.patchtext/x-diff; charset=US-ASCII; name=20150812_committs_dereferenced.patchDownload+16-6
Michael Paquier wrote:
TransactionIdGetCommitTsData@commit_ts.c does the following:
if (ts)
*ts = entry.time;
[...]
return *ts != 0;
This is a bad idea, because if TransactionIdGetCommitTsData is called
with ts == NULL this would simply crash. It seems to me that it makes
little sense to have ts == NULL either way because this function is
intended to return a timestamp in any case, hence I think that we
should simply Assert(ts == NULL) and remove those checks.
I pushed this. But without the assert, because it's pointless: if the
passed pointer were indeed null, we would still crash later in the
function, getting the same effect as the assert.
Thanks,
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers