Re: New Defects reported by Coverity Scan for PostgreSQL

Ning Yu <nyu(at)pivotal(dot)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