Re: [PATCH] Enhanced access logging, added ICAP logging, bug #2495 fix.

I am committing a version 3 of the patch.

Amos Jeffries wrote:
> Tsantilas Christos wrote:
>> Alex Rousskov wrote:
>>> On 07/09/2009 12:06 AM, Amos Jeffries wrote:
>>>> Tsantilas Christos wrote:
>
>
>>>> ** Please make fqdncache_entry / ipcache_entry public classes instead
>>>> of functional structs. Some gcc complain and doxygen won't document
>>>> their functions properly.
>>>
>>> Sorry, I am not sure what you mean. What is a "functional struct"? Do
>>> you want us to replace "struct fqdncache_entry" with "class
>>> fqdncache_entry" and add "public:"?
>>>
>>> No objections, of course, just wanted to minimize merging headaches by
>>> keeping out-of-scope changes to the minimum...
>>>
>> I did not touch it. The struct used in many places inside squid3
>>
>
> Please do change.
>
> The rest of the code fails to prefix the names with "struct" the typedef
> made that possible.
>
> Using class and public: is the C++ upgrade equivalent to the typedef and
> allows functions/methods inside the struct where I suspect the typedef
> did not.
>
> If there are code references to "struct fqdncache_entry" or "struct
> ipcache_entry", they are well within scope of the change removing
> relevant typedefs.
> FWIW: I find none in the existing code. Which makes the change safe.
>

OK done.

>..............
>>
>> My compiler (because it is very clever @#%$##$%#!) does not allow me
>> to remove the LFT_HTTP_SENT_STATUS_CODE_OLD_30 from switch statements.
>
> Is that because of a lack of default: ?
Yes, but it is not good idea to add a "default:" statement here.

>
> Never mind then. Just add a note saying what compiler complains if its
> removed so I don't get just as smart and do it :)

OK I add the comment.

>
>>>
>>>> I spy a goto that can die...
>>>> case CLF_NONE:
>>>> goto last;
>>>> @@ -1458,7 +1863,15 @@
>>>> last:
>>>> (void)0; /* NULL statement for label */
>>>> +}
>>>>
>>>> ==> equates to "return;". Please check for others in that same
>>>> function.
>>
>> I prefer to not touch it at this time. We can change it in trunk with
>> a separate patch.
>
> Okay. Fair enough.
>
>
> Amos