3 ms·

Where's the commit that introduced the extra goto? I haven't seen it yet.

0x0 13y ago
As outsiders we can't see commits, but the next best thing is a diff between the public source code releases: https://gist.github.com/alexyakoubian/9151610/revisions https://gist.github.com/alexyakoubian/9151610/revisions
- thirsteh 13y ago
  Line 631 in the second column for anyone wondering.
- MBCook 13y ago
  It could have been merge between public releases. My money would be on a copy & paste error, but there is no more evidence for that than the merge error theory.
- thrownaway2424 13y ago
  Pretty yak-shavy. Looks like some symbols were renamed and many of the APIs were (probably pointlessly) mutated. In total, unreviewable. But as you say, this may not have been the internal diff seen by someone at Apple. They may have seen individual, focussed patches that did single things like changing from the 2-argument to the 1-argument SSLFreeBuffer, another patch for changing selectedCipherSpec to selectedCipherSpecParams (hello, yak shaving), renaming noErr to errSecSuccess, and so forth. The question is, in any of those changes, the extra "goto fail;" line would have stuck out as irrelevant. In a gigantic 500-line delta the reviewer's eyes may have been glazed over by the time they got to that one. Assuming anyone reviews code at Apple, of course.
  - 0x0 13y ago
    I'm shocked and find that hard to believe; even without separate commits, it's not a huge single diff and it should really not be too much to ask for a proper review. You'd think one would be a little less sloppy when hacking on the very core of the SSL libraries being deployed on hundreds of million of devices.
    - tptacek 13y ago
      You'd think one would be a little less sloppy when hacking on the very core of the SSL libraries being deployed on hundreds of million of devices Hahahahahaha.
- dandelany 13y ago
  > As outsiders we can't see commits Exactly. I was going to refute your point but you did it for me.

0x0 13y ago

As outsiders we can't see commits, but the next best thing is a diff between the public source code releases: https://gist.github.com/alexyakoubian/9151610/revisions https://gist.github.com/alexyakoubian/9151610/revisions

thirsteh 13y ago
Line 631 in the second column for anyone wondering.
MBCook 13y ago
It could have been merge between public releases. My money would be on a copy & paste error, but there is no more evidence for that than the merge error theory.
thrownaway2424 13y ago
Pretty yak-shavy. Looks like some symbols were renamed and many of the APIs were (probably pointlessly) mutated. In total, unreviewable. But as you say, this may not have been the internal diff seen by someone at Apple. They may have seen individual, focussed patches that did single things like changing from the 2-argument to the 1-argument SSLFreeBuffer, another patch for changing selectedCipherSpec to selectedCipherSpecParams (hello, yak shaving), renaming noErr to errSecSuccess, and so forth. The question is, in any of those changes, the extra "goto fail;" line would have stuck out as irrelevant. In a gigantic 500-line delta the reviewer's eyes may have been glazed over by the time they got to that one. Assuming anyone reviews code at Apple, of course.
- 0x0 13y ago
  I'm shocked and find that hard to believe; even without separate commits, it's not a huge single diff and it should really not be too much to ask for a proper review. You'd think one would be a little less sloppy when hacking on the very core of the SSL libraries being deployed on hundreds of million of devices.
  - tptacek 13y ago
    You'd think one would be a little less sloppy when hacking on the very core of the SSL libraries being deployed on hundreds of million of devices Hahahahahaha.
dandelany 13y ago
> As outsiders we can't see commits Exactly. I was going to refute your point but you did it for me.