Re: [PATCH] Updated Comm::Read I/O API

On 06/05/2014 02:33 AM, Amos Jeffries wrote:
>> > Please detail along these lines:
>> >
>> > // without a buffer, just call back and the caller will ReadNow()

> We place no criteria on what the caller will do next. The connection
> monitor callers will in fact close() instead of ReadNow().

This comment is not a precise description of the interface. The intent
is to better document the otherwise invisible expected sequences of
calls, not to enumerate all possible code paths. "What happens next?" is
a question many developers that try to work on Squid get blocked on
AFAICT. You may s/will/may/ to be more precise here.

>>> + char *b = buf.rawSpace(sz);
>>
>> Please rename "b" to "space", "rawSpace", or something like that to ease
>> reading/searching.
>>
> Done. "theBuf"

"The" is reserved for class data members and globals. Please avoid it in
local variable names.

>> Avoid non-changes.
>>
>
> Needs doing eventually and we put off a big patch so we could do these
> incrementally like this. I am re-witing the whole of the callers read
> handler function here.

Non-changes increase future conflicts without significant benefits. They
are also usually outside the declared patch scope.

Cheers,

Alex.
Received on Thu Jun 05 2014 - 13:52:32 MDT