pg.ddx.io pgsql-committers@postgresql.org mailing list archivehelp / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Ning Yu <nyu@pivotal.io>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Cc: Emre Hasegeli <emre@hasegeli.com>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>
Subject: Re: New Defects reported by Coverity Scan for PostgreSQL
Date: Tue, 31 Jul 2018 22:22:16 -0400
Message-ID: <26769.1533090136@sss.pgh.pa.us> (raw)
In-Reply-To: <CAKmaiL2o6584gLFP=6YX-MUqmvOiT==uny+LA8z3=uAnriuzXA@mail.gmail.com>
References: <E1fjZo1-00019i-GG@gemulon.postgresql.org>
<20180731172149.kdjl3xdjjtwxaj4s@alvherre.pgsql>
<CAKmaiL2o6584gLFP=6YX-MUqmvOiT==uny+LA8z3=uAnriuzXA@mail.gmail.com>
Ning Yu <nyu@pivotal.io> writes:
> From my point of view it's better to also put some comments for humans to
> understand why we are passing l1 and l2 in reverse order.
The header comment for lseg_closept_lseg() is pretty far from adequate
as well. After studying it for awhile I think I've puzzled out the
indeterminacies, but I shouldn't have had to. I think it probably
should have read
* Closest point on line segment l2 to line segment l1
*
* This returns the minimum distance from l1 to the closest point on l2.
* If result is not NULL, the closest point on l2 is stored at *result.
Or perhaps I have it backwards and "l1" and "l2" need to be swapped in
that description. But the mere fact that there is any question about
that means that the function is poorly documented and perhaps poorly
named as well. For that matter, is there a good reason why l1/l2
have those roles and not the reverse?
While Coverity is surely operating from a shaky heuristic here, it's
got a point. The fact that it makes a difference which argument is
given first means that you've got to be really careful about which
is which, and this API spec is giving no help in that regard at all.
regards, tom lane
view thread (13+ messages) latest in thread
Message-ID: <26769.1533090136@sss.pgh.pa.us> Permalink: ../26769.1533090136@sss.pgh.pa.us/ Also on: postgresql.org/message-id/26769.1533090136@sss.pgh.pa.us ·
reply
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Reply to all the recipients using the --to and --cc options: reply via email To: pgsql-committers@postgresql.org Cc: tgl@sss.pgh.pa.us, nyu@pivotal.io, alvherre@2ndquadrant.com, tomas.vondra@2ndquadrant.com, emre@hasegeli.com, pgsql-hackers@postgresql.org Subject: Re: New Defects reported by Coverity Scan for PostgreSQL In-Reply-To: <26769.1533090136@sss.pgh.pa.us> * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions for how to clone and mirror all data and code used for this inbox