On Sun, 10 Jan 2021, Patrick Monnerat via curl-library wrote:
Considering the large number of references, I would suggest starting to fix this "bottom up" by smaller commits rather than having a big patch, even if it increases the reference count at first while the work is not complete.
Right. That's my thinking as well and a reason for not just removing the 'conn->data' pointer at once. I want the removal to be the goal, but the journey of getting to that point is allowed to take time.
However, it is also one of these architectual changes that once you start to change two places you'll notice that you also need to update a third place, which reveals the forth etc and all of a sudden the change is massive and interconnected.
I started out by changing the protocol handler function pointers to accept 'Curl_easy *' instead of 'connectdata *'. It took me down a rabit hole for a while but I think I can breathe again now:
https://github.com/curl/curl/pull/6425
I also noticed there are a lot of references to conn->data for logging purposes only (infof, failf, debug) in connection-oriented procedures: maybe we should discuss an alternate strategy for logging from those procedures.
I don't think we have a lot of wiggle room to do that. Everything we do in libcurl is oriented around a transfer and properties like callbacks and VERBOSE are set in the transfer object. We can't output any logs without also knowing the transfer!
In *most* of those cases we can reverse what we pass in to the functions. Pass in 'data' and extract 'conn' from that, instead of the other way around. Or if the function is called in setup/disconnect (before or after the connection is assiciated with the transfer), pass in both.
-- / daniel.haxx.se | Commercial curl support up to 24x7 is available! | Private help, bug fixes, support, ports, new features | https://www.wolfssl.com/contact/ ------------------------------------------------------------------- Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library Etiquette: https://curl.se/mail/etiquette.html
