[FFmpeg-cvslog] r16452 - trunk/libavformat/metadata.c

[Aurelien Jacobs]

M?ns Rullg?rd wrote:

> 
> Aurelien Jacobs wrote:
> > On Tue, 6 Jan 2009 15:52:54 +0100
> > Reimar D?ffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de> wrote:
> >
> >> On Tue, Jan 06, 2009 at 01:56:21PM +0100, aurel wrote:
> >> > Modified: trunk/libavformat/metadata.c
> >> > ==============================================================================
> >> > --- trunk/libavformat/metadata.c	Tue Jan  6 13:51:35 2009	(r16451)
> >> > +++ trunk/libavformat/metadata.c	Tue Jan  6 13:56:21 2009	(r16452)
> >> > @@ -84,7 +84,7 @@ int av_metadata_set(AVMetadata **pm, AVM
> >> >  #define FILL_METADATA_INT(s, key) {                                      \
> >> >      char number[10];                                                     \
> >> >      snprintf(number, sizeof(number), "%d", s->key);                      \
> 
> Unrelated to this patch, but that buffer is too small.  A signed 32-bit
> number needs up to 11 characters in decimal (minus sign and 10 digits).

Hopefully this is only used to convert year and track number, so
the buffer is more than enough for any real world file.
And snprintf() protect us from malicious files.
So I see no problem here (especially for a temporary compatibility
layer).

> >> > -    FILL_METADATA(s, key, number) }
> >> > +    if(s->key)  FILL_METADATA(s, key, number) }
> >>
> >> Why two spaces?
> >
> > Why not...
> > When I write a statement on the same line as a if(),
> 
> Why do you do that?

I do that because it is more readable (at least to my eyes, and
only in a few places).
But once again, feel free to change this to whatever you want.
I don't care much about cosmetics regarding this temporary code.

Aurel