[MapServer] SIZEITEM on POINT file causes crash

[jmckenna@…]

I added SIZEITEM as part of bug#1357 and tested it with the new symbol type
HATCH...that works, but if you use SIZEITEM on a POINT file i get the same thing
as this user:

*****
If I publish the minimal map file with SIZEITEM based on an integer
attribute, the size of the symbol doesn't change. Worse: if I add a LABEL
definition, Mapserver crashes, publishing only half a jpeg.

Here is my mapfile with the LABLE definition:

MAP
  #IMAGETYPE JPEG
  EXTENT 189775 4716305 731662 5472414
  SIZE 400 600
  SHAPEPATH "data\shp"
  FONTSET "D:\ms4w\fonts\fonts.txt"

  SYMBOL
    NAME "circle"
    TYPE ellipse
    FILLED true
    POINTS
      8 8
    END
  END

  PROJECTION
   "init=epsg:102010"
  END

  LAYER # States polygon layer begins here
      NAME escale
      DATA escale
      STATUS DEFAULT
      TYPE POINT

      PROJECTION
        "init=epsg:102010"
      END

      LABELITEM "id"
      CLASS
        STYLE
            SYMBOL "circle"
            SIZEITEM "duration"
            COLOR 1 255 1
            #SIZE 28
            #MINSIZE 12
            OUTLINECOLOR 0 0 0
        END
        LABEL
          COLOR 155 135 0
          TYPE TRUETYPE
          FONT arial
          SIZE 12
          FORCE true
          POSITION uc

        END #LABEL
      END #CLASS
  END #LAYER
END

Pierre Racine

[thomas.bonfort@…]

fix a double free on labelcache objects which are memcpy's of the actual styles

[thomas.bonfort@…]

changing resolution to fixed. 

maybe this isn't the right bugreport for the proposed patch, but this patch does
fix a serious memory management issue

[sdlime]

Has this patch been applied? I see it's assigned to me but I don't recall doing 
anything with it...

Steve

[thomas.bonfort@…]

no it hasn't, at least I wasn't informed if it was the case.
I had a look at the bugzilla doc to see if I should have marked this as fixed
myself or not, and understood that yes, but please tell me if I shouldn't have

[sdlime]

It shouldn't be marked as fixed until the patch has been applied and at least 
rudimentary testing has happened. No biggie... I've re-opened.

Steve

[sdlime]

Hmmm... I've applied this patch to 4.8 (will be in beta 3). I'm not sure if this
is the best fix or if we should properly initialize sizeitem and angleitem when
populating the labelcache in the first place. That is, in maplabel.c changing:

/* copy the style symbol name */
if (classPtr->numstyles > 0) {
  for(i=0; i<classPtr->numstyles; i++)
    if (classPtr->styles[i].symbolname) cachePtr->styles[i].symbolname =
strdup(classPtr->styles[i].symbolname);
}

to:

/* copy the style symbol name */
if (classPtr->numstyles > 0) {
  for(i=0; i<classPtr->numstyles; i++) {
    if (classPtr->styles[i].symbolname) cachePtr->styles[i].symbolname =
strdup(classPtr->styles[i].symbolname);
    cachePtr->styles[i].sizeitem = cachePtr->styles[i].angleitem = NULL;
  }
}

Seems safer. Thoughts?

Steve

[sdlime]

Actually I think we should just be using the msCopyStyle() function from
mapcopy.c  (in maplabel.c) that Sean added. That wasn't available then the label
cache stuff was added.

Steve

[sdlime]

Ok, I went with the msCopyStyle() fix. Had to fix a potential problem in that
function first with Sean's help. Seems to work fine although I don't have a
great test case. Thomas can you give it a shot. The fix is in CVS head...

Steve

[thomas.bonfort@…]

tried it here and it seems to be OK.
there still is a small glitch where something probably isn't freed correctly/in
the right order, but this doesn't affect functionality:

720 bytes in 112 blocks are definitely lost in loss record 9 of 14
==10338==    at 0x1B8FF886: malloc (vg_replace_malloc.c:149)
==10338==    by 0x1BF7F41F: strdup (in /lib/tls/i686/cmov/libc-2.3.5.so)
==10338==    by 0x80879B1: msCopyStyle (in /usr/local/bin/shp2img)
==10338==    by 0x80604D9: msAddLabel (in /usr/local/bin/shp2img)
==10338==    by 0x806DD5A: msDrawShape (in /usr/local/bin/shp2img)
==10338==    by 0x806F2D6: msDrawVectorLayer (in /usr/local/bin/shp2img)
==10338==    by 0x806F74E: msDrawLayer (in /usr/local/bin/shp2img)
==10338==    by 0x80703ED: msDrawMap (in /usr/local/bin/shp2img)
==10338==    by 0x804FD00: main (in /usr/local/bin/shp2img)

tb

[jmckenna@…]

i'm wondering if this last small 'glitch' should be tracked in a new bug.  The
initial problem (sizeitem crash on point layer) is fixed.

[sdlime]

Closing this one...