Re: New Defects reported by Coverity Scan for PostgreSQL

Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> writes:
> On 08/01/2018 11:55 AM, Emre Hasegeli wrote:
>> Consistency. I organized all xxx_closept_yyy(Point *result, xxx *l1,
>> yyy *l2) functions in a way that they find the find the point on "l1".

> IMHO the main issue here is that the rule is not obvious / documented
> anywhere. I think the best way to do that is by making it clear in a
> comment for each such such function.

I think there are three different things that need to be addressed:

* Underspecified comments.

* 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.

* 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.

regards, tom lane