[FFmpeg-devel] [PATCH]lavfi/avfiltergraph: Do not return ENOMEM if filter is missing

[James Almer]

On 2/5/2018 1:20 PM, Nicolas George wrote:
> James Almer (2018-02-05):
>> If it takes a parameter like the type of "something" you want to alloc
>> then yes, I'd support that policy as there's no way to prevent the
>> argument passed from being invalid, so NULL in that case will not mean
>> ENOMEM but EINVAL.
> 
> Ok.
> 
>> But if there's no parameters at all, or if the parameter is a size one,
>> then the failure can only be OOM.
> 
> I can think of a few cases where the failure could be something else.
> For example if the structure that is allocated contains a mutex, the
> error could come from pthread_mutex_init(), which can return other
> errors than ENOMEM.

We ignore pthread_* errors as the wrappers (w32, os2) can only return 0.
And afaik, things like this are why we many times provide both alloc()
and init() functions.

In any case, while i agree that returning an error value for future API
additions should be heavily encouraged if not enforced for most cases, i
insist that we should not outright forbid the addition of new simple
malloc-wrapping functions, like the kind used to allocate structs where
the size is not part of the ABI, as it's clear OOM will always be the
only case of failure. See most alloc functions in lavu (crypto modules,
packet/frame side data structs, etc).

> 
>> 				    In those cases I prefer the latter as
>> it's more versatile in some situations, like using the function itself
>> as an argument when calling another function.
> 
> That is true, but that also prevents from using the usual idiom:
> 
> 	ret = ...;
> 	if (ret < 0)
> 	    return ret; /* or goto fail */
> 
> It requires an extra "ret = AVERROR(ENOMEM)", plus the braces. I think
> this is more common than calling a function with the result without
> checking it.
> 
> In terms of API regularity and simplicity, I find the hardcoding of
> AVERROR(ENOMEM) in the call site very distasteful.

It has gotten a bit out of hand, true. That's why I'm with you that some
rules needs to be defined.

> 
> Regards,
> 
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>