Skip to content

Commit

Permalink
Fixing a large number of programming errors found by static analysis
Browse files Browse the repository at this point in the history
issues found by the coverity scanner, most are memory leaks on error
conditions.
  • Loading branch information
tbonfort committed Mar 24, 2014
1 parent 3d174b4 commit 55a5688
Show file tree
Hide file tree
Showing 69 changed files with 1,246 additions and 861 deletions.
24 changes: 3 additions & 21 deletions cgiutil.c
Original file line number Diff line number Diff line change
Expand Up @@ -386,33 +386,15 @@ int rind(char *s, char c)
return -1;
}

int _getline(char *s, int n, FILE *f)
{
register int i=0;

while(1) {
s[i] = (char)fgetc(f);

if(s[i] == CR)
s[i] = fgetc(f);

if((s[i] == 0x4) || (s[i] == LF) || (i == (n-1))) {
s[i] = '\0';
return (feof(f) ? 1 : 0);
}
++i;
}
}

void send_fd(FILE *f, FILE *fd)
{
char c;
int c;

while (1) {
c = fgetc(f);
if(feof(f))
if(c == EOF)
return;
fputc(c,fd);
fputc((char)c,fd);
}
}

Expand Down
1 change: 0 additions & 1 deletion cgiutil.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ MS_DLL_EXPORT char x2c(char *);
MS_DLL_EXPORT void unescape_url(char *);
MS_DLL_EXPORT void plustospace(char *);
MS_DLL_EXPORT int rind(char *, char);
MS_DLL_EXPORT int _getline(char *, int, FILE *);
MS_DLL_EXPORT void send_fd(FILE *, FILE *);
MS_DLL_EXPORT int ind(char *, char);
MS_DLL_EXPORT void escape_shell_cmd(char *);
Expand Down
2 changes: 1 addition & 1 deletion hittest.c
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ void initClassHitTests(classObj *c, class_hittest *ch, int default_status) {
void initLayerHitTests(layerObj *l, layer_hittest *lh) {
int i,default_status;
lh->classhits = msSmallCalloc(l->numclasses,sizeof(class_hittest));
lh->status = default_status;

switch(l->type) {
case MS_LAYER_POLYGON:
Expand All @@ -71,6 +70,7 @@ void initLayerHitTests(layerObj *l, layer_hittest *lh) {
default_status = 1; /* no hittesting needed, use traditional mode */
break;
}
lh->status = default_status;
for(i=0; i<l->numclasses; i++) {
initClassHitTests(l->class[i],&lh->classhits[i], default_status);
}
Expand Down
31 changes: 13 additions & 18 deletions mapagg.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,6 @@ class polygon_adaptor
}
private:
shapeObj *s;
double ox,oy;
lineObj *m_line, /*pointer to current line*/
*m_lend; /*pointer to after last line of the shape*/
pointObj *m_point, /*pointer to current vertex*/
Expand Down Expand Up @@ -562,44 +561,41 @@ int agg2RenderBitmapGlyphs(imageObj *img, double x, double y, labelStyleObj *sty
glyph_gen glyph(0);
mapserver::renderer_raster_htext_solid<renderer_base, glyph_gen> rt(r->m_renderer_base, glyph);
glyph.font(rasterfonts[size]);
int numlines=0;
char **lines;
int numlines=1;
char **lines = NULL;
/*masking out the out-of-range character codes*/
int len;
int cc_start = rasterfonts[size][2];
int cc_end = cc_start + rasterfonts[size][3];
if(msCountChars(text,'\n')) {
if((lines = msStringSplit((const char*)text, '\n', &(numlines))) == NULL)
return(-1);
} else {
lines = &text;
numlines = 1;
}
y -= glyph.base_line();
for(int n=0; n<numlines; n++) {
len = strlen(lines[n]);
if(lines) text = lines[n];
len = strlen(text);
for (int i = 0; i < len; i++)
if (lines[n][i] < cc_start || lines[n][i] > cc_end)
lines[n][i] = '.';
if (text[i] < cc_start || text[i] > cc_end)
text[i] = '.';
if(style->outlinewidth > 0) {
rt.color(aggColor(style->outlinecolor));
for(int i=-1; i<=1; i++) {
for(int j=-1; j<=1; j++) {
if(i||j) {
rt.render_text(x+i, y+j, lines[n], true);
rt.render_text(x+i, y+j, text, true);
}
}
}
}
assert(style->color);
rt.color(aggColor(style->color));
rt.render_text(x, y, lines[n], true);
rt.render_text(x, y, text, true);
y += glyph.height();
}
if(*lines != text)
if(lines)
msFreeCharArray(lines, numlines);
return MS_SUCCESS;
return MS_SUCCESS;
}

int agg2RenderGlyphsLine(imageObj *img, labelPathObj *labelpath, labelStyleObj *style, char *text)
Expand Down Expand Up @@ -820,6 +816,10 @@ int agg2RenderTruetypeSymbol(imageObj *img, double x, double y,

msUTF8ToUniChar(symbol->character, &unicode);
const mapserver::glyph_cache* glyph = cache->m_fman.glyph(unicode);
if(!glyph || glyph->glyph_index == 0) {
msSetError(MS_TTFERR, "invalid/not-found glpyh index", "agg2RenderTruetypeSymbol()");
return MS_FAILURE;
}
double ox = (glyph->bounds.x1 + glyph->bounds.x2) / 2.;
double oy = (glyph->bounds.y1 + glyph->bounds.y2) / 2.;

Expand Down Expand Up @@ -994,11 +994,6 @@ int agg2GetTruetypeTextBBox(rendererVTableObj *renderer, char **fonts, int numfo
const mapserver::glyph_cache* glyph;
string += msUTF8ToUniChar(string, &unicode);

if(curfontidx != 0) {
if(aggLoadFont(cache,fonts[0],size) == MS_FAILURE)
return MS_FAILURE;
curfontidx = 0;
}
glyph = cache->m_fman.glyph(unicode);
if(!glyph || glyph->glyph_index == 0) {
int i;
Expand Down
8 changes: 3 additions & 5 deletions mapcairo.c
Original file line number Diff line number Diff line change
Expand Up @@ -855,9 +855,7 @@ static void msTransformToGeospatialPDF(imageObj *img, mapObj *map, cairo_rendere
msBufferResize(r->outputStream, nFileSize);

VSIFSeekL(fp, 0, SEEK_SET);
VSIFReadL(r->outputStream->data, 1, nFileSize, fp);

r->outputStream->size = nFileSize;
r->outputStream->size = VSIFReadL(r->outputStream->data, 1, nFileSize, fp);

VSIFCloseL(fp);
fp = NULL;
Expand Down Expand Up @@ -990,7 +988,7 @@ int getRasterBufferCopyCairo(imageObj *img, rasterBufferObj *rb)
rb->data.rgba.pixel_step=4;
rb->width = cairo_image_surface_get_width(r->surface);
rb->height = cairo_image_surface_get_height(r->surface);
pb = (unsigned char*)malloc(rb->height * rb->data.rgba.row_step * sizeof(unsigned char*));
pb = (unsigned char*)malloc(rb->height * rb->data.rgba.row_step * sizeof(unsigned char));
memcpy(pb,cairo_image_surface_get_data(r->surface),rb->height * rb->data.rgba.row_step);
rb->data.rgba.pixels = pb;
rb->data.rgba.r = &(pb[2]);
Expand All @@ -1012,7 +1010,7 @@ int mergeRasterBufferCairo(imageObj *img, rasterBufferObj *rb, double opacity,
cairo_surface_t *src;
cairo_renderer *r;
/* not implemented for src,dst,width and height */
if(!rb->type == MS_BUFFER_BYTE_RGBA) {
if(rb->type != MS_BUFFER_BYTE_RGBA) {
return MS_FAILURE;
}
r = CAIRO_RENDERER(img);
Expand Down
3 changes: 2 additions & 1 deletion mapcontext.c
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,7 @@ int msLoadMapContextLayerFormat(CPLXMLNode *psFormat, layerObj *layer)
pszValue = msLookupHashTable(&(layer->metadata), "wms_format");

if (
pszValue && (
#if !(defined USE_GD_PNG || defined USE_PNG)
strcasecmp(pszValue, "image/png") == 0 ||
strcasecmp(pszValue, "PNG") == 0 ||
Expand All @@ -404,7 +405,7 @@ int msLoadMapContextLayerFormat(CPLXMLNode *psFormat, layerObj *layer)
strcasecmp(pszValue, "image/gif") == 0 ||
strcasecmp(pszValue, "GIF") == 0 ||
#endif
0 ) {
0 )) {
char **papszList=NULL;
int i, numformats=0;

Expand Down
4 changes: 4 additions & 0 deletions mapcontour.c
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,10 @@ static int msContourLayerReadRaster(layerObj *layer, rectObj rect)
src_yoff = 0;
dst_xsize = src_xsize = MIN(map->width,src_xsize);
dst_ysize = src_ysize = MIN(map->height,src_ysize);
copyRect.minx = copyRect.miny = 0;
copyRect.maxx = map->width;
copyRect.maxy = map->height;
dst_cellsize_y = dst_cellsize_x = 1;
}

/* -------------------------------------------------------------------- */
Expand Down
3 changes: 0 additions & 3 deletions mapcopy.c
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,6 @@ int msCopyLeader(labelLeaderObj *dst, labelLeaderObj *src)
if( freeStyle(dst->styles[i]) == MS_SUCCESS ) msFree(dst->styles[i]);
}
}
msFree(dst->styles);
dst->numstyles = 0;

for (i = 0; i < src->numstyles; i++) {
Expand Down Expand Up @@ -366,7 +365,6 @@ int msCopyLabel(labelObj *dst, labelObj *src)
if( freeStyle(dst->styles[i]) == MS_SUCCESS ) msFree(dst->styles[i]);
}
}
msFree(dst->styles);
dst->numstyles = 0;

for (i = 0; i < src->numstyles; i++) {
Expand Down Expand Up @@ -524,7 +522,6 @@ int msCopyClass(classObj *dst, classObj *src, layerObj *layer)
}
}
}
msFree(dst->styles);
dst->numstyles = 0;

for (i = 0; i < src->numstyles; i++) {
Expand Down
20 changes: 8 additions & 12 deletions mapdebug.c
Original file line number Diff line number Diff line change
Expand Up @@ -90,17 +90,13 @@ debugInfoObj *msGetDebugInfoObj()
else if( link == NULL || link->next == NULL ) {
debugInfoObj *new_link;

new_link = (debugInfoObj *) malloc(sizeof(debugInfoObj));
if (new_link != NULL) {
new_link->next = debuginfo_list;
new_link->thread_id = thread_id;
new_link->global_debug_level = MS_DEBUGLEVEL_ERRORSONLY;
new_link->debug_mode = MS_DEBUGMODE_OFF;
new_link->errorfile = NULL;
new_link->fp = NULL;
} else
msSetError(MS_MEMERR, "Out of memory allocating %u bytes.\n", "msGetDebugInfoObj()", (unsigned int)sizeof(debugInfoObj));

new_link = (debugInfoObj *) msSmallMalloc(sizeof(debugInfoObj));
new_link->next = debuginfo_list;
new_link->thread_id = thread_id;
new_link->global_debug_level = MS_DEBUGLEVEL_ERRORSONLY;
new_link->debug_mode = MS_DEBUGMODE_OFF;
new_link->errorfile = NULL;
new_link->fp = NULL;
debuginfo_list = new_link;
}

Expand Down Expand Up @@ -146,7 +142,7 @@ int msSetErrorFile(const char *pszErrorFile, const char *pszRelToPath)
pszErrorFile = extended_path;
}

if (debuginfo && debuginfo->errorfile && pszErrorFile &&
if (debuginfo->errorfile && pszErrorFile &&
strcmp(debuginfo->errorfile, pszErrorFile) == 0) {
/* Nothing to do, already writing to the right place */
return MS_SUCCESS;
Expand Down
57 changes: 39 additions & 18 deletions mapdraw.c
Original file line number Diff line number Diff line change
Expand Up @@ -446,14 +446,13 @@ imageObj *msDrawMap(mapObj *map, int querymap)
return(NULL);
}
}
}

if(map->debug >= MS_DEBUGLEVEL_TUNING || lp->debug >= MS_DEBUGLEVEL_TUNING) {
msGettimeofday(&endtime, NULL);
msDebug("msDrawMap(): Layer %d (%s), %.3fs\n",
map->layerorder[i], lp->name?lp->name:"(null)",
(endtime.tv_sec+endtime.tv_usec/1.0e6)-
(starttime.tv_sec+starttime.tv_usec/1.0e6) );
if(map->debug >= MS_DEBUGLEVEL_TUNING || lp->debug >= MS_DEBUGLEVEL_TUNING) {
msGettimeofday(&endtime, NULL);
msDebug("msDrawMap(): Layer %d (%s), %.3fs\n",
map->layerorder[i], lp->name?lp->name:"(null)",
(endtime.tv_sec+endtime.tv_usec/1.0e6)-
(starttime.tv_sec+starttime.tv_usec/1.0e6) );
}
}
}

Expand All @@ -467,6 +466,13 @@ imageObj *msDrawMap(mapObj *map, int querymap)

if(MS_SUCCESS != msEmbedScalebar(map, image)) {
msFreeImage( image );
#if defined(USE_WMS_LYR) || defined(USE_WFS_LYR)
/* Cleanup WMS/WFS Request stuff */
if (pasOWSReqInfo) {
msHTTPFreeRequestObj(pasOWSReqInfo, numOWSRequests);
msFree(pasOWSReqInfo);
}
#endif
return NULL;
}

Expand All @@ -478,6 +484,13 @@ imageObj *msDrawMap(mapObj *map, int querymap)
if(map->legend.status == MS_EMBED && !map->legend.postlabelcache) {
if( msEmbedLegend(map, image) != MS_SUCCESS ) {
msFreeImage( image );
#if defined(USE_WMS_LYR) || defined(USE_WFS_LYR)
/* Cleanup WMS/WFS Request stuff */
if (pasOWSReqInfo) {
msHTTPFreeRequestObj(pasOWSReqInfo, numOWSRequests);
msFree(pasOWSReqInfo);
}
#endif
return NULL;
}
}
Expand Down Expand Up @@ -565,6 +578,13 @@ imageObj *msDrawMap(mapObj *map, int querymap)

if(MS_SUCCESS != msEmbedScalebar(map, image)) {
msFreeImage( image );
#if defined(USE_WMS_LYR) || defined(USE_WFS_LYR)
/* Cleanup WMS/WFS Request stuff */
if (pasOWSReqInfo) {
msHTTPFreeRequestObj(pasOWSReqInfo, numOWSRequests);
msFree(pasOWSReqInfo);
}
#endif
return NULL;
}

Expand Down Expand Up @@ -1270,19 +1290,16 @@ int msDrawQueryLayer(mapObj *map, layerObj *layer, imageObj *image)
/* if MS_HILITE, alter the one style (always at least 1 style), and set a MINDISTANCE for the labelObj to avoid duplicates */
if(map->querymap.style == MS_HILITE) {
if (layer->numclasses > 0) {
colorbuffer = (colorObj*)malloc(layer->numclasses*sizeof(colorObj));
mindistancebuffer = (int*)malloc(layer->numclasses*sizeof(int));

if (colorbuffer == NULL || mindistancebuffer == NULL) {
msSetError(MS_MEMERR, "Failed to allocate memory for colorbuffer/mindistancebuffer", "msDrawQueryLayer()");
return MS_FAILURE;
}
colorbuffer = (colorObj*)msSmallMalloc(layer->numclasses*sizeof(colorObj));
mindistancebuffer = (int*)msSmallMalloc(layer->numclasses*sizeof(int));
}

for(i=0; i<layer->numclasses; i++) {
if(layer->type == MS_LAYER_POLYGON) { /* alter BOTTOM style since that's almost always the fill */
if (layer->class[i]->styles == NULL) {
msSetError(MS_MISCERR, "Don't know how to draw class %s of layer %s without a style definition.", "msDrawQueryLayer()", layer->class[i]->name, layer->name);
msFree(colorbuffer);
msFree(mindistancebuffer);
return(MS_FAILURE);
}
if(MS_VALID_COLOR(layer->class[i]->styles[0]->color)) {
Expand Down Expand Up @@ -1368,7 +1385,11 @@ int msDrawQueryLayer(mapObj *map, layerObj *layer, imageObj *image)
}

if(cache) {
if(insertFeatureList(&shpcache, &shape) == NULL) return(MS_FAILURE); /* problem adding to the cache */
if(insertFeatureList(&shpcache, &shape) == NULL) {
msFree(colorbuffer);
msFree(mindistancebuffer);
return(MS_FAILURE); /* problem adding to the cache */
}
}

maxnumstyles = MS_MAX(maxnumstyles, layer->class[shape.classindex]->numstyles);
Expand Down Expand Up @@ -2229,7 +2250,7 @@ int msDrawPoint(mapObj *map, layerObj *layer, pointObj *point, imageObj *image,
} else
msOffsetPointRelativeTo(point, layer);

if(labeltext) {
if(label) {
if(layer->labelcache) {
if(msAddLabel(map, label, layer->index, classindex, NULL, point, NULL, -1) != MS_SUCCESS) return(MS_FAILURE);
} else {
Expand All @@ -2256,7 +2277,7 @@ int msDrawPoint(mapObj *map, layerObj *layer, pointObj *point, imageObj *image,
if(msScaleInBounds(map->scaledenom, theclass->styles[s]->minscaledenom, theclass->styles[s]->maxscaledenom))
msDrawMarkerSymbol(&map->symbolset, image, point, theclass->styles[s], layer->scalefactor);
}
if(labeltext) {
if(label) {
if(layer->labelcache) {
if(msAddLabel(map, label, layer->index, classindex, NULL, point, NULL, -1) != MS_SUCCESS) return(MS_FAILURE);
} else
Expand Down
Loading

0 comments on commit 55a5688

Please sign in to comment.