Re: Possible pointer dereference

On Wed, May 27, 2015 at 8:57 PM, Haribabu Kommi
<kommi(dot)haribabu(at)gmail(dot)com> wrote:
> On Thu, May 28, 2015 at 6:07 AM, Gaetano Mendola <mendola(at)gmail(dot)com> wrote:
>> I'm playing with a static analyzer and it's giving out some real error
>> analyzing postgresql code base like the following one
>>
>> src/backend/access/transam/commit_ts.c
>> return *ts != 0 // line 321
>> but a few line up (line 315) ts is checked for null, so either is not needed
>> to check for null or *ts can lead to a null pointer dereference. Same
>> happens a few line later lines 333 and 339
>
> Thanks for providing detailed information.
>
> The function "TransactionIdGetCommitTsData" is currently used only at
> one place. The caller
> always passes an valid pointer to this function. So there shouldn't be
> a problem. But in future
> if the same function is used at somewhere by passing the NULL pointer
> then it leads to a crash.
>
> By correcting the following way will solve the problem.
>
> return ts ? (*ts != 0) : false; instead of retun *ts != 0;
>
> Attached a patch for it.

If the only caller always passes a valid pointer, there's no point in
adding this check. We have many functions in our source base that
assume that the caller will pass a valid pointer, and changing them
all would make the code bigger, harder to read, and possibly slower,
without any real benefit.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company