[FFmpeg-devel] [PATCH] Store Major brand, Minor version and compatible brands of a mov file using the metadata API

[Reimar Döffinger]

On Wed, Sep 09, 2009 at 02:00:47PM +0300, haim alon wrote:
> > No, at least av_strlcpy reads at most "size-1" bytes, 4 in this case.
> > The problem is with the return value of it:
> > return len + strlen(src) - 1;
> > So my suggestion to replace strncpy with av_strlcpy actually completely
> > broke things, (av_)strlcpy can only be used for correctly 0-terminated
> > strings, and I fear that this is used wrongly in some places in
> > FFmpeg, e.g. rtmppkt.c looks suspicious.
> >
> >
> 
> Why there is a problem with the return value?
> In this case, it copies 4 characters from the source uint32_t and then add
> NULL terminator.

And then it will call strlen on the input value to calculate the return
value and overread until it either hits a 0 by chance or crashes.

> By the way, I've tried using AV_WB32 instead but it swapped the characters.
> Maybe AV_WL32 should be used.

Since get_le32 was used, yes it should be AV_WL32.
But the log message will still print it in the wrong order on big-endian
systems.
IMO the whole casting mess should be removed and type would better be
uint8_t type[5] ={0};
get_buffer(pb, type, 4);

> +        // check that char is legal - not NULL
> +        if (next_comp_brand_ptr[0] && next_comp_brand_ptr[1] && next_comp_brand_ptr[2] && next_comp_brand_ptr[3]) {

Not sure if this should be checked more tightly, e.g. for ASCII, or for
printable or...