TransactionIdGetCommitTsData and its dereferenced pointers

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Cc: Petr Jelinek <petr(at)2ndquadrant(dot)com>, alvherre(at)2ndquadrant(dot)com
Subject: TransactionIdGetCommitTsData and its dereferenced pointers
Date: 2015-08-12 05:36:43
Message-ID: CAB7nPqS_kF7vJaQA6qNbcuQnr5B6JBDyxAz5MuXPsB3KtmoBbA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi all,

TransactionIdGetCommitTsData(at)commit_ts(dot)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

Attachment Content-Type Size
20150812_committs_dereferenced.patch text/x-diff 2.1 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2015-08-12 07:29:21 Re: Raising our compiler requirements for 9.6
Previous Message Michael Paquier 2015-08-12 04:37:25 Re: WIP: SCRAM authentication