[vlc-devel] [PATCH 7/7] config: simplify keycmp

Rémi Denis-Courmont remi at remlab.net
Tue May 6 23:38:08 CEST 2014


Le mardi 6 mai 2014, 23:28:04 Felix Abecassis a écrit :
> 2014-05-06 22:59 GMT+02:00 Rémi Denis-Courmont <remi at remlab.net>:
> > Le mardi 6 mai 2014, 22:37:44 Felix Abecassis a écrit :
> >> Do we really need speed here?
> >> 
> >> It avoids an conversion to int after an unsigned overflow.
> > 
> > I believe we depend on the reasonable/GCC modulo behaviour in many paths.
> 
> I thought so too, but I've used -fsanitize=integer using clang and
> only had a few (4-8) undefined behaviors reported for each run with
> audio and subtitles.

The case of arithmetic integer overflow or underflow is a bug. GCC will also 
complain with the sanitizer.

The case of conversion from unsigned is more dubious, I would argue.

> > Sometimes it is even more or less enforced by the underlying libraries.
> > For instance, iconv() returns (size_t)-1 on error.
> 
> This looks like a different situation since iconv returns size_t, the
> result of the conversion is well defined and I think this is actually
> a portable way to set all bits to 1.

Then take RTP as an example that definitely does it the "wrong" way. I would 
also suspect some sanity checks do this, but only happen in corner cases.

Thing is, we cannot rely on run-time sanity tests here; there are just far too 
many code paths involved. To me, 4-8 failures on a smoke tests seems like a 
lot. Without compile time analysis, I doubt we can really fix this :-(

-- 
Rémi Denis-Courmont
http://www.remlab.net/




More information about the vlc-devel mailing list