Re: New Defects reported by Coverity Scan for PostgreSQL

From: Emre Hasegeli <emre(at)hasegeli(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, Ning Yu <nyu(at)pivotal(dot)io>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: New Defects reported by Coverity Scan for PostgreSQL
Date: 2018-08-01 14:07:47
Message-ID: CAE2gYzyut4=O4qLwY+AkabB7Gm=rkwODr3_9kkAqx9WCFLzfcg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

> I think there are three different things that need to be addressed:
>
> * Underspecified comments.

I agree. My patch added comments, the next one with actual fixes adds
more. I just didn't want to invest even more time on them while the
code is its current shape.

> * The function names and argument names are badly chosen IMO, because even
> granted a convention such as the above, it's not very obvious what roles
> "l1" and "l2" play. I'm not exactly sure what would be better, but if you
> used names like "ofseg" and "otherseg" you'd at least be trying. I'd go
> with an asymmetrical function name too, to make miswriting of calls less
> likely.

Good idea. I haven't though about that, but now such names makes more
sense to me. I will prepare another patch to improve the naming and
comments to be applied on top of my current patches.

> * And lastly, are we sure there aren't actual *bugs* here? I'd initially
> supposed that lseg_closept_lseg acted as Emre says above, but reading the
> code makes me think it's the other way around. Its first two potential
> assignments to *result are definitely assigning points on l2 not l1.

Yes, it is wrong beyond understanding. The committed patch intended
to keep answers as they were. The next one actually fixes those.

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Tom Lane 2018-08-01 16:31:01 pgsql: Fix libpq's code for searching .pgpass; rationalize empty-list-i
Previous Message Tom Lane 2018-08-01 13:57:04 Re: New Defects reported by Coverity Scan for PostgreSQL

Browse pgsql-hackers by date

  From Date Subject
Next Message Dave Cramer 2018-08-01 14:08:38 Re: patch to ensure logical decoding errors early
Previous Message Nikhil Sontakke 2018-08-01 14:00:21 Re: [HACKERS] logical decoding of two-phase transactions