Re: SqString

On Fri, 2007-05-25 at 20:39 +1200, Amos Jeffries wrote:

> I have just been experimenting with a few options short of a full backout.
>
> My initial idea of dropping the constructor drags the changes into areas
> the initial patch didn't touch. No go there.

Yes, of course.

> I have had some success dropping the str* overloading entirely.

Since the old code did not have those, I suspect the new code should
have "complete success" without them.

> Replacing it all with a single casting operator which explicitly forces
> the magic casts to reverse from the expensive copy 'up' toward string
> into a cheap cast 'down' to a c_str() call.

I am not sure why we need that operator now. On the IRC, you said that
it "it seems to be the only working alternative to complete removal of
the change" but I do not understand why.

Moreover, if we must have implicit casts for the new API to work,
perhaps we should back out the entire API change for now because we are
not good at predicting the side-effects of such implicit casts.

> Keep in mind this c_str() call being explicitly done all over squid at
> present anyway. The only known risk is a buffer over- or under-run which
> would occur anyway if the alternative explicit c_str() call was coded.

Here is my problem: The original API change changed some strcmp-like
calls. Some of the changes were incorrect. It is difficult for me to say
whether removing strcmp overloading and adding implicit char* casts will
expose all incorrect changes that we have not found yet. Unfortunately,
I doubt it.

What I would like to see is a diff against the pre-new-API code showing
that no calls got rewritten. Renaming a method is fine, but most other
changes should be postponed. The new patch you posted does not show me
how the main code is affected by the two changes combined, which is the
only thing that matters. Comparing with the current, broken code is not
very informative...

> *please* test it this time. patch is attached or checkout and test the
> squid3 branch labeled 'ayjwork'.
>
> Alex: I'd particularly like a OK/DIES from you and your ICAP testing.

We do not have a robust testing environment yet, so an "OK" from me
would not mean much.

Alex.
Received on Fri May 25 2007 - 08:18:05 MDT