Opened 15 years ago

Closed 15 years ago

#2938 closed defect (fixed)

Maputil.c : msEvalExpression() Memory Leak

Reported by: laurent Owned by: sdlime
Priority: normal Milestone: 5.4.1 release
Component: MapServer C Library Version: svn-trunk (development)
Severity: normal Keywords:
Cc: laurent

Description

Hello

Valgrind tells me there's a memory leak in the msEvalExpression function, on strdup for tmpstr2 in the for loop.

As it makes Valgrind happy on my tests, I propose to correct it this way :

int msEvalExpression(expressionObj *expression, int itemindex, char **items, int numitems)
{
  int i;
  char *tmpstr=NULL, *tmpstr2=NULL;
  int status;
  int expresult;  /* result of logical expression parsing operation */

  if(!expression->string) return(MS_TRUE); /* empty expressions are ALWAYS true */

  switch(expression->type) {
  case(MS_STRING):
    if(itemindex == -1) {
      msSetError(MS_MISCERR, "Cannot evaluate expression, no item index defined.", "msEvalExpression()");
      return(MS_FALSE);
    }
    if(itemindex >= numitems) {
      msSetError(MS_MISCERR, "Invalid item index.", "msEvalExpression()");
      return(MS_FALSE);
    }
    if(expression->flags & MS_EXP_INSENSITIVE) {
      if(strcasecmp(expression->string, items[itemindex]) == 0) return(MS_TRUE); /* got a match */
    } else {
      if(strcmp(expression->string, items[itemindex]) == 0) return(MS_TRUE); /* got a match */
    }
    break;
  case(MS_EXPRESSION):
    tmpstr = strdup(expression->string);

    for(i=0; i<expression->numitems; i++) {
      tmpstr2 = strdup(items[expression->indexes[i]]);
      tmpstr2 = msReplaceSubstring(tmpstr2, "\'", "\\\'");
      tmpstr2 = msReplaceSubstring(tmpstr2, "\"", "\\\"");
      tmpstr = msReplaceSubstring(tmpstr, expression->items[i], tmpstr2);
      free(tmpstr2);
    }
    msAcquireLock( TLOCK_PARSER );
    msyystate = MS_TOKENIZE_EXPRESSION;
    msyystring = tmpstr; /* set lexer state to EXPRESSION_STRING */
    status = msyyparse();
    expresult = msyyresult;
    msReleaseLock( TLOCK_PARSER );

    if (status != 0)
    {
        msSetError(MS_PARSEERR, "Failed to parse expression: %s", "msEvalExpression", tmpstr);
        free(tmpstr);

        return MS_FALSE;
    }
    free(tmpstr);
    return expresult;

    break;
  case(MS_REGEX):
    if(itemindex == -1) {
      msSetError(MS_MISCERR, "Cannot evaluate expression, no item index defined.", "msEvalExpression()");
      return(MS_FALSE);
    }
    if(itemindex >= numitems) {
      msSetError(MS_MISCERR, "Invalid item index.", "msEvalExpression()");
      return(MS_FALSE);
    }

    if(!expression->compiled) {
      if(expression->flags & MS_EXP_INSENSITIVE)
      {
        if(ms_regcomp(&(expression->regex), expression->string, MS_REG_EXTENDED|MS_REG_NOSUB|MS_REG_ICASE) != 0) { /* compile the expression */
          msSetError(MS_REGEXERR, "Invalid regular expression.", "msEvalExpression()");
          return(MS_FALSE);
        }
      }
      else
      {
        if(ms_regcomp(&(expression->regex), expression->string, MS_REG_EXTENDED|MS_REG_NOSUB) != 0) { /* compile the expression */
          msSetError(MS_REGEXERR, "Invalid regular expression.", "msEvalExpression()");
          return(MS_FALSE);
        }
      }
      expression->compiled = MS_TRUE;
    }

    if(ms_regexec(&(expression->regex), items[itemindex], 0, NULL, 0) == 0) return(MS_TRUE); /* got a match */
    break;
  }

  return(MS_FALSE);
}

Thanks
Laurent BAEY

Change History (2)

comment:1 by laurent, 15 years ago

Cc: laurent added

comment:2 by sdlime, 15 years ago

Milestone: 5.4 release5.4.1 release
Resolution: fixed
Status: newclosed

Fixed in 5.4 branch (r8979) and in trunk (r8980). Closing.

Thanks for the fix!

Steve

Note: See TracTickets for help on using tickets.