Re: [FFmpeg-devel] [PATCHv2] avfilter/buffersrc: add av_warn_unused_result attributes

2015-10-11 Thread Ronald S. Bultje
Hi,

On Sun, Oct 11, 2015 at 2:20 PM, Ganesh Ajjanagadde 
wrote:

> On Wed, Oct 7, 2015 at 10:18 AM, Ganesh Ajjanagadde 
> wrote:
> > On Wed, Oct 7, 2015 at 10:01 AM, Clément Bœsch  wrote:
> >> On Wed, Oct 07, 2015 at 09:31:49AM -0400, Ganesh Ajjanagadde wrote:
> >>> On Wed, Oct 7, 2015 at 8:59 AM, Clément Bœsch  wrote:
> >>> > On Tue, Oct 06, 2015 at 06:53:47PM -0400, Ganesh Ajjanagadde wrote:
> >>> >> This adds av_warn_unused_result whenever it is relevant.
> >>> >>
> >>> >> Signed-off-by: Ganesh Ajjanagadde 
> >>> >> ---
> >>> >>  libavfilter/buffersrc.h | 3 +++
> >>> >>  1 file changed, 3 insertions(+)
> >>> >>
> >>> >> diff --git a/libavfilter/buffersrc.h b/libavfilter/buffersrc.h
> >>> >> index cd3d95f..847c093 100644
> >>> >> --- a/libavfilter/buffersrc.h
> >>> >> +++ b/libavfilter/buffersrc.h
> >>> >> @@ -78,6 +78,7 @@ unsigned
> av_buffersrc_get_nb_failed_requests(AVFilterContext *buffer_src);
> >>> >>   * This function is equivalent to av_buffersrc_add_frame_flags()
> with the
> >>> >>   * AV_BUFFERSRC_FLAG_KEEP_REF flag.
> >>> >>   */
> >>> >> +av_warn_unused_result
> >>> >>  int av_buffersrc_write_frame(AVFilterContext *ctx, const AVFrame
> *frame);
> >>> >>
> >>> >>  /**
> >>> >> @@ -98,6 +99,7 @@ int av_buffersrc_write_frame(AVFilterContext
> *ctx, const AVFrame *frame);
> >>> >>   * This function is equivalent to av_buffersrc_add_frame_flags()
> without the
> >>> >>   * AV_BUFFERSRC_FLAG_KEEP_REF flag.
> >>> >>   */
> >>> >> +av_warn_unused_result
> >>> >>  int av_buffersrc_add_frame(AVFilterContext *ctx, AVFrame *frame);
> >>> >>
> >>> >>  /**
> >>> >> @@ -115,6 +117,7 @@ int av_buffersrc_add_frame(AVFilterContext
> *ctx, AVFrame *frame);
> >>> >>   * @return>= 0 in case of success, a negative AVERROR
> code
> >>> >>   *in case of failure
> >>> >>   */
> >>> >> +av_warn_unused_result
> >>> >>  int av_buffersrc_add_frame_flags(AVFilterContext *buffer_src,
> >>> >>   AVFrame *frame, int flags);
> >>> >>
> >>> >
> >>> > Aren't you just supposed to (void)-prefix the call in the caller
> when you
> >>> > explicitly don't care about the result?
> >>> >
> >>> > These functions certainly looks like you actually want to check for
> the
> >>> > result most of the time.
> >>>
> >>> Exactly - this addition to the declaration in the header will trigger
> >>> a warning whenever this function is used without obtaining the return
> >>> value.
> >>>
> >>
> >> Oh, my bad, I misunderstood, sounds indeed saner than what I had in
> mind.
> >>
> >> Thanks for the clarification.
> >
> > By the way, I highly encourage all developers to slowly start adding
> > this to relevant headers, at least in the things they respectively
> > maintain. For instance, a bunch of possible bugs/robustness issues
> > will be fixed. Here is one I noticed today: by applying to
> > init_get_bits8, a few warnings get triggered. There have been commits
> > from Michael addressing some CID's related to this - this will ensure
> > a complete weeding out of that particular issue.
>
> ping


Yes, sorry, pushed.

Ronald
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCHv2] avfilter/buffersrc: add av_warn_unused_result attributes

2015-10-11 Thread Ganesh Ajjanagadde
On Wed, Oct 7, 2015 at 10:18 AM, Ganesh Ajjanagadde  wrote:
> On Wed, Oct 7, 2015 at 10:01 AM, Clément Bœsch  wrote:
>> On Wed, Oct 07, 2015 at 09:31:49AM -0400, Ganesh Ajjanagadde wrote:
>>> On Wed, Oct 7, 2015 at 8:59 AM, Clément Bœsch  wrote:
>>> > On Tue, Oct 06, 2015 at 06:53:47PM -0400, Ganesh Ajjanagadde wrote:
>>> >> This adds av_warn_unused_result whenever it is relevant.
>>> >>
>>> >> Signed-off-by: Ganesh Ajjanagadde 
>>> >> ---
>>> >>  libavfilter/buffersrc.h | 3 +++
>>> >>  1 file changed, 3 insertions(+)
>>> >>
>>> >> diff --git a/libavfilter/buffersrc.h b/libavfilter/buffersrc.h
>>> >> index cd3d95f..847c093 100644
>>> >> --- a/libavfilter/buffersrc.h
>>> >> +++ b/libavfilter/buffersrc.h
>>> >> @@ -78,6 +78,7 @@ unsigned 
>>> >> av_buffersrc_get_nb_failed_requests(AVFilterContext *buffer_src);
>>> >>   * This function is equivalent to av_buffersrc_add_frame_flags() with 
>>> >> the
>>> >>   * AV_BUFFERSRC_FLAG_KEEP_REF flag.
>>> >>   */
>>> >> +av_warn_unused_result
>>> >>  int av_buffersrc_write_frame(AVFilterContext *ctx, const AVFrame 
>>> >> *frame);
>>> >>
>>> >>  /**
>>> >> @@ -98,6 +99,7 @@ int av_buffersrc_write_frame(AVFilterContext *ctx, 
>>> >> const AVFrame *frame);
>>> >>   * This function is equivalent to av_buffersrc_add_frame_flags() 
>>> >> without the
>>> >>   * AV_BUFFERSRC_FLAG_KEEP_REF flag.
>>> >>   */
>>> >> +av_warn_unused_result
>>> >>  int av_buffersrc_add_frame(AVFilterContext *ctx, AVFrame *frame);
>>> >>
>>> >>  /**
>>> >> @@ -115,6 +117,7 @@ int av_buffersrc_add_frame(AVFilterContext *ctx, 
>>> >> AVFrame *frame);
>>> >>   * @return>= 0 in case of success, a negative AVERROR code
>>> >>   *in case of failure
>>> >>   */
>>> >> +av_warn_unused_result
>>> >>  int av_buffersrc_add_frame_flags(AVFilterContext *buffer_src,
>>> >>   AVFrame *frame, int flags);
>>> >>
>>> >
>>> > Aren't you just supposed to (void)-prefix the call in the caller when you
>>> > explicitly don't care about the result?
>>> >
>>> > These functions certainly looks like you actually want to check for the
>>> > result most of the time.
>>>
>>> Exactly - this addition to the declaration in the header will trigger
>>> a warning whenever this function is used without obtaining the return
>>> value.
>>>
>>
>> Oh, my bad, I misunderstood, sounds indeed saner than what I had in mind.
>>
>> Thanks for the clarification.
>
> By the way, I highly encourage all developers to slowly start adding
> this to relevant headers, at least in the things they respectively
> maintain. For instance, a bunch of possible bugs/robustness issues
> will be fixed. Here is one I noticed today: by applying to
> init_get_bits8, a few warnings get triggered. There have been commits
> from Michael addressing some CID's related to this - this will ensure
> a complete weeding out of that particular issue.

ping

>
>>
>> --
>> Clément B.
>>
>> ___
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCHv2] avfilter/buffersrc: add av_warn_unused_result attributes

2015-10-07 Thread Ganesh Ajjanagadde
On Wed, Oct 7, 2015 at 10:01 AM, Clément Bœsch  wrote:
> On Wed, Oct 07, 2015 at 09:31:49AM -0400, Ganesh Ajjanagadde wrote:
>> On Wed, Oct 7, 2015 at 8:59 AM, Clément Bœsch  wrote:
>> > On Tue, Oct 06, 2015 at 06:53:47PM -0400, Ganesh Ajjanagadde wrote:
>> >> This adds av_warn_unused_result whenever it is relevant.
>> >>
>> >> Signed-off-by: Ganesh Ajjanagadde 
>> >> ---
>> >>  libavfilter/buffersrc.h | 3 +++
>> >>  1 file changed, 3 insertions(+)
>> >>
>> >> diff --git a/libavfilter/buffersrc.h b/libavfilter/buffersrc.h
>> >> index cd3d95f..847c093 100644
>> >> --- a/libavfilter/buffersrc.h
>> >> +++ b/libavfilter/buffersrc.h
>> >> @@ -78,6 +78,7 @@ unsigned 
>> >> av_buffersrc_get_nb_failed_requests(AVFilterContext *buffer_src);
>> >>   * This function is equivalent to av_buffersrc_add_frame_flags() with the
>> >>   * AV_BUFFERSRC_FLAG_KEEP_REF flag.
>> >>   */
>> >> +av_warn_unused_result
>> >>  int av_buffersrc_write_frame(AVFilterContext *ctx, const AVFrame *frame);
>> >>
>> >>  /**
>> >> @@ -98,6 +99,7 @@ int av_buffersrc_write_frame(AVFilterContext *ctx, 
>> >> const AVFrame *frame);
>> >>   * This function is equivalent to av_buffersrc_add_frame_flags() without 
>> >> the
>> >>   * AV_BUFFERSRC_FLAG_KEEP_REF flag.
>> >>   */
>> >> +av_warn_unused_result
>> >>  int av_buffersrc_add_frame(AVFilterContext *ctx, AVFrame *frame);
>> >>
>> >>  /**
>> >> @@ -115,6 +117,7 @@ int av_buffersrc_add_frame(AVFilterContext *ctx, 
>> >> AVFrame *frame);
>> >>   * @return>= 0 in case of success, a negative AVERROR code
>> >>   *in case of failure
>> >>   */
>> >> +av_warn_unused_result
>> >>  int av_buffersrc_add_frame_flags(AVFilterContext *buffer_src,
>> >>   AVFrame *frame, int flags);
>> >>
>> >
>> > Aren't you just supposed to (void)-prefix the call in the caller when you
>> > explicitly don't care about the result?
>> >
>> > These functions certainly looks like you actually want to check for the
>> > result most of the time.
>>
>> Exactly - this addition to the declaration in the header will trigger
>> a warning whenever this function is used without obtaining the return
>> value.
>>
>
> Oh, my bad, I misunderstood, sounds indeed saner than what I had in mind.
>
> Thanks for the clarification.

By the way, I highly encourage all developers to slowly start adding
this to relevant headers, at least in the things they respectively
maintain. For instance, a bunch of possible bugs/robustness issues
will be fixed. Here is one I noticed today: by applying to
init_get_bits8, a few warnings get triggered. There have been commits
from Michael addressing some CID's related to this - this will ensure
a complete weeding out of that particular issue.

>
> --
> Clément B.
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCHv2] avfilter/buffersrc: add av_warn_unused_result attributes

2015-10-07 Thread Clément Bœsch
On Wed, Oct 07, 2015 at 09:31:49AM -0400, Ganesh Ajjanagadde wrote:
> On Wed, Oct 7, 2015 at 8:59 AM, Clément Bœsch  wrote:
> > On Tue, Oct 06, 2015 at 06:53:47PM -0400, Ganesh Ajjanagadde wrote:
> >> This adds av_warn_unused_result whenever it is relevant.
> >>
> >> Signed-off-by: Ganesh Ajjanagadde 
> >> ---
> >>  libavfilter/buffersrc.h | 3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >> diff --git a/libavfilter/buffersrc.h b/libavfilter/buffersrc.h
> >> index cd3d95f..847c093 100644
> >> --- a/libavfilter/buffersrc.h
> >> +++ b/libavfilter/buffersrc.h
> >> @@ -78,6 +78,7 @@ unsigned 
> >> av_buffersrc_get_nb_failed_requests(AVFilterContext *buffer_src);
> >>   * This function is equivalent to av_buffersrc_add_frame_flags() with the
> >>   * AV_BUFFERSRC_FLAG_KEEP_REF flag.
> >>   */
> >> +av_warn_unused_result
> >>  int av_buffersrc_write_frame(AVFilterContext *ctx, const AVFrame *frame);
> >>
> >>  /**
> >> @@ -98,6 +99,7 @@ int av_buffersrc_write_frame(AVFilterContext *ctx, const 
> >> AVFrame *frame);
> >>   * This function is equivalent to av_buffersrc_add_frame_flags() without 
> >> the
> >>   * AV_BUFFERSRC_FLAG_KEEP_REF flag.
> >>   */
> >> +av_warn_unused_result
> >>  int av_buffersrc_add_frame(AVFilterContext *ctx, AVFrame *frame);
> >>
> >>  /**
> >> @@ -115,6 +117,7 @@ int av_buffersrc_add_frame(AVFilterContext *ctx, 
> >> AVFrame *frame);
> >>   * @return>= 0 in case of success, a negative AVERROR code
> >>   *in case of failure
> >>   */
> >> +av_warn_unused_result
> >>  int av_buffersrc_add_frame_flags(AVFilterContext *buffer_src,
> >>   AVFrame *frame, int flags);
> >>
> >
> > Aren't you just supposed to (void)-prefix the call in the caller when you
> > explicitly don't care about the result?
> >
> > These functions certainly looks like you actually want to check for the
> > result most of the time.
> 
> Exactly - this addition to the declaration in the header will trigger
> a warning whenever this function is used without obtaining the return
> value.
> 

Oh, my bad, I misunderstood, sounds indeed saner than what I had in mind.

Thanks for the clarification.

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCHv2] avfilter/buffersrc: add av_warn_unused_result attributes

2015-10-07 Thread Ganesh Ajjanagadde
On Wed, Oct 7, 2015 at 8:59 AM, Clément Bœsch  wrote:
> On Tue, Oct 06, 2015 at 06:53:47PM -0400, Ganesh Ajjanagadde wrote:
>> This adds av_warn_unused_result whenever it is relevant.
>>
>> Signed-off-by: Ganesh Ajjanagadde 
>> ---
>>  libavfilter/buffersrc.h | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/libavfilter/buffersrc.h b/libavfilter/buffersrc.h
>> index cd3d95f..847c093 100644
>> --- a/libavfilter/buffersrc.h
>> +++ b/libavfilter/buffersrc.h
>> @@ -78,6 +78,7 @@ unsigned 
>> av_buffersrc_get_nb_failed_requests(AVFilterContext *buffer_src);
>>   * This function is equivalent to av_buffersrc_add_frame_flags() with the
>>   * AV_BUFFERSRC_FLAG_KEEP_REF flag.
>>   */
>> +av_warn_unused_result
>>  int av_buffersrc_write_frame(AVFilterContext *ctx, const AVFrame *frame);
>>
>>  /**
>> @@ -98,6 +99,7 @@ int av_buffersrc_write_frame(AVFilterContext *ctx, const 
>> AVFrame *frame);
>>   * This function is equivalent to av_buffersrc_add_frame_flags() without the
>>   * AV_BUFFERSRC_FLAG_KEEP_REF flag.
>>   */
>> +av_warn_unused_result
>>  int av_buffersrc_add_frame(AVFilterContext *ctx, AVFrame *frame);
>>
>>  /**
>> @@ -115,6 +117,7 @@ int av_buffersrc_add_frame(AVFilterContext *ctx, AVFrame 
>> *frame);
>>   * @return>= 0 in case of success, a negative AVERROR code
>>   *in case of failure
>>   */
>> +av_warn_unused_result
>>  int av_buffersrc_add_frame_flags(AVFilterContext *buffer_src,
>>   AVFrame *frame, int flags);
>>
>
> Aren't you just supposed to (void)-prefix the call in the caller when you
> explicitly don't care about the result?
>
> These functions certainly looks like you actually want to check for the
> result most of the time.

Exactly - this addition to the declaration in the header will trigger
a warning whenever this function is used without obtaining the return
value.

>
> --
> Clément B.
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCHv2] avfilter/buffersrc: add av_warn_unused_result attributes

2015-10-07 Thread Clément Bœsch
On Tue, Oct 06, 2015 at 06:53:47PM -0400, Ganesh Ajjanagadde wrote:
> This adds av_warn_unused_result whenever it is relevant.
> 
> Signed-off-by: Ganesh Ajjanagadde 
> ---
>  libavfilter/buffersrc.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/libavfilter/buffersrc.h b/libavfilter/buffersrc.h
> index cd3d95f..847c093 100644
> --- a/libavfilter/buffersrc.h
> +++ b/libavfilter/buffersrc.h
> @@ -78,6 +78,7 @@ unsigned 
> av_buffersrc_get_nb_failed_requests(AVFilterContext *buffer_src);
>   * This function is equivalent to av_buffersrc_add_frame_flags() with the
>   * AV_BUFFERSRC_FLAG_KEEP_REF flag.
>   */
> +av_warn_unused_result
>  int av_buffersrc_write_frame(AVFilterContext *ctx, const AVFrame *frame);
>  
>  /**
> @@ -98,6 +99,7 @@ int av_buffersrc_write_frame(AVFilterContext *ctx, const 
> AVFrame *frame);
>   * This function is equivalent to av_buffersrc_add_frame_flags() without the
>   * AV_BUFFERSRC_FLAG_KEEP_REF flag.
>   */
> +av_warn_unused_result
>  int av_buffersrc_add_frame(AVFilterContext *ctx, AVFrame *frame);
>  
>  /**
> @@ -115,6 +117,7 @@ int av_buffersrc_add_frame(AVFilterContext *ctx, AVFrame 
> *frame);
>   * @return>= 0 in case of success, a negative AVERROR code
>   *in case of failure
>   */
> +av_warn_unused_result
>  int av_buffersrc_add_frame_flags(AVFilterContext *buffer_src,
>   AVFrame *frame, int flags);
>  

Aren't you just supposed to (void)-prefix the call in the caller when you
explicitly don't care about the result?

These functions certainly looks like you actually want to check for the
result most of the time.

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCHv2] avfilter/buffersrc: add av_warn_unused_result attributes

2015-10-06 Thread Ganesh Ajjanagadde
This adds av_warn_unused_result whenever it is relevant.

Signed-off-by: Ganesh Ajjanagadde 
---
 libavfilter/buffersrc.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libavfilter/buffersrc.h b/libavfilter/buffersrc.h
index cd3d95f..847c093 100644
--- a/libavfilter/buffersrc.h
+++ b/libavfilter/buffersrc.h
@@ -78,6 +78,7 @@ unsigned av_buffersrc_get_nb_failed_requests(AVFilterContext 
*buffer_src);
  * This function is equivalent to av_buffersrc_add_frame_flags() with the
  * AV_BUFFERSRC_FLAG_KEEP_REF flag.
  */
+av_warn_unused_result
 int av_buffersrc_write_frame(AVFilterContext *ctx, const AVFrame *frame);
 
 /**
@@ -98,6 +99,7 @@ int av_buffersrc_write_frame(AVFilterContext *ctx, const 
AVFrame *frame);
  * This function is equivalent to av_buffersrc_add_frame_flags() without the
  * AV_BUFFERSRC_FLAG_KEEP_REF flag.
  */
+av_warn_unused_result
 int av_buffersrc_add_frame(AVFilterContext *ctx, AVFrame *frame);
 
 /**
@@ -115,6 +117,7 @@ int av_buffersrc_add_frame(AVFilterContext *ctx, AVFrame 
*frame);
  * @return>= 0 in case of success, a negative AVERROR code
  *in case of failure
  */
+av_warn_unused_result
 int av_buffersrc_add_frame_flags(AVFilterContext *buffer_src,
  AVFrame *frame, int flags);
 
-- 
2.6.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel