On Fri, 27 May 2011 21:25:21 -0400, "Ronald S. Bultje" <[email protected]> 
wrote:
> Hi,
> 
> On Fri, May 27, 2011 at 7:53 PM, Stefano Sabatini
> <[email protected]> wrote:
> > +int av_parse_number(double *res, const char *numstr, enum 
> > AVParseNumberType type,
> > +                    double min, double max,
> > +                    int log_offset, void *log_ctx)
> > +{
> > +    ParseUtils parseutils = { &parseutils_class, log_offset, log_ctx };
> > +
> > +    char *tail;
> > +    double d = av_strtod(numstr, &tail);
> > +    if ((unsigned)type >= AV_PARSE_NUM_TYPE_NB) {
> > +        av_log(&parseutils, AV_LOG_ERROR, "Unknown parse number type 
> > '%d'\n", type);
> 
> This is a little bit of an ugly way to set up a logging context each
> time this is called. I admit this code isn't performance-critical,
> just looks a little ... Awkward. Can't we just pass it an existing
> logging context? The point of the log context is generally not to know
> where the error occurred (grep -r rocks), but rather to see what
> object it happened with.

I think those functions have way too many parameters to be considered
developer-friendly. If the problem is that the function has nothing
to log to -- well, then it shouldn't log at all. Just return
an appropriate error code and have the caller deal with it.

--
Anton Khirnov
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to