On Sun, Jul 1, 2012 at 7:48 PM, Måns Rullgård <[email protected]> wrote:
> Reinhard Tartler <[email protected]> writes:
>
>> Also use it in the declaration of the various exit_program
>> implementations in avtools.
>>
>> inspired by a clang-scan report.
>> ---
>>  cmdutils.h             |    2 +-
>>  libavutil/attributes.h |    8 ++++++++
>>  2 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/cmdutils.h b/cmdutils.h
>> index 793a1e8..4a62298 100644
>> --- a/cmdutils.h
>> +++ b/cmdutils.h
>> @@ -373,7 +373,7 @@ FILE *get_preset_file(char *filename, size_t 
>> filename_size,
>>   * Do all the necessary cleanup and abort.
>>   * This function is implemented in the avtools, not cmdutils.
>>   */
>> -void exit_program(int ret);
>> +void exit_program(int ret) av_noreturn;
>
> We usually put attributes at the start of function declarations
> alongside static, const, etc.

the gcc documentation gives all function attributes at the end of
function declarations, that's why I put it at the end.

Does anyone know a good way to test if this still works?

>
>>  /**
>>   * Realloc array to hold new_size elements of elem_size.
>> diff --git a/libavutil/attributes.h b/libavutil/attributes.h
>> index c0bac3f..bf831b4 100644
>> --- a/libavutil/attributes.h
>> +++ b/libavutil/attributes.h
>> @@ -135,4 +135,12 @@
>>  #    define av_printf_format(fmtpos, attrpos)
>>  #endif
>>
>> +#ifndef av_noreturn
>> +#if AV_GCC_VERSION_AT_LEAST(2,5)
>
> That version doesn't look right.

According to 
http://gcc.gnu.org/onlinedocs/gcc-4.7.1/gcc/Function-Attributes.html#Function-Attributes,
2.5 seems correct. Or did I misunderstand you here?

>
>> +#    define av_noreturn __attribute__((noreturn))
>> +#else
>> +#    define av_noreturn
>> +#endif
>> +#endif
>
> Why almost all of these under #ifndef av_foo?  Is there ever a
> legitimate reason to define them to something else outside this file?

I don't think so, but that really seems like a seperate change. The
proposed patch keeps consistency with the rest of the file.

-- 
regards,
    Reinhard
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to