[FFmpeg-devel] [PATCH 2/2] avfilter/formats: add av_warn_unused_result to function prototypes

[Ronald S. Bultje]

Hi,

On Mon, Oct 5, 2015 at 3:20 PM, Ganesh Ajjanagadde <gajjanagadde at gmail.com>
wrote:

> This uses the av_warn_unused_result attribute liberally to catch some
> forms of improper
> usage of functions defined in avfilter/formats.h.
>
> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
> ---
>  libavfilter/formats.h | 38 +++++++++++++++++++-------------------
>  1 file changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/libavfilter/formats.h b/libavfilter/formats.h
> index 5a8ee5e..e01be4a 100644
> --- a/libavfilter/formats.h
> +++ b/libavfilter/formats.h
> @@ -116,25 +116,25 @@ typedef struct AVFilterChannelLayouts {
>   * If a and b do not share any common elements, neither is modified, and
> NULL
>   * is returned.
>   */
> -AVFilterChannelLayouts *ff_merge_channel_layouts(AVFilterChannelLayouts
> *a,
> +av_warn_unused_result AVFilterChannelLayouts
> *ff_merge_channel_layouts(AVFilterChannelLayouts *a,
>                                                   AVFilterChannelLayouts
> *b);
> -AVFilterFormats *ff_merge_samplerates(AVFilterFormats *a,
> +av_warn_unused_result AVFilterFormats
> *ff_merge_samplerates(AVFilterFormats *a,
>                                        AVFilterFormats *b);


I'm not sure this is the intention of warn_unused_result. My understanding
is that you use this for strict error reporting only, i.e. "you need to
check this return code for errors because it may fail!", not for "if you
don't store the result of this call, you're not using my API correctly".

The thing is, if I use ff_merge_samplerates() and I don't store the result,
my application cannot possibly function correctly. It's not possible for it
to function correctly. Imagine the following application:

int main() {
  malloc(2);
  return 0;
}

My application cannot possibly function as intended without storing the
result of malloc(). Yet I don't think anyone is seriously considering
marking malloc with warn_unused_result. Likewise, we should only use
warn_unused_result for cases where your application probably works
correctly without checking a return value, but you should check the return
value anyway because in real usage, the value may indicate problems that
the application should be aware of before continuing its operations, e.g.
avformat_open_input() or something like that.

Ronald