Re: [PATCH] TCP logging & UDP Logging - Dhaval Varia

On 04/19/2010 03:26 AM, Amos Jeffries wrote:
> Kinkie wrote:
>> On Thu, Apr 15, 2010 at 6:54 AM, Dhaval Varia <dhavalkvaria_at_gmail.com>
>> wrote:
>>> Dear Sir,
>>>
>>> Please find the attached patch file for the
>>>
>>> 1. TCP logging (Newly created) ------- modtcp.patch
>>
>> The patch is not malformed now. I still don't understand the
>> file-related error messages, so I leave this to someone with more clue
>> than I do.
>
> Neither. They came in from ModUdp. I suspect they will never happen by
> have not felt keen to remove them as part of this change.
>
>>
>>> 2. UDP logging (Modified) ------- modudpfix.patch
>>
>> The test reversal seems fine. +1 (I don't understand why adding the
>> four empty lines in logfile_mod_udp_open(); it clashes with Squid's
>> coding style guidelines.)
>>
>
> The needed 1-byte change to UDP has been in 3.HEAD for a while now. The
> UDP patch can be ignored now.
>
>>
>>> would you please commit this TCP logging and UDP fix first??
>>>
>>> I will see what to do to handle back pressure later.
>>
>> Ok by me. It's probably implicitly done if the socket is blocking.
>>
>
> It's a regular comm_open TCP socket. So I think its non-blocking by
> default.
>
> Buffering is done by the module. But we neglected to increase the buffer
> size yet. I'm bumping it up to 64K on commit in a few minutes.

As far as I could see the TCP logging code had no user-visible
documentation and left most new types/functions undocumented as well,
including the globally visible function. Please insist on documentation
when accepting patches.

Thank you,

Alex.
Received on Sat May 22 2010 - 22:25:49 MDT