# HG changeset patch # User stefano # Date 1275379627 0 # Node ID 026edf66e3a92f852a6dbe5ce675c51dd61f69c8 # Parent 966aa6b53dcf07cbdbfb129fd0cdaaf89dbcbfa3 Make ff_parse_expr() and ff_parse_and_eval_expr() return an int containing an error code. Allow these functions to convey the reason of the failure to the calling function, failure which is not always due to a parsing error but it may depend for example on a memory problem. Also fix several potential memleaks. diff -r 966aa6b53dcf -r 026edf66e3a9 avcodec.h --- a/avcodec.h Mon May 31 22:01:31 2010 +0000 +++ b/avcodec.h Tue Jun 01 08:07:07 2010 +0000 @@ -31,7 +31,7 @@ #define LIBAVCODEC_VERSION_MAJOR 52 #define LIBAVCODEC_VERSION_MINOR 72 -#define LIBAVCODEC_VERSION_MICRO 0 +#define LIBAVCODEC_VERSION_MICRO 1 #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \ LIBAVCODEC_VERSION_MINOR, \ diff -r 966aa6b53dcf -r 026edf66e3a9 eval.c --- a/eval.c Mon May 31 22:01:31 2010 +0000 +++ b/eval.c Tue Jun 01 08:07:07 2010 +0000 @@ -167,7 +167,7 @@ return NAN; } -static AVExpr * parse_expr(Parser *p); +static int parse_expr(AVExpr **e, Parser *p); void ff_free_expr(AVExpr * e) { if (!e) return; @@ -176,20 +176,22 @@ av_freep(&e); } -static AVExpr * parse_primary(Parser *p) { +static int parse_primary(AVExpr **e, Parser *p) +{ AVExpr * d = av_mallocz(sizeof(AVExpr)); char *next= p->s; - int i; + int ret, i; if (!d) - return NULL; + return AVERROR(ENOMEM); /* number */ d->value = av_strtod(p->s, &next); if(next != p->s){ d->type = e_value; p->s= next; - return d; + *e = d; + return 0; } d->value = 1; @@ -199,7 +201,8 @@ p->s+= strlen(p->const_name[i]); d->type = e_const; d->a.const_index = i; - return d; + *e = d; + return 0; } } @@ -208,29 +211,34 @@ av_log(p, AV_LOG_ERROR, "undefined constant or missing (\n"); p->s= next; ff_free_expr(d); - return NULL; + return AVERROR(EINVAL); } p->s++; // "(" if (*next == '(') { // special case do-nothing av_freep(&d); - d = parse_expr(p); + if ((ret = parse_expr(&d, p)) < 0) + return ret; if(p->s[0] != ')'){ av_log(p, AV_LOG_ERROR, "missing )\n"); ff_free_expr(d); - return NULL; + return AVERROR(EINVAL); } p->s++; // ")" - return d; + *e = d; + return 0; } - d->param[0] = parse_expr(p); + if ((ret = parse_expr(&(d->param[0]), p)) < 0) { + ff_free_expr(d); + return ret; + } if(p->s[0]== ','){ p->s++; // "," - d->param[1] = parse_expr(p); + parse_expr(&d->param[1], p); } if(p->s[0] != ')'){ av_log(p, AV_LOG_ERROR, "missing )\n"); ff_free_expr(d); - return NULL; + return AVERROR(EINVAL); } p->s++; // ")" @@ -265,7 +273,8 @@ if(strmatch(next, p->func1_name[i])){ d->a.func1 = p->func1[i]; d->type = e_func1; - return d; + *e = d; + return 0; } } @@ -273,16 +282,18 @@ if(strmatch(next, p->func2_name[i])){ d->a.func2 = p->func2[i]; d->type = e_func2; - return d; + *e = d; + return 0; } } av_log(p, AV_LOG_ERROR, "unknown function\n"); ff_free_expr(d); - return NULL; + return AVERROR(EINVAL); } - return d; + *e = d; + return 0; } static AVExpr * new_eval_expr(int type, int value, AVExpr *p0, AVExpr *p1){ @@ -296,67 +307,116 @@ return e; } -static AVExpr * parse_pow(Parser *p, int *sign){ +static int parse_pow(AVExpr **e, Parser *p, int *sign) +{ *sign= (*p->s == '+') - (*p->s == '-'); p->s += *sign&1; - return parse_primary(p); + return parse_primary(e, p); } -static AVExpr * parse_factor(Parser *p){ - int sign, sign2; - AVExpr * e = parse_pow(p, &sign); +static int parse_factor(AVExpr **e, Parser *p) +{ + int sign, sign2, ret; + AVExpr *e0, *e1, *e2; + if ((ret = parse_pow(&e0, p, &sign)) < 0) + return ret; while(p->s[0]=='^'){ + e1 = e0; p->s++; - e= new_eval_expr(e_pow, 1, e, parse_pow(p, &sign2)); - if (!e) - return NULL; - if (e->param[1]) e->param[1]->value *= (sign2|1); + if ((ret = parse_pow(&e2, p, &sign2)) < 0) { + ff_free_expr(e1); + return ret; + } + e0 = new_eval_expr(e_pow, 1, e1, e2); + if (!e0) { + ff_free_expr(e1); + ff_free_expr(e2); + return AVERROR(ENOMEM); + } + if (e0->param[1]) e0->param[1]->value *= (sign2|1); } - if (e) e->value *= (sign|1); - return e; + if (e0) e0->value *= (sign|1); + + *e = e0; + return 0; } -static AVExpr * parse_term(Parser *p){ - AVExpr * e = parse_factor(p); +static int parse_term(AVExpr **e, Parser *p) +{ + int ret; + AVExpr *e0, *e1, *e2; + if ((ret = parse_factor(&e0, p)) < 0) + return ret; while(p->s[0]=='*' || p->s[0]=='/'){ int c= *p->s++; - e= new_eval_expr(c == '*' ? e_mul : e_div, 1, e, parse_factor(p)); - if (!e) - return NULL; + e1 = e0; + if ((ret = parse_factor(&e2, p)) < 0) { + ff_free_expr(e1); + return ret; + } + e0 = new_eval_expr(c == '*' ? e_mul : e_div, 1, e1, e2); + if (!e0) { + ff_free_expr(e1); + ff_free_expr(e2); + return AVERROR(ENOMEM); + } } - return e; + *e = e0; + return 0; } -static AVExpr * parse_subexpr(Parser *p) { - AVExpr * e = parse_term(p); +static int parse_subexpr(AVExpr **e, Parser *p) +{ + int ret; + AVExpr *e0, *e1, *e2; + if ((ret = parse_term(&e0, p)) < 0) + return ret; while(*p->s == '+' || *p->s == '-') { - e= new_eval_expr(e_add, 1, e, parse_term(p)); - if (!e) - return NULL; + e1 = e0; + if ((ret = parse_term(&e2, p)) < 0) { + ff_free_expr(e1); + return ret; + } + e0 = new_eval_expr(e_add, 1, e1, e2); + if (!e0) { + ff_free_expr(e1); + ff_free_expr(e2); + return AVERROR(ENOMEM); + } }; - return e; + *e = e0; + return 0; } -static AVExpr * parse_expr(Parser *p) { - AVExpr * e; - +static int parse_expr(AVExpr **e, Parser *p) +{ + int ret; + AVExpr *e0, *e1, *e2; if(p->stack_index <= 0) //protect against stack overflows - return NULL; + return AVERROR(EINVAL); p->stack_index--; - e = parse_subexpr(p); - + if ((ret = parse_subexpr(&e0, p)) < 0) + return ret; while(*p->s == ';') { + e1 = e0; + if ((ret = parse_subexpr(&e2, p)) < 0) { + ff_free_expr(e1); + return ret; + } p->s++; - e= new_eval_expr(e_last, 1, e, parse_subexpr(p)); - if (!e) - return NULL; + e0 = new_eval_expr(e_last, 1, e1, e2); + if (!e0) { + ff_free_expr(e1); + ff_free_expr(e2); + return AVERROR(ENOMEM); + } }; p->stack_index++; - - return e; + *e = e0; + return 0; } static int verify_expr(AVExpr * e) { @@ -373,7 +433,7 @@ } } -AVExpr *ff_parse_expr(const char *s, +int ff_parse_expr(AVExpr **expr, const char *s, const char * const *const_name, const char * const *func1_name, double (* const *func1)(void *, double), const char * const *func2_name, double (* const *func2)(void *, double, double), @@ -383,9 +443,10 @@ AVExpr *e = NULL; char *w = av_malloc(strlen(s) + 1); char *wp = w; + int ret = 0; if (!w) - goto end; + return AVERROR(ENOMEM); while (*s) if (!isspace(*s++)) *wp++ = s[-1]; @@ -402,14 +463,17 @@ p.log_offset = log_offset; p.log_ctx = log_ctx; - e = parse_expr(&p); + if ((ret = parse_expr(&e, &p)) < 0) + goto end; if (!verify_expr(e)) { ff_free_expr(e); - e = NULL; + ret = AVERROR(EINVAL); + goto end; } + *expr = e; end: av_free(w); - return e; + return ret; } double ff_eval_expr(AVExpr * e, const double *const_value, void *opaque) { @@ -420,18 +484,22 @@ return eval_expr(&p, e); } -double ff_parse_and_eval_expr(const char *s, +int ff_parse_and_eval_expr(double *d, const char *s, const char * const *const_name, const double *const_value, const char * const *func1_name, double (* const *func1)(void *, double), const char * const *func2_name, double (* const *func2)(void *, double, double), void *opaque, int log_offset, void *log_ctx) { - AVExpr *e = ff_parse_expr(s, const_name, func1_name, func1, func2_name, func2, log_offset, log_ctx); - double d; - if (!e) return NAN; - d = ff_eval_expr(e, const_value, opaque); + AVExpr *e = NULL; + int ret = ff_parse_expr(&e, s, const_name, func1_name, func1, func2_name, func2, log_offset, log_ctx); + + if (ret < 0) { + *d = NAN; + return ret; + } + *d = ff_eval_expr(e, const_value, opaque); ff_free_expr(e); - return d; + return isnan(*d) ? AVERROR(EINVAL) : 0; } #ifdef TEST @@ -448,12 +516,15 @@ }; int main(void){ int i; - printf("%f == 12.7\n", ff_parse_and_eval_expr("1+(5-2)^(3-1)+1/2+sin(PI)-max(-2.2,-3.1)", const_names, const_values, NULL, NULL, NULL, NULL, NULL, 0, NULL)); - printf("%f == 0.931322575\n", ff_parse_and_eval_expr("80G/80Gi", const_names, const_values, NULL, NULL, NULL, NULL, NULL, NULL)); + double d; + ff_parse_and_eval_expr(&d, "1+(5-2)^(3-1)+1/2+sin(PI)-max(-2.2,-3.1)", const_names, const_values, NULL, NULL, NULL, NULL, NULL, 0, NULL); + printf("%f == 12.7\n", d); + ff_parse_and_eval_expr(&d, "80G/80Gi", const_names, const_values, NULL, NULL, NULL, NULL, NULL, NULL); + printf("%f == 0.931322575\n", d); for(i=0; i<1050; i++){ START_TIMER - ff_parse_and_eval_expr("1+(5-2)^(3-1)+1/2+sin(PI)-max(-2.2,-3.1)", const_names, const_values, NULL, NULL, NULL, NULL, NULL, 0, NULL); + ff_parse_and_eval_expr(&d, "1+(5-2)^(3-1)+1/2+sin(PI)-max(-2.2,-3.1)", const_names, const_values, NULL, NULL, NULL, NULL, NULL, 0, NULL); STOP_TIMER("ff_parse_and_eval_expr") } return 0; diff -r 966aa6b53dcf -r 026edf66e3a9 eval.h --- a/eval.h Mon May 31 22:01:31 2010 +0000 +++ b/eval.h Tue Jun 01 08:07:07 2010 +0000 @@ -32,6 +32,8 @@ * Parses and evaluates an expression. * Note, this is significantly slower than ff_eval_expr(). * + * @param res a pointer to a double where is put the result value of + * the expression, or NAN in case of error * @param s expression as a zero terminated string for example "1+2^3+5*5+sin(2/3)" * @param const_name NULL terminated array of zero terminated strings of constant identifers for example {"PI", "E", 0} * @param const_value a zero terminated array of values for the identifers from const_name @@ -41,9 +43,10 @@ * @param func2 NULL terminated array of function pointers for functions which take 2 arguments * @param opaque a pointer which will be passed to all functions from func1 and func2 * @param log_ctx parent logging context - * @return the value of the expression + * @return 0 in case of success, a negative value corresponding to an + * AVERROR code otherwise */ -double ff_parse_and_eval_expr(const char *s, +int ff_parse_and_eval_expr(double *res, const char *s, const char * const *const_name, const double *const_value, const char * const *func1_name, double (* const *func1)(void *, double), const char * const *func2_name, double (* const *func2)(void *, double, double), @@ -52,6 +55,10 @@ /** * Parses an expression. * + * @param expr a pointer where is put an AVExpr containing the parsed + * value in case of successfull parsing, or NULL otherwise. + * The pointed to AVExpr must be freed with ff_free_expr() by the user + * when it is not needed anymore. * @param s expression as a zero terminated string for example "1+2^3+5*5+sin(2/3)" * @param const_name NULL terminated array of zero terminated strings of constant identifers for example {"PI", "E", 0} * @param func1_name NULL terminated array of zero terminated strings of func1 identifers @@ -59,10 +66,10 @@ * @param func2_name NULL terminated array of zero terminated strings of func2 identifers * @param func2 NULL terminated array of function pointers for functions which take 2 arguments * @param log_ctx parent logging context - * @return AVExpr which must be freed with ff_free_expr() by the user when it is not needed anymore - * NULL if anything went wrong + * @return 0 in case of success, a negative value corresponding to an + * AVERROR code otherwise */ -AVExpr *ff_parse_expr(const char *s, +int ff_parse_expr(AVExpr **expr, const char *s, const char * const *const_name, const char * const *func1_name, double (* const *func1)(void *, double), const char * const *func2_name, double (* const *func2)(void *, double, double), diff -r 966aa6b53dcf -r 026edf66e3a9 opt.c --- a/opt.c Mon May 31 22:01:31 2010 +0000 +++ b/opt.c Tue Jun 01 08:07:07 2010 +0000 @@ -165,10 +165,10 @@ else if(!strcmp(buf, "none" )) d= 0; else if(!strcmp(buf, "all" )) d= ~0; else { - d = ff_parse_and_eval_expr(buf, const_names, const_values, NULL, NULL, NULL, NULL, NULL, 0, obj); - if (isnan(d)){ + int res = ff_parse_and_eval_expr(&d, buf, const_names, const_values, NULL, NULL, NULL, NULL, NULL, 0, obj); + if (res < 0) { av_log(obj, AV_LOG_ERROR, "Unable to parse option value \"%s\"\n", val); - return AVERROR(EINVAL); + return res; } } } diff -r 966aa6b53dcf -r 026edf66e3a9 ratecontrol.c --- a/ratecontrol.c Mon May 31 22:01:31 2010 +0000 +++ b/ratecontrol.c Tue Jun 01 08:07:07 2010 +0000 @@ -66,7 +66,7 @@ int ff_rate_control_init(MpegEncContext *s) { RateControlContext *rcc= &s->rc_context; - int i; + int i, res; static const char * const const_names[]={ "PI", "E", @@ -106,10 +106,10 @@ }; emms_c(); - rcc->rc_eq_eval = ff_parse_expr(s->avctx->rc_eq ? s->avctx->rc_eq : "tex^qComp", const_names, func1_names, func1, NULL, NULL, 0, s->avctx); - if (!rcc->rc_eq_eval) { + res = ff_parse_expr(&rcc->rc_eq_eval, s->avctx->rc_eq ? s->avctx->rc_eq : "tex^qComp", const_names, func1_names, func1, NULL, NULL, 0, s->avctx); + if (res < 0) { av_log(s->avctx, AV_LOG_ERROR, "Error parsing rc_eq \"%s\"\n", s->avctx->rc_eq); - return -1; + return res; } for(i=0; i<5; i++){