Reinhard Tartler <[email protected]> writes: > 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.
The documentation is weird. That placement is inconsistent with both standard keywords and existing code. Please move it. > Does anyone know a good way to test if this still works? Of course it works. We do that all over the place. >>> /** >>> * 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? 2.5 is so old it barely even existed. Nobody ever talks about anything before 2.95, so I though perhaps it was a typo for that. >>> +# 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. So rather than consider what you're doing, you prefer to charge ahead and cargo-cult everything you see, sensible or not? Sorry, the consistency argument doesn't fly here. -- Måns Rullgård [email protected] _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
