Github user jablko commented on a diff in the pull request:

    https://github.com/apache/trafficserver/pull/1438#discussion_r100697717
  
    --- Diff: iocore/net/BIO_fastopen.cc ---
    @@ -27,6 +27,67 @@
     
     #include "BIO_fastopen.h"
     
    +// For BoringSSL, which for some reason doesn't have this function.
    +// (In BoringSSL, sock_read() and sock_write() use the internal
    +// bio_fd_non_fatal_error() instead.) #1437
    +//
    +// The following is copied from
    +// 
https://github.com/openssl/openssl/blob/3befffa39dbaf2688d823fcf2bdfc07d2487be48/crypto/bio/bss_sock.c
    --- End diff --
    
    That works ... Is the purpose to avoid having to attribute it?
    
    In general, I like to code for OpenSSL, and add fixes to make other 
environments conform to it -- unless there's a reason not to. So in this case 
I'd continue calling BIO_sock_non_fatal_error(), and copy it verbatim from 
OpenSSL if it's missing.
    
    Functionally, your solution is equivalent (and cleaner!), but unless 
there's a reason not to use OpenSSL's function, that's the style I prefer. I'm 
happy either way, though :-)
    
    I'll open a PR to add attribution to the NOTICE file.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

Reply via email to