On 06/01/2014 02:37 AM, Amos Jeffries wrote: > Lets take a step aside and cover why the buf2 is a pointer instead of an > separate SBuf.
I do not think those reasons are important enough in this context. It is like suggesting "let's examine the benefits of using uninitialized variables and creating dangling pointers" -- whatever the benefits are, they are insufficient to knowingly add such code to Squid. Giving Comm a raw pointer to an internal job member is very similar to "using uninitialized variables and creating dangling pointers". No amount of resulting optimizations can justify adding that kind of API IMO. > * simple solution and ony way to guarantee that data-copy is not > performed is to have comm_read operate on a pointer to sb1. AFAICT, among correct solutions, there is no simple solution that avoids data copy. We can either remain "simple" and copy OR add "complexity" to avoid copying. > comm_write() is a slightly different story. It *is* possible to take sb2 > as a copy then the write handler sb1.consume(len) for the written bytes. > > However, since read has the "SBuf *buf2" as a pointer ... so we are back to the discussion above: Since I do not think having buf2 pointer should be allowed, any conclusion based on having it there becomes irrelevant. > If you can provide a way to avoid all the above problems while > performing comm_read() on a non-pointer IoCallback "SBuf buf2;" I am > very interested. I do not think me providing a solution to all problems should be a precondition to removing unaudited code that introduces a raw pointer shared between Comm and jobs. If I understand the intent behind the audit procedures correctly, it suggests the opposite: removal of the code until a better solution is agreed upon. There are no simple solutions here and the complex ones require serious discussions and significant investment of time. IMO, there are better ways to spend those resources right now. In other words, there are more important problems to fix. Removing a data copy while crossing Comm boundary can wait, especially if nobody wants to take a lead on that project. Meanwhile, we can add a safe SBuf-friendly API and continue to copy. Alex.
