Janne Grunau <[email protected]> writes: > On 2012-11-29 13:46:12 +0000, Måns Rullgård wrote: >> Janne Grunau <[email protected]> writes: >> >> > On 2012-11-29 00:08:52 +0000, Måns Rullgård wrote: >> >> Janne Grunau <[email protected]> writes: >> >> >> >> > On 2012-11-28 23:52:27 +0100, Luca Barbato wrote: >> >> >> >> >> >> Is INVALID_VLC value negative? >> >> > >> >> > no, 0x80000000. But arithmetic conversion saves us. >> >> >> >> Ouch, that's _really_ bad. The svq3_get_ue_golomb() return type is >> >> (inexplicably) int, so returning that value entails a conversion with >> >> implementation-defined behaviour. Most compilers leave the bits intact >> >> in such conversions, but I'd rather not depend on it. Also, such code >> >> is nothing short of obfuscated even if it does work reliably. >> > >> > 6.3.1.3 reads to me as if signed int to unsigned int conversion is well >> > defined: >> >> Yes, but we're dealing with unsigned to signed here. The integer >> constant 0x80000000 has type unsigned int (if int is 32-bit). Returning >> this from svq3_get_ue_golomb() as a signed int invokes an >> implementation-defined conversion. > > svq3_get_ue_golomb() doesn't return INVALID_VLC explicitly.
Right, I confused it with svq3_get_se_golomb(). svq3_get_ue_golomb() is actually worse. > The only it can return something negative is due to signed arithmetic > on the int ret in the else branch. The function has two return statements. The first returns a value from ff_interleaved_ue_golomb_vlc_code[] (array of uint8_t), so this one is safe. The other returns "ret - 1" where ret is a signed int. For this to produce a negative value other than -1, ret must itself be negative. This can only happen through the left shifts in the loop overflowing into the sign bit. Such an overflow has *undefined* behaviour. If any input can cause this to happen (and your patch suggests this is the case), the code is broken and must be fixed here. Checking it after the fact is not good enough. -- Måns Rullgård [email protected] _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
