memory leak in msAddLabel

[sgillies@…]

Found a small leak in msAddLabel.  Exposed when I ran the attached Python
script within valgrind using the mapserver/tests/test.map file like:

[sean@lenny tests]$ valgrind --tool=memcheck
--suppressions=/home/sean/projects/valgrind-python.supp --num-callers=12
--leak-check=yes python testdraw.py

==5304== 5 bytes in 1 blocks are definitely lost in loss record 2 of 75
==5304==    at 0x1B904A80: malloc (vg_replace_malloc.c:131)
==5304==    by 0x1B9DA4EF: strdup (in /lib/tls/libc-2.3.3.so)
==5304==    by 0x1BF1E6B0: msAddLabel (maplabel.c:90)
==5304==    by 0x1BF37792: msDrawShape (mapdraw.c:1246)
==5304==    by 0x1BF36646: msDrawVectorLayer (mapdraw.c:774)
==5304==    by 0x1BF36244: msDrawLayer (mapdraw.c:627)
==5304==    by 0x1BF3598A: msDrawMap (mapdraw.c:322)
==5304==    by 0x1BECD27E: mapObj_draw (mapscript_wrap.c:1438)
==5304==    by 0x1BEEC354: _wrap_mapObj_draw (mapscript_wrap.c:16905)
==5304==    by 0x80EA40C: PyCFunction_Call (in /usr/bin/python)
==5304==    by 0x805B3B6: PyObject_Call (in /usr/bin/python)
==5304==    by 0x80AB92A: (within /usr/bin/python)
==5304==
==5304== LEAK SUMMARY:
==5304==    definitely lost: 5 bytes in 1 blocks.
==5304==    possibly lost:   0 bytes in 0 blocks.
==5304==    still reachable: 1086179 bytes in 708 blocks.
==5304==         suppressed: 200 bytes in 1 blocks.
==5304== Reachable blocks (those to which a pointer was found) are not shown.
==5304== To see them, rerun with: --show-reachable=yes

There's only one label being drawn, so this small leak might be worse if there
are more labels.

[sgillies@…]

Python script

[sgillies@…]

Python error suppression file for valgrind 2.2

[sgillies@…]

Steve, after Frank's fix of bug 838, I'm no longer seeing this memory leak.

[fwarmerdam]

Pere, Sean and I are still seeing this. 

I have isolated the problem to apparently be the font not getting freed.  In
4.5 the following change seems to fix it. 

warmerda@gdal2200[613]% cvs diff mapfile.c
Index: mapfile.c
===================================================================
RCS file: /data2/cvsroot/mapserver/mapfile.c,v
retrieving revision 1.292
diff -r1.292 mapfile.c
4078a4079,4080
>     if( cache->labels[i].label.font != NULL )
>         msFree( cache->labels[i].label.font );


Does anyone see any danger in applying this in 4.5 and 4.4?

[fwarmerdam]

Applied in 4.5.  Waiting for others (ie. Steve) to review before applying 
to 4.4.

[sdlime]

I don't see where that could cause problems, go for it. I think you should be 
able to call msFree without the if-then test since that's done inside msFree 
anyway.

Steve

[sgillies@…]

This fixes the leak i've been seeing in 4.4 as well.  All my other tests pass
and so I am committing this to branch-4-4.