чт, 27 июн. 2019 г. в 15:33, Lukas Tribus <[email protected]>: > Hello, > > On Thu, 27 Jun 2019 at 08:43, Илья Шипицин <[email protected]> wrote: > >> However this commit also changes error handling with OpenSSL: > >> > >> > https://github.com/haproxy/haproxy/commit/54832b97c65c43ee50b6661a9b4b05885427b5e9#diff-cc9b3f376afbc81cf478eeac17839172L5533 > >> > >> > >> Can you clarify if the OpenSSL change was intentional and if yes, > elaborate? > > > > > > it was intentional. libressl does not support "packet_size" there. > > however, openssl is fine with the above way > > Then I have to criticize that commit message. >
you are right, commit messages is not my best. sorry about that anyway, if you think it is wrong, you are welcome to provide a fix (sometimes, I do wrong things, I'm ok with that) > > It fixes the build, but it changes a function call, so it's more than > just a build fix. It's also not affecting only the LibreSSL code path, > but the OpenSSL code path also. > A single commit should do one thing only (as opposed to a number of > things) and reflect the changes it does in both title and message. > > "BUILD: enable several LibreSSL hacks, including" when touching > OpenSSL code paths is not enough. This should have been 1) a separate > commit and 2) have a descriptive message like: > > "ssl: change error handling on empty handshake > > Also fixes the build with LibreSSL" > > Because that's what the change does: a different error handling for > empty handshakes, which fixes the build with LibreSSL but the change > affects (affects in that in changes the code) OpenSSL as well. > > The change breaks empty handshake detection in OpenSSL < 1.1.0. We > need to revert this and put the LibreSSL fix in a different #if > branch. > > > > however, openssl is fine with the above way > > That assumption is wrong, but that's not the point. The point is that > we need to do a better job of making one change only per commit and > have title and commit message reflect the actual change and intention, > not the assumed impact. > > > thanks, > lukas >
