[1/2] Add av_format_printf, for marking printf style format strings and their parameters
diff mbox

Message ID 1308644023-93606-1-git-send-email-martin@martin.st
State Superseded
Headers show

Commit Message

Martin Storsjö June 21, 2011, 8:13 a.m. 
---
 doc/APIchanges         |    3 +++
 libavutil/attributes.h |    2 ++
 libavutil/avutil.h     |    2 +-
 3 files changed, 6 insertions(+), 1 deletions(-)

Comments

Mans Rullgard June 21, 2011, 8:15 a.m. | #1 
Martin Storsjö <martin@martin.st> writes:

> ---
>  doc/APIchanges         |    3 +++
>  libavutil/attributes.h |    2 ++
>  libavutil/avutil.h     |    2 +-
>  3 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 79d9749..77dc510 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -13,6 +13,9 @@ libavutil:   2011-04-18
>  
>  API changes, most recent first:
>  
> +2011-06-xx - xxxxxxx - lavu 51.8.0 - attributes.h
> +  Add av_format_printf().
> +
>  2011-06-xx - xxxxxxx - lavf 53.2.0 - avformat.h
>    Add avformat_open_input and avformat_write_header().
>    Deprecate av_open_input_stream, av_open_input_file,
> diff --git a/libavutil/attributes.h b/libavutil/attributes.h
> index 4abe78f..d64f907 100644
> --- a/libavutil/attributes.h
> +++ b/libavutil/attributes.h
> @@ -127,8 +127,10 @@
>  
>  #ifdef __GNUC__
>  #    define av_builtin_constant_p __builtin_constant_p
> +#    define av_format_printf(fmtpos, attrpos) __attribute__((__format__(__printf__, fmtpos, attrpos)))
>  #else
>  #    define av_builtin_constant_p(x) 0
> +#    define av_format_printf(fmtpos, attrpos)
>  #endif

Is this available in all gcc versions?
Martin Storsjö June 21, 2011, 8:17 a.m. | #2 
On Tue, 21 Jun 2011, Måns Rullgård wrote:

> Martin Storsjö <martin@martin.st> writes:
> 
> > ---
> >  doc/APIchanges         |    3 +++
> >  libavutil/attributes.h |    2 ++
> >  libavutil/avutil.h     |    2 +-
> >  3 files changed, 6 insertions(+), 1 deletions(-)
> >
> > diff --git a/doc/APIchanges b/doc/APIchanges
> > index 79d9749..77dc510 100644
> > --- a/doc/APIchanges
> > +++ b/doc/APIchanges
> > @@ -13,6 +13,9 @@ libavutil:   2011-04-18
> >  
> >  API changes, most recent first:
> >  
> > +2011-06-xx - xxxxxxx - lavu 51.8.0 - attributes.h
> > +  Add av_format_printf().
> > +
> >  2011-06-xx - xxxxxxx - lavf 53.2.0 - avformat.h
> >    Add avformat_open_input and avformat_write_header().
> >    Deprecate av_open_input_stream, av_open_input_file,
> > diff --git a/libavutil/attributes.h b/libavutil/attributes.h
> > index 4abe78f..d64f907 100644
> > --- a/libavutil/attributes.h
> > +++ b/libavutil/attributes.h
> > @@ -127,8 +127,10 @@
> >  
> >  #ifdef __GNUC__
> >  #    define av_builtin_constant_p __builtin_constant_p
> > +#    define av_format_printf(fmtpos, attrpos) __attribute__((__format__(__printf__, fmtpos, attrpos)))
> >  #else
> >  #    define av_builtin_constant_p(x) 0
> > +#    define av_format_printf(fmtpos, attrpos)
> >  #endif
> 
> Is this available in all gcc versions?

As far as I know, yes - we've at least been using it within #ifdef 
__GNUC__ on av_log before this.

// Martin
Mans Rullgard June 21, 2011, 11:37 a.m. | #3 
Martin Storsjö <martin@martin.st> writes:

> On Tue, 21 Jun 2011, Måns Rullgård wrote:
>
>> Martin Storsjö <martin@martin.st> writes:
>> 
>> > ---
>> >  doc/APIchanges         |    3 +++
>> >  libavutil/attributes.h |    2 ++
>> >  libavutil/avutil.h     |    2 +-
>> >  3 files changed, 6 insertions(+), 1 deletions(-)
>> >
>> > diff --git a/doc/APIchanges b/doc/APIchanges
>> > index 79d9749..77dc510 100644
>> > --- a/doc/APIchanges
>> > +++ b/doc/APIchanges
>> > @@ -13,6 +13,9 @@ libavutil:   2011-04-18
>> >  
>> >  API changes, most recent first:
>> >  
>> > +2011-06-xx - xxxxxxx - lavu 51.8.0 - attributes.h
>> > +  Add av_format_printf().
>> > +
>> >  2011-06-xx - xxxxxxx - lavf 53.2.0 - avformat.h
>> >    Add avformat_open_input and avformat_write_header().
>> >    Deprecate av_open_input_stream, av_open_input_file,
>> > diff --git a/libavutil/attributes.h b/libavutil/attributes.h
>> > index 4abe78f..d64f907 100644
>> > --- a/libavutil/attributes.h
>> > +++ b/libavutil/attributes.h
>> > @@ -127,8 +127,10 @@
>> >  
>> >  #ifdef __GNUC__
>> >  #    define av_builtin_constant_p __builtin_constant_p
>> > +#    define av_format_printf(fmtpos, attrpos) __attribute__((__format__(__printf__, fmtpos, attrpos)))
>> >  #else
>> >  #    define av_builtin_constant_p(x) 0
>> > +#    define av_format_printf(fmtpos, attrpos)
>> >  #endif
>> 
>> Is this available in all gcc versions?
>
> As far as I know, yes - we've at least been using it within #ifdef 
> __GNUC__ on av_log before this.

Patch OK then.
Alex Converse June 21, 2011, 5:20 p.m. | #4 
On Tue, Jun 21, 2011 at 1:13 AM, Martin Storsjö <martin@martin.st> wrote:
> ---
>  doc/APIchanges         |    3 +++
>  libavutil/attributes.h |    2 ++
>  libavutil/avutil.h     |    2 +-
>  3 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 79d9749..77dc510 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -13,6 +13,9 @@ libavutil:   2011-04-18
>
>  API changes, most recent first:
>
> +2011-06-xx - xxxxxxx - lavu 51.8.0 - attributes.h
> +  Add av_format_printf().
> +

maybe this is too nitpicky but I think the name is misleading (based
on the name I would guess this is part of lavf). Perhaps
av_printf_format would be better?
Justin Ruggles June 21, 2011, 7:01 p.m. | #5 
On 06/21/2011 01:20 PM, Alex Converse wrote:

> On Tue, Jun 21, 2011 at 1:13 AM, Martin Storsjö <martin@martin.st> wrote:
>> ---
>>  doc/APIchanges         |    3 +++
>>  libavutil/attributes.h |    2 ++
>>  libavutil/avutil.h     |    2 +-
>>  3 files changed, 6 insertions(+), 1 deletions(-)
>>
>> diff --git a/doc/APIchanges b/doc/APIchanges
>> index 79d9749..77dc510 100644
>> --- a/doc/APIchanges
>> +++ b/doc/APIchanges
>> @@ -13,6 +13,9 @@ libavutil:   2011-04-18
>>
>>  API changes, most recent first:
>>
>> +2011-06-xx - xxxxxxx - lavu 51.8.0 - attributes.h
>> +  Add av_format_printf().
>> +
> 
> maybe this is too nitpicky but I think the name is misleading (based
> on the name I would guess this is part of lavf). Perhaps
> av_printf_format would be better?


+1

-Justin
Mans Rullgard June 21, 2011, 7:39 p.m. | #6 
Alex Converse <alex.converse@gmail.com> writes:

> On Tue, Jun 21, 2011 at 1:13 AM, Martin Storsjö <martin@martin.st> wrote:
>> ---
>>  doc/APIchanges         |    3 +++
>>  libavutil/attributes.h |    2 ++
>>  libavutil/avutil.h     |    2 +-
>>  3 files changed, 6 insertions(+), 1 deletions(-)
>>
>> diff --git a/doc/APIchanges b/doc/APIchanges
>> index 79d9749..77dc510 100644
>> --- a/doc/APIchanges
>> +++ b/doc/APIchanges
>> @@ -13,6 +13,9 @@ libavutil:   2011-04-18
>>
>>  API changes, most recent first:
>>
>> +2011-06-xx - xxxxxxx - lavu 51.8.0 - attributes.h
>> +  Add av_format_printf().
>> +
>
> maybe this is too nitpicky but I think the name is misleading (based
> on the name I would guess this is part of lavf). Perhaps
> av_printf_format would be better?

I don't care about the name, but keeping it with the other attribute
stuff makes sense to me.
Martin Storsjö June 21, 2011, 7:41 p.m. | #7 
On Tue, 21 Jun 2011, Alex Converse wrote:

> On Tue, Jun 21, 2011 at 1:13 AM, Martin Storsjö <martin@martin.st> wrote:
> > ---
> >  doc/APIchanges         |    3 +++
> >  libavutil/attributes.h |    2 ++
> >  libavutil/avutil.h     |    2 +-
> >  3 files changed, 6 insertions(+), 1 deletions(-)
> >
> > diff --git a/doc/APIchanges b/doc/APIchanges
> > index 79d9749..77dc510 100644
> > --- a/doc/APIchanges
> > +++ b/doc/APIchanges
> > @@ -13,6 +13,9 @@ libavutil:   2011-04-18
> >
> >  API changes, most recent first:
> >
> > +2011-06-xx - xxxxxxx - lavu 51.8.0 - attributes.h
> > +  Add av_format_printf().
> > +
> 
> maybe this is too nitpicky but I think the name is misleading (based
> on the name I would guess this is part of lavf). Perhaps
> av_printf_format would be better?

Good point, I wasn't totally satisfied with the name either - 
av_printf_format is better.

// Martin

Patch
diff mbox 

diff --git a/doc/APIchanges b/doc/APIchanges
index 79d9749..77dc510 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -13,6 +13,9 @@  libavutil:   2011-04-18
 
 API changes, most recent first:
 
+2011-06-xx - xxxxxxx - lavu 51.8.0 - attributes.h
+  Add av_format_printf().
+
 2011-06-xx - xxxxxxx - lavf 53.2.0 - avformat.h
   Add avformat_open_input and avformat_write_header().
   Deprecate av_open_input_stream, av_open_input_file,
diff --git a/libavutil/attributes.h b/libavutil/attributes.h
index 4abe78f..d64f907 100644
--- a/libavutil/attributes.h
+++ b/libavutil/attributes.h
@@ -127,8 +127,10 @@ 
 
 #ifdef __GNUC__
 #    define av_builtin_constant_p __builtin_constant_p
+#    define av_format_printf(fmtpos, attrpos) __attribute__((__format__(__printf__, fmtpos, attrpos)))
 #else
 #    define av_builtin_constant_p(x) 0
+#    define av_format_printf(fmtpos, attrpos)
 #endif
 
 #endif /* AVUTIL_ATTRIBUTES_H */
diff --git a/libavutil/avutil.h b/libavutil/avutil.h
index 1846146..d6e4668 100644
--- a/libavutil/avutil.h
+++ b/libavutil/avutil.h
@@ -40,7 +40,7 @@ 
 #define AV_VERSION(a, b, c) AV_VERSION_DOT(a, b, c)
 
 #define LIBAVUTIL_VERSION_MAJOR 51
-#define LIBAVUTIL_VERSION_MINOR  7
+#define LIBAVUTIL_VERSION_MINOR  8
 #define LIBAVUTIL_VERSION_MICRO  0
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \