fix memory leaks

[tomkralidis]

There are various memory leaks / issues when running msautotest/wxs/run_tests.py -valgrind ./msautotest/wxs/sos.map, which can be displayed when checking out the output in msautotest/wxs/results/sos_*.xml.vgrind.txt.txt.

[tomkralidis]

FYI first pass of changes in r7098

[dmorissette]

Tom, are you sure that r7098 is the correct fix? By removing the strdup() calls you actually make references in the sosParamsObj to memory owned by the cgiRequestObj... whether this is a good or a bad thing is arguable, but that makes me nervous in this specific case.

In addition to that, msSOSFreeParamsObj() will attempt to free the memory pointed to by sosparams->pszVersion and other members... but this memory is owned by the cgiRequestObj so it's bad bad bad to do that.

To be consistent with the way we do things elsewhere in MapServer, specifically the wfsParams data structure from which you probably copied the idea, I think you should revert your changes to strdup() all param names as before, and fix the leak by calling msSOSFreeParamsObj() everywhere it is needed.

[tomkralidis]

Replying to dmorissette:

Tom, are you sure that r7098 is the correct fix? By removing the strdup() calls you actually make references in the sosParamsObj to memory owned by the cgiRequestObj... whether this is a good or a bad thing is arguable, but that makes me nervous in this specific case. In addition to that, msSOSFreeParamsObj() will attempt to free the memory pointed to by sosparams->pszVersion and other members... but this memory is owned by the cgiRequestObj so it's bad bad bad to do that. To be consistent with the way we do things elsewhere in MapServer, specifically the wfsParams data structure from which you probably copied the idea, I think you should revert your changes to strdup() all param names as before, and fix the leak by calling msSOSFreeParamsObj() everywhere it is needed.

Daniel: I'm confused; msSOSParseRequest in latest svn trunk does strdup() all param names. r7098 adds the strdup() calls, not remove them.

[dmorissette]

Replying to tomkralidis:

Daniel: I'm confused; msSOSParseRequest in latest svn trunk does strdup() all param names. r7098 adds the strdup() calls, not remove them.

Oopps... sorry, I got into this ticket thinking it was about memory leaks only (as the ticket summary suggests) and didn't notice that the strdup() were actually added and not removed (i.e. fixing a double free instead of a leak).

I think attaching the valgrind output to the ticket, combined with a more complete description of the fixes in comment:1 would have helped... that's why I had to go view the changeset to figure what this "first pass of changes in r7098" comment meant exactly.

[tomkralidis]

Fixed a few of the more major leaks which came with the initial SOS 1.0.0 efforts. Closing for now. Can be reopened as required or new tickets against specific leaks.

[assefa]

memory leak fixed in r7637.

[tomkralidis]

A few more fixed in r7638