I'm very curious to see the change that introduced the bug in the first place. According to the announcement it was introduced in 1.0.1. That's the version that added Heartbeat support, so maybe it was a bug from the beginning.
Probably to make it more clear what you're referring to, and double-check yourself. There are probably components that are 1 byte, 2 bytes, and 16 bytes long. Writing it out makes it clear and eliminates a chance for human error in the sum, more than a magic 19 does. (I guess 16 is pretty magical too, though. At least it's a "round" number, and in context may be a well-known field size of something in the protocol.)
Yeah, this. It is fairly common to see things like this in C-like languages when it comes to times, like when including the milliseconds in a day you might see:
int timeout = 1000 * 60 * 60 * 24;
(milliseconds in a second * 60 for minute, * 60 for hour, * 24 for hours in a day).
Much more obvious than just putting in 86400000, and the compiler will optimize away the math and putting the math in there explicitly is arguably better than a comment that could easily become unanchored from the real value (if someone changes the value and forgets to update the comment).
When it comes to byte-sizes of things, though, most code will use sizeof() to both make it more clear where these numbers are coming from and to make them automatically adjust if the structure sizes change (granted this is unlikely to happen on a mature protocol).
At the very least having them be preprocessor defines would certainly make things a lot more clear here, so even for C I'd consider this a bit of a "code smell" (though the people who work on this code regularly are probably versed enough in the ssl3 record structure enough that they immediately grok this when they see it).
http://git.openssl.org/gitweb/?p=openssl.git;a=commitdiff;h=...