[Ffmpeg-devel] Re: [PATCH] MXF tag parser

[Rich Felker]

On Wed, Aug 02, 2006 at 03:14:55PM +0200, Baptiste Coudurier wrote:
> Hi
> 
> Reimar D?ffinger wrote:
> > Hello,
> > the attached patch implements a special mxf tag parsing function to
> > avoid code duplication.
> > It also has the side effect of doing more checks and playing my
> > problematic (i.e. non-conformant) files while also showing a warning
> > about the problem.
> > It might be possible to optimize a bit further, esp. some names, but I
> > think it is already quite helpful, esp. for avoiding copy-and-paste when
> > adding support for more stuff.
> > It also changes quite a few types from int to uint32_t, because
> > 1) I think it's more correct
> 
> Isn't "int" always at least 32bit ? If types size are exactly like in
> 377M I'm all for it, can you please seperate that patch from the other ?

Yes, POSIX requires at least 32bit int, and although ISO C still
allows 16bit crap, no such system has any hope of ever running ffmpeg.
Never use int32_t unless you explicitly need exactly 32 bits, like for
accessing 32bit audio samples in an array or something. Using
int_fast32_t may be useful sometimes, but generally int is fine.

> > 2) checks like "if (tmp_u32 >= UINT_MAX / sizeof(UID))" (before
> > malloc(tmp_u32 * sizeof(UID)) ) are not correct for signed types AFAICT.
> 
> Humm, I'm not sure. Problem here is mul overflow. You mean negative
> tmp_u32 ?

With a name like tmp_u32 I hope it's not signed... BTW if it's of type
uint32_t then using UINT_MAX is not quite appropriate. I would always
compare against SIZE_MAX, actually..

Rich