[geos-devel] Request for Eyeballs

[Paul Ramsey]

I'm not sure moving the std::vector<SegmentString*> to shared pointers will
be necessary, simply using std::vector<unique_ptr<SegmentString>> is
probably sufficient. The "real problem" as you note is just that the
contract isn't explicit anywhere. I think that the move the unique_ptr
across other parts of the API avoided this chunk of code because it was and
remains pretty gnarly. I did not want to refactor it because I'm dealing
with enough mess just bringing in the new code.
A refactor is on my list now though, including some work to reduce some of
the huge amount of copying that goes on under the covers, some of it
unnecessary

In other news, I have identified and removed the memory leaks, everything I
run through valgrind is now clean. So, onwards towards merging the branch
to master, once I get CI green.

P.

On Fri, Jul 24, 2020 at 8:00 AM Sandro Santilli <strk at kbt.io> wrote:

> On Fri, Jul 24, 2020 at 12:07:44PM +0200, Sandro Santilli wrote:
> >
> > The ownership of those SegmentStrings is not documented in
> > NodedSegmentString.h, which would help. My impression is that
> > those segment strings should be shared pointers, to overcome this
> > long standing issue (it was a problem before snaprounding as well).
>
> More information. I think the primary problem with this API is that
> the ownership of SegmentString in Noder.h is not properly documented.
>
> Comments are even clearly expressing the confusion:
>
>      * Some Noders may add all these nodes to the input SegmentStrings;
>      * others may only add some or none at all.
>
> The methods defined for a Noder are:
>
>   virtual void computeNodes(std::vector<SegmentString*>* segStrings) = 0;
>   virtual std::vector<SegmentString*>* getNodedSubstrings() const = 0;
>
> What isn't clear is:
>
>   - Who owns the SegmentString passed to computeNodes ?
>   - Who owns the SegmentString returned by getNodedSubstrings ?
>   - Can the same SegmentString be present in both input and output
>     containers ?
>
> I think these questions should be answered with proper documentation
> in that class, and then we need to make sure all subclass implement
> the documented semantic properly.
>
> Using shared pointers of those SegmentStrings may help simplifying
> the implementations, with the downside of some more overhead. But
> other implementations could be made to work, as long as it is clear
> who is responsible for what...
>
> --strk;
> _______________________________________________
> geos-devel mailing list
> geos-devel at lists.osgeo.org
> https://lists.osgeo.org/mailman/listinfo/geos-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/geos-devel/attachments/20200727/38781bd4/attachment.html>