[FFmpeg-devel] [PATCH 2/2] Make ff_parse_expr() and ff_parse_and_eval_expr() return an int containing an error code.

[Stefano Sabatini]

On date Tuesday 2010-05-25 00:57:57 +0200, Stefano Sabatini encoded:
> On date Saturday 2010-05-22 00:56:20 +0200, Stefano Sabatini encoded:
> > On date Friday 2010-05-21 04:32:28 +0200, Michael Niedermayer encoded:
> > > On Thu, May 20, 2010 at 11:18:29PM +0200, Stefano Sabatini wrote:
> > > > On date Thursday 2010-05-20 19:30:31 +0200, Michael Niedermayer encoded:
> > > > > On Thu, May 20, 2010 at 09:21:03AM +0200, Stefano Sabatini wrote:
> > > > > > On date Thursday 2010-05-20 03:07:08 +0200, Michael Niedermayer encoded:
> > > > > > > On Thu, May 20, 2010 at 02:08:46AM +0200, Stefano Sabatini wrote:
> > > > > > > > Allow these functions to convey to the calling application the reason
> > > > > > > > of the failure, which is not always due to a parsing error but for
> > > > > > > > example it may depend on a memory problem.
> > > > > > > 
> > > > > > > this makes the code more complex
> > > > > > > so iam not sure if this is a good idea.
> > > > > > > Could you explain what use you see in the error code?
> > > > > > 
> > > > > > I'm not very strong about this change, currently the only practical
> > > > > > use I can see is for distinguishing an invalid input error from a
> > > > > > memory error, as they are semantically two very different things, so
> > > > > > I'd prefer to allow programs to distinguish amongst the two.
> > > > > 
> > > > > then maybe pass a int *error and alow it to be NULL
> > > > > its imho more convenient, but its drifting toward bikeshed
> > > > 
> > > > And so I'll leave the choice to you, between these three (in my order
> > > > of preference):
> > > > 
> > > > 1) return an int (more consistent with the rest of the API)
> > > > 
> > > > 2) leave it as it is, this way there is no way to know if the failure
> > > > happened because of a parsing/evaluation error of because other
> > > > errors, simpler interface
> > > > 
> > > > 3) put the error code information in an int* (expressive but somehow
> > > > inconsistent with the rest of the API).
> > > 
> > > no strong oppinon
> > > what is the other devs oppinion on this?
> > 
> > If no one else will show up, I'll apply 1) in three days.
> 
> Patch updated, now it covers also the internal eval.c functions and
> fixes some memleak.
> 
> I'm always intentioned to commit this change if no one is against
> (I'll wait some more few days since I applied some changes to it).

Applied.
-- 
FFmpeg = Fantastic & Forgiving Most Philosophical Emblematic Guru