<div dir="ltr">Yes, I was thinking that with PARANOIA_LEVEL at zero, we shouldn't need these types of runtime checks at all. (In other words, a function that is reading a geometry should be checking the number of points before attempting to access them, so that we don't need to check for emptiness on every single point access.)  I realize now that this is unrelated to your commit, so maybe it's a question for the broader group. I will experiment with making the change and seeing if it regresses OK.<div><br><div>Dan</div></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Sat, Sep 9, 2017 at 8:35 AM, Sandro Santilli <span dir="ltr"><<a href="mailto:strk@kbt.io" target="_blank">strk@kbt.io</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Fri, Sep 08, 2017 at 08:28:12PM -0400, Daniel Baston wrote:<br>
> Should callers be doing this type of checking themselves, instead of<br>
> incurring the penalty on all point access?<br>
<br>
</span>Yes, I think it was good for that check to be only activated<br>
on PARANOIA_LEVEL > 1<br>
<br>
The change of mine (r15671) is really the outermost caller (from the<br>
liblwgeom point of view) as it's done on a point itself.<br>
<br>
Or do you think  lwpoint_getPointXX_p should also not perform<br>
the test ?<br>
<br>
I'd be fine either way, as long as we then patch the higher levels<br>
(a unit test, in this case, IIRC)<br>
<br>
--strk;<br>
<div class="HOEnZb"><div class="h5"><br>
><br>
> On Fri, Sep 8, 2017 at 5:46 PM, Sandro Santilli <<a href="mailto:strk@kbt.io">strk@kbt.io</a>> wrote:<br>
><br>
> > Author: strk<br>
> > Date: 2017-09-08 14:46:17 -0700 (Fri, 08 Sep 2017)<br>
> > New Revision: 15671<br>
> ><br>
> > Modified:<br>
> >    trunk/liblwgeom/lwpoint.c<br>
> > Log:<br>
> > Do not call getPointxx_p on empty or null pointarray<br>
> ><br>
> > Add more 0 returns on error<br>
> ><br>
> > Modified: trunk/liblwgeom/lwpoint.c<br>
> > ==============================<wbr>==============================<wbr>=======<br>
> > --- trunk/liblwgeom/lwpoint.c   2017-09-08 21:46:10 UTC (rev 15670)<br>
> > +++ trunk/liblwgeom/lwpoint.c   2017-09-08 21:46:17 UTC (rev 15671)<br>
> > @@ -39,24 +39,24 @@<br>
> >  int<br>
> >  lwpoint_getPoint2d_p(const LWPOINT *point, POINT2D *out)<br>
> >  {<br>
> > -       return getPoint2d_p(point->point, 0, out);<br>
> > +       return lwpoint_is_empty(point) ? 0 : getPoint2d_p(point->point, 0,<br>
> > out);<br>
> >  }<br>
> ><br>
> >  /* convenience functions to hide the POINTARRAY */<br>
> >  int<br>
> >  lwpoint_getPoint3dz_p(const LWPOINT *point, POINT3DZ *out)<br>
> >  {<br>
> > -       return getPoint3dz_p(point->point,0,<wbr>out);<br>
> > +       return lwpoint_is_empty(point) ? 0 : getPoint3dz_p(point->point,0,<br>
> > out);<br>
> >  }<br>
> >  int<br>
> >  lwpoint_getPoint3dm_p(const LWPOINT *point, POINT3DM *out)<br>
> >  {<br>
> > -       return getPoint3dm_p(point->point,0,<wbr>out);<br>
> > +       return lwpoint_is_empty(point) ? 0 : getPoint3dm_p(point->point,0,<br>
> > out);<br>
> >  }<br>
> >  int<br>
> >  lwpoint_getPoint4d_p(const LWPOINT *point, POINT4D *out)<br>
> >  {<br>
> > -       return getPoint4d_p(point->point,0,<wbr>out);<br>
> > +       return lwpoint_is_empty(point) ? 0 : getPoint4d_p(point->point,0,<br>
> > out);<br>
> >  }<br>
> ><br>
> >  double<br>
> > @@ -64,7 +64,10 @@<br>
> >  {<br>
> >         POINT4D pt;<br>
> >         if ( lwpoint_is_empty(point) )<br>
> > +       {<br>
> >                 lwerror("lwpoint_get_x called with empty geometry");<br>
> > +               return 0;<br>
> > +       }<br>
> >         getPoint4d_p(point->point, 0, &pt);<br>
> >         return pt.x;<br>
> >  }<br>
> > @@ -74,7 +77,10 @@<br>
> >  {<br>
> >         POINT4D pt;<br>
> >         if ( lwpoint_is_empty(point) )<br>
> > +       {<br>
> >                 lwerror("lwpoint_get_y called with empty geometry");<br>
> > +               return 0;<br>
> > +       }<br>
> >         getPoint4d_p(point->point, 0, &pt);<br>
> >         return pt.y;<br>
> >  }<br>
> > @@ -84,9 +90,15 @@<br>
> >  {<br>
> >         POINT4D pt;<br>
> >         if ( lwpoint_is_empty(point) )<br>
> > +       {<br>
> >                 lwerror("lwpoint_get_z called with empty geometry");<br>
> > +               return 0;<br>
> > +       }<br>
> >         if ( ! FLAGS_GET_Z(point->flags) )<br>
> > +       {<br>
> >                 lwerror("lwpoint_get_z called without z dimension");<br>
> > +               return 0;<br>
> > +       }<br>
> >         getPoint4d_p(point->point, 0, &pt);<br>
> >         return pt.z;<br>
> >  }<br>
> > @@ -96,9 +108,15 @@<br>
> >  {<br>
> >         POINT4D pt;<br>
> >         if ( lwpoint_is_empty(point) )<br>
> > +       {<br>
> >                 lwerror("lwpoint_get_m called with empty geometry");<br>
> > +               return 0;<br>
> > +       }<br>
> >         if ( ! FLAGS_GET_M(point->flags) )<br>
> > +       {<br>
> >                 lwerror("lwpoint_get_m called without m dimension");<br>
> > +               return 0;<br>
> > +       }<br>
> >         getPoint4d_p(point->point, 0, &pt);<br>
> >         return pt.m;<br>
> >  }<br>
</div></div></blockquote></div><br></div>