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

Reply via email to