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 <s...@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@lists.osgeo.org > https://lists.osgeo.org/mailman/listinfo/geos-devel
_______________________________________________ geos-devel mailing list geos-devel@lists.osgeo.org https://lists.osgeo.org/mailman/listinfo/geos-devel