Re: [PATCH] raw-FD related AsyncCall classes

 On Wed, 23 Nov 2011 18:35:19 -0700, Alex Rousskov wrote:
> On 11/23/2011 05:37 PM, Amos Jeffries wrote:
>> On Wed, 23 Nov 2011 10:56:24 -0700, Alex Rousskov wrote:
>>> On 11/23/2011 05:11 AM, Amos Jeffries wrote:
>
 <snip>
>
>>>> - int fd; // raw FD which the call was about. use conn instead
>>>> for
>>>> new code.
>>>> + int fd; /// FD which the call was about. Set by the caller
>>>> creator. Use conn instead for new code.
>>>
>>> s/caller creator/caller/? The creator of the callback does not set
>>> the
>>> fd value.
>>
>> The current ones using this do. See _comm_close() in comm.cc.
>
> The _comm_close() code does not create callbacks. It creates two
> async
> calls (commStartSslClose and comm_close_complete) and then updates
> and
> schedules callbacks created by others (via ...->finish() calls).
>

 Okay to be precise then s/caller/async call/ and dropping the mention
 of conn.

>
>> I would like the commCallCloseHandlers() not care about duplicating
>> that
>> effort. Just to schedule them all and finish.
>
> Not sure what you mean. I was only commenting on the unclear fd data
> member description, nothing else.
>

 Okay nevermind. Talking about future patches, we can come back to that
 when there is a relevant patch to look at.

>
>>> Also, since not all new code should use Connection after your
>>> changes, I
>>> would replace "Use conn instead for new code" with "Use conn if
>>> available" or something like that. I have to say that your changes
>>
>> ?? missing text?
>
> Sorry. Please ignore the "I have to say that your changes" part.
> IIRC, I
> was going to say that it becomes increasingly difficult to know when
> one
> should use fd and when one should use conn, but I already complained
> about it, and I do not want to argue about that again.

 This may help:
  * If it is a TCP socket, use conn.
  * If you don't have a conn available, avoid creating temporary ones
 for the sake of it.
  * If there is doubt and ability to avoid FD, do so.

 Amos
Received on Thu Nov 24 2011 - 02:14:08 MST