On Thu, 17 Dec 2015, Inca R wrote:
I have been working on curl tfo support and managed to get a working version. This is my first pass at TFO with some basic understanding of curl code. Would love if you can have a look at it and guide me with whats needed to be done to make it in upstream. I have attached the patch for it. If you prefer a pull request on github I can do that as well.
Whatever you prefer. Either a patch here or a pull-request, both are fine with me.
First out what's really missing in your patch: * Documentation for the new option. * Make sure all test cases still run fine. Would be good to enable the option too and run to make sure that everything works like that too. * configure checks to see if MSG_FASTOPEN exists! Have you tested this on anything else than Linux? (I don't know what others that might support it.) And then some other things I thought about when reading it: * You've left printf() calls in there. For debugging/tracing, you can check out for example http2.c how we log stuff. * If a client supports TFO, is there a downside if we'd enable it by default? * The swriteto() macro seems pointless since it is only used at one place * Did you try this with a host that has both IPv4+IPv6 addresses to see that happy eyeballs play fine with it? * Does it handle failed connects properly and try the next address? Run a server on localhost and connect to bad10.haxx.se for example. (It is a DNS entry that returns 5 addresses, 4 of them are 10.0.0.X and one is 127.0.0.1) (I haven't tried it yet!) -- / daniel.haxx.se ------------------------------------------------------------------- List admin: http://cool.haxx.se/list/listinfo/curl-library Etiquette: http://curl.haxx.se/mail/etiquette.html
