Re: ayjwork squid3/src HttpRequestMethod...

Amos Jeffries wrote:
> Alex Rousskov wrote:
>> On Sat, 2008-02-02 at 13:12 +0000, Amos Jeffries wrote:
>>> Update of cvs.devel.squid-cache.org:/cvsroot/squid/squid3/src
>>>
>>> Modified Files:
>>> Tag: ayjwork
>>> HttpRequestMethod.cc HttpRequestMethod.h SquidString.h
>>> String.cci Log Message:
>>> Compile errors. From HttpRequestMethod class addition.
>>>
>>> Test failures are occuring and still need fixing.
>>
>> Amos,
>>
>> I could not tell from the diff what went wrong (too much formatting
>> noise?).
>
> Several compiling problems at once when "make check" is run. Try it on
> HEAD to see what I mean if you like:
> - operator << overloading problem (more on that below).
> - CPPUNIT_ASSERT_EQUALS requires two parameters of identical types. It
> will NOT do any silent conversion. So _method_t vs HTTPRequestMethod
> tests all failed building.
> - Const_str() method changed to image() but some calls left unaltered.
>
>> Please let me know if you need any help with HttpRequestMethod
>> polishing other than the test cases. If am especially worried about
>> this:
>>
>>> + #if 0
>>> + /* AYJ: Duplicated code. Seem an HttpRequestMethod can itself be
>>> converted to String() and
>>> + * this operator clashes with that operator.
>>> + */
>>> inline std::ostream &
>>> operator << (std::ostream &os, HttpRequestMethod const &method)
>>> ***************
>>> *** 171,174 ****
>>> --- 173,177 ----
>>> return os;
>>> }
>>> + #endif
>>
>> I do not see how HttpRequestMethod can be converted into a String, but
>> if that is the case, we should probably remove such auto-conversion.
>>
>> I do not know what error you are getting, but it is possible that
>> auto-conversion from HttpRequestMethod to method_t is at fault. If so,
>> we should probably try to remove that conversion.
>
> Yes it was, I found that in the tests hat were still failing it keeps
> displaying HttpRequestMethod("GET") as '1' instead of "GET".
>
> For some weird reason my compiler was saying a bad overloading between:
> operator <<(ostream &os, String const &s)
> and operator <<(ostream &os, HttpRequestMethod const &s)
> which threw me for a while.
>
> Dropping the silent _method_t / int conversion proves to be a better
> solution than dropping the <<. I'm testing making access to the int
> value an explicit call to value() instead of silent conversion.
>
>>
>> Again, let me know if you need a hand.
>
> After my next patch goes into ayjwork branch, if you wouldn't mind
> running every test you know on the branch it would be good. :-)
> But as usual I'm not expecting anyone but me will go near the code until
> its already in HEAD. :-(
>

Sigh, it looked promising fro a while.

The exact errors is when compiling testHeaders_HttpRequestMethod.cc.
The test where there is no code run, just the dependency and inline build.

./HttpRequestMethod.h: In function ‘std::ostream&
operator<<(std::ostream&, const HttpRequestMethod&)’:
./HttpRequestMethod.h:181: error: ambiguous overload for ‘operator<<’ in
‘os << ((const HttpRequestMethod*)method)->HttpRequestMethod::image()’
./String.cci:186: note: candidates are: std::ostream&
operator<<(std::ostream&, const String&)
./HttpRequestMethod.h:179: note: std::ostream&
operator<<(std::ostream&, const HttpRequestMethod&)

And its back, removing the silent _method_t() conversion has no effect.
I'll keep experimenting, but if you have any ideas please let me know.

Amos

-- 
Please use Squid 2.6STABLE17+ or 3.0STABLE1+
There are serious security advisories out on all earlier releases.