[vlc-devel] [vlc-commits] update: use U32_AT

Rémi Denis-Courmont remi at remlab.net
Tue Nov 18 13:21:26 CET 2014


Le 2014-11-18 13:50, Rafaël Carré a écrit :
>> Indeed the original code is undefined while the new code is defined 
>> from ISO
>> C point of view (possibly not compatible with OpenPGP though).
>
> Our users don't care about ISO C.

Some users do. And some users also care about the off-by-one read 
overflow.

>> >so obviously untested.
>>
>> Indeed I do not usually test that code remains undefined, since I 
>> rarely
>> strive for undefined code.
>
> You should have read the spec for that code before changing it

I doubt that the specification would have helped me determine which of 
the buffer length predicate and the buffer read was off-by-one and which 
was right. I was fixing the integer overflow, not the off-by-one which I 
had not even noticed.

It is obvious that I did not test this beyond compilation. But it is 
also obvious that this code is poorly tested and poorly maintained - as 
is much of VLC due to lack of resources and automated test cases (I do 
not need to remind you that SRTP has unit tests to try to avoid this 
kind of problems).

-- 
Rémi Denis-Courmont



More information about the vlc-devel mailing list