On 30/11/2011 1:21 p.m., Alex Rousskov wrote:
On 11/24/2011 04:04 AM, Amos Jeffries wrote:
Updated patch for review.
+ UnaryCbdataDialer(Handler *aHandler, CbcPointer<Argument1> aArg) :
The second parameter can probably be adjusted to "const
CbcPointer<Argument1>&" to minimize cbdata-refcounting overheads, but
see below.
Fixed by doing the constructor change below.
+// helper function to simplify Call creation.
s/Call/UnaryCbdataDialer/ or just s/Call/Dialer/
Done.
+cbdataDialer(typename UnaryCbdataDialer<Argument1>::Handler *handler,
Argument1 *arg1)
I am concerned that cbdataDialer() arg1 type does not match that of
UnaryCbdataDialer constructor. This is atypical for such helper
functions. You may want to change the constructor to use a raw pointer
as well.
Done.
+#if 0
+// dialer to run cbdata object members as Async Calls
+// to ease the transition of these cbdata objects to full Jobs
+template<class T>
+class UnaryCbdataDialer : public CallDialer
Please do not forget to remove during commit.
Oops. Dropped now.
+ int fd; /// FD which the call was about. Set by the async call creator.
Missing "<" after "///".
Done.
+ virtual bool canDial(AsyncCall&call) { return arg1.valid(); }
+ void dial(AsyncCall&call) { handler(arg1.get()); }
I do not think canDial() is virtual. AsyncCallT<Dialer> knows its dialer
so it does not need canDial() to be virtual to work. There is a TODO to
make canDial() and dial() virtual but it is blocked by
CommCbFunPtrCallT doing things differently.
grep -R "canDial" src/
It is virtual everywhere else except RockIoState. That might be a bug in
RockStore.
dial() is the one blocked being a virtual by CommCalls.
I did not find any serious bugs in this patch.
Giving it another run around the block for the dialer constructor change
and will commit then if it passes.
Amos
