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