Re: [PATCH] Http::MethodType upgrade

On 25.07.2012 02:28, Alex Rousskov wrote:
> On 07/23/2012 08:31 PM, Amos Jeffries wrote:
>
>> * url.cc parsing switch cases used to split CONNECT are converted
>> to
>> if-else syntax
>> - not a major change, but worth mentioning since it is logic
>> alteration.
>
> I see the switch/if change, but it is not clear to me what changed in
> the processing logic itself. Perhaps the commit message can
> document/explain that?

Hopefully nothing. Just mentioning in case I made a typo or something
stupid.

>
> BTW, there seems to be quite a bit of duplicated snprintf()-related
> code
> in those url.cc changes. Please factor it out if possible.
>

I have bug 1961 URL code cleanup coming in a later patch.

>
>> HttpRequest class changes:
>>
>> - updated to cache the complex calculation result.
>
> I think the maybeCacheable_ caching must be removed because it will
> create problems similar to canonical URL caching: There is no code to
> invalidate the cached value when related request properties change.
> And
> the calculation is not that costly from CPU cycles point of view to
> necessitate caching anyway.

Okay. Dropped.

>
>
>> TODO: needs testing through co-advisor to check for RFC compliance
>> regressions.
>
> Please ping Dmitry (CCed) to organize this regression check when the
> final patch version is available.
>

Attached patch is on top of trunk rev.12228.

Amos