Opened 8 years ago

Closed 7 years ago

#1553 closed task (fixed)

TGEOM serialization documentation file

Reported by: colivier Owned by: colivier
Priority: blocker Milestone: PostGIS 2.0.4
Component: postgis Version: 2.0.x
Keywords: TGEOM serialization Cc:

Description

Text file to describe TGEOM internal serialization (use g_serialized.txt as template)

Change History (9)

comment:1 Changed 8 years ago by robe

Milestone: PostGIS 2.0.0PostGIS 2.0.1

Olivier, Feel free to push this back to 2.0.0 if you think you'll get to it soon.

comment:2 Changed 8 years ago by strk

Milestone: PostGIS 2.0.1PostGIS 2.0.2

Olivier: feel free to push back to 2.0.1 if you think you'll get to it soon :)

comment:3 Changed 7 years ago by robe

Milestone: PostGIS 2.0.2PostGIS 2.0.3

comment:4 Changed 7 years ago by strk

Priority: mediumblocker

Olivier: are you planning to ever do this ?

The unit tests in liblwgeom show very bad memory issues, so much that I'm tempted to propose drop of the whole TGEOM code to avoid the security issue related to its exposure.

I was willing to attempt a fix but the serialization format is not documented.

What I see in tgeom_serialize is confusing, in that the serializer is allocating two distinct memory segments and linking one segment to the other: http://trac.osgeo.org/postgis/browser/tags/2.0.2/liblwgeom/libtgeom.c#L806 http://trac.osgeo.org/postgis/browser/tags/2.0.2/liblwgeom/libtgeom.c#L815 http://trac.osgeo.org/postgis/browser/tags/2.0.2/liblwgeom/libtgeom.c#L818

So the returned pointer is NOT into a _serialized_ representation of the object

comment:5 Changed 7 years ago by colivier

Hi Sandro,

You're right this documentation stuff, was not that much in the top of my priority till now.

I notice, with your post, that it's becoming an issue, and will do (soon).

You mention memory issue. What do you refer to ?

comment:6 Changed 7 years ago by strk

Valgrind reports memory leaks when running the cu_surface unit test. In turn this one points to the serialize routing which is clearly _not_ serializing: lwfree(tser) isn't enough to cleanup the product of tgeom_serialize() because the returned object is not a single memory allocation.

There are no memory errors (other than leaks) shown when running the unit test, evidently because the serialize/deserialize routine are consistent with themselves, but when it comes to put the serialized version into the database it cannot work, right ? But maybe we're never putting it in there ?

Now I realize that the memory errors I were remembering you fixed already (in #665)

Anyway I think for the purpose of this ticket it would be good if you could fix all the memory leaks reported by valgrind while running the unit tests for tgeom. Doing that should reveal the problem with lack of documentation :)

comment:7 Changed 7 years ago by strk

Milestone: PostGIS 2.0.3PostGIS 2.0.4

Olivier: did you ever try to valgrind the cu_tester run ?

cd liblwgeom/cunit;
libtool --mode=execute valgrind --leak-check=full ./cu_tester surface

comment:8 Changed 7 years ago by colivier

As r11205 fix cu_surface missing free call, so now valgrind clean. Remove (for now) TGEOM structure, and use only LWGEOM for TIN/PolyhedralSurface, so documentation is no more an issue.

comment:9 Changed 7 years ago by colivier

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.