On Sun, Feb 21, 2016 at 4:51 PM, Alex Rousskov <rouss...@measurement-factory.com> wrote: > On 02/20/2016 11:27 AM, Kinkie wrote: > >> Sorry to bring this topic up again, but honestly I don't understand >> your position. >> >> I believe that the deadlock we currently are in > > There is no deadlock. I think the design decision to use templated > escape functions to accomodate std::string-using helpers was wrong, but > I am not bold enough to stop you from committing it. Much worse code > goes in despite my objections or without proper audit.
Unfortunately there is: I am prepared to abandon this effort unless we reach consensus - at least non-opposition. It'd be a pity, as it's an useful feature. >> is caused by the fact >> that we are discussing about two topics at the same time. >> One is: should Rfc3986 target both SBuf and std::string, if the >> performance and complexity penalty in doing so is none or small? I >> (and I believe Amos) think it's a good, or at least not a bad idea. > > Any "Should I do X if X is good?" question is pointless. The question is > "Should I do X?". I have already outlined why I think doing X is harmful. > > >> The other is: should we use std::string or SBuf or c-strings in >> helpers? My opinion is that std::string would be preferable, yours is >> that SBuf is preferable, if any change is performed over the current >> state of using c-strings. I think change over current state is a good >> idea, you - I believe - are averse enough to using std::string that >> would prefer no change. > > I think helpers should use whatever works best for them, given the > available Squid APIs and other factors. The question is not about what > helpers should use but whether Squid code should bend over backwards > (e.g., providing a templated function to escape strings and adjust SBuf > to work with that function) to accommodate a helper. The answer, IMO, is > "no". It is much better to > * provide SBuf to helpers that want top performance > > and > > * provide a trivial std::string-escaping function (that uses > SBuf-escaping function internally) to helpers that want to use C-strings > or std::strings. Ok, I believe here is the crux of the issue. rfc3986 is a library; we could split it into two versions, one to be used by helpers and one to be used by squid. The two versions would be remarkably similar and have a lot of code duplications, or as you propose be one an adapter of the other. An adapter exposing a std::string API and using SBuf internally would be quite inefficient as it'd need to copy data back and forth. Performance is not a big concern when it comes to helpers, but I argue that this approach would require more code, be less efficient, and increase coupling between squid and helpers than the template-based approach. In order to clarify, when I talk about couplign I mean link-time coupling. This can be directly understood from the list of objects needed for testSBuf: between headers, objects and stubs that's about 30 files (on top of the testSBuf files themselves). >> Can we try to decouple the two discussions and see if this helps us >> reach a consensus? >> >> My point of view is: >> - having a more generic API costs us nothing - the code is compact, >> readable and efficient regardless the genericity, so we should merge >> Rfc3986. > > A single template costs virtually nothing. I speculate that the design > decision to treat std::string-using helpers as the primary driving > factor for Squid APIs will cost a lot long-term. Let's be clear: Squid API are in no way influenced by this. However Squid has a need (a rfc3986/1738 codec library) which also helpers have. Using a template leverages the opportunity offered by the very similar APIs that std::string and SBuf have to increase applicability of the library. > This is very similar to assert(string_size < 64K). The cost of that > single line was nothing when it was written. However, the cost of the > design decision that Strings can self-limit their size is a growing > collection of CVEs. IMO, you are opening a Pandora box. The cost of > lifting the lid is indeed negligible, but that cost is not what I am > worried about. I still can't see the long-term harm, while I can see the short-term harm in not following my idea. To the cost of repeating myself, we want a SBuf API for squid where performance and COW matter a lot. I wish a std::string API for helpers to avoid having to link 30+ squid files and stubs for each helper (object size is not a concern, it's more about maintaining this long list of dependencies). >> - there is no performance or readability benefit in using SBuf in >> helpers due to the way client code is structured. > > I agree regarding readability. SBuf is about the same or a little worse > than std::string. I agree > If you are right about performance and safety across all helpers, then > there is no benefit in using SBuf in Squid. Here I disagree. SBuf is relevant for squid. It is not for helpers. BTW: if there was a way to reasonably integrate std::string and mempools, that would be worth investigating. I haven't found it. >> Also, we would need >> to stub a lot of functionality it relies on (e.g. mempools, >> statistics, cachemgr). > > I am not sure that is true, but even if it is true, it should not be > important for this discussion. The vast majority of such refactoring, > would benefit Squid. For example, if our SBuf code depends on cache > manager, then we made a mistake that should be fixed. It doesn't depend from cachemgr to work, but it links to it. This can probably be refactored to decrease decoupling. -- Francesco _______________________________________________ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev