[FFmpeg-devel] [PATCH v6] Fix integer parameters size check in SDP fmtp line

Reimar Döffinger Reimar.Doeffinger at gmx.de
Wed Jul 3 18:23:36 EEST 2019


On 03.07.2019, at 09:28, Olivier MAIGNIAL <olivier.maignial at smile.fr> wrote:

> Hello, thanks for review,
> 
> 1) Can you give some reason about why we shouldn't use errno? I think it is
> more clear to accept the full int32 range, even if min/max int32 values are
> very unlikely to be used.

It is a global variable, with all the issues that has, also for code clarity (to review code you need to know which functions modify it).
On some systems it is not even thread-local, so using it becomes completely unsafe once threads are involved, but even when it is, signal handlers can by accident change it.
I see the argument that these are rare, special and broken cases, but it's not like using errno gains all that much (esp. when using stroll is also an alternative that avoids the need).
The shorter version of that argument is that errno is a major misdesign beyond even strcpy and strncpy levels, which I personally find good reason to avoid it where reasonable.
Note that there is also the issue that "long" is 32 bit on 32 bit systems, I would expect the if condition to trigger compiler warnings there, which strtoll should avoid.

> 2) The RFC 4566 don't give any limit on fmtp parameters values. In addition
> I can't find a spec that says fmtp integers parameters for AAC must be
> positive. So I don't think we should limit values to positive integers here
> as it would not be consistent to RFC.

I admit I have not found any requirement on it being integer at all, so not sure if that assumption is not wrong to start with and should at least not trigger an error.
And nothing that would make > 32 bit wrong.


More information about the ffmpeg-devel mailing list