Closed
Bug 790428
Opened 12 years ago
Closed 12 years ago
C++ sutagent needs heartbeat channel
Categories
(Testing Graveyard :: SUTAgent, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: mihneadb, Assigned: mihneadb)
Details
Attachments
(1 file, 2 obsolete files)
15.21 KB,
patch
|
mcote
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → mbalaur
Assignee | ||
Comment 1•12 years ago
|
||
https://github.com/mihneadb/Negatus/tree/factory I had to rewrite those loops in Reactor.cpp because they would segfault. Removing the first / last element was buggy. It's safer now.
Attachment #660641 -
Flags: feedback?(mcote)
Comment 2•12 years ago
|
||
Looks like you didn't pull from me. I already have fixes to those loops along with asynchronizing the exec command. Sorry but can you rebase your patch? We'd gotten into a back-and-forth pattern lately so I didn't think to send a pull request or anything. It shouldn't conflict much except the Reactor loop fixes (would prefer if you kept mine).
Assignee | ||
Comment 3•12 years ago
|
||
Sure, I'll update and repost the diff. :)
Comment 4•12 years ago
|
||
Good catch regardless - those Reactor bugs took me a while to sort out the other day. :)
Assignee | ||
Comment 5•12 years ago
|
||
Thanks, long live valgrind ;)
Assignee | ||
Comment 6•12 years ago
|
||
Attachment #660641 -
Attachment is obsolete: true
Attachment #660641 -
Flags: feedback?(mcote)
Attachment #660704 -
Flags: feedback?(mcote)
Assignee | ||
Comment 7•12 years ago
|
||
https://github.com/mihneadb/Negatus/tree/factory
Attachment #660704 -
Attachment is obsolete: true
Attachment #660704 -
Flags: feedback?(mcote)
Attachment #661010 -
Flags: review?(mcote)
Comment 8•12 years ago
|
||
Comment on attachment 661010 [details] [diff] [review] Move cmdeventhandler functions back Review of attachment 661010 [details] [diff] [review]: ----------------------------------------------------------------- This is great. :) Just a few comments below. Also if you wanted to merge the factory files into the event handler files (e.g. EventHandlerFactory.h/.cpp into EventHandler.h/.cpp, CommandEventHandlerFactory.h/.cpp into CommandEventHandler.h/.cpp, etc.), I would be okay with that. Up to you. :) r+ with the small changes below. ::: src/CommandEventHandlerFactory.cpp @@ +9,5 @@ > + > +EventHandler* > +CommandEventHandlerFactory::createEventHandler(PRFileDesc* socket) > +{ > + return new CommandEventHandler(socket); 2-space indents. ::: src/CommandEventHandlerFactory.h @@ +11,5 @@ > + > +class CommandEventHandlerFactory: public EventHandlerFactory > +{ > +public: > + virtual EventHandler* createEventHandler(PRFileDesc* socket); 2-space idents. ::: src/EventHandlerFactory.h @@ +9,5 @@ > + > + > +class EventHandlerFactory { > +public: > + virtual EventHandler* createEventHandler(PRFileDesc* socket) = 0; Ditto. ::: src/HeartbeatEventHandler.cpp @@ +86,5 @@ > +void > +HeartbeatEventHandler::handleTimeout() > +{ > + sendThump(); > + setTimeout(PR_SecondsToInterval(TIMEOUT)); It might be better to set the timeout before sending the thump to get this closer to once every minute. I might have to put in a recurring timeout function to really prevent it from wandering, though. @@ +96,5 @@ > + PRTime now = PR_Now(); > + PRExplodedTime ts; > + PR_ExplodeTime(now, PR_LocalTimeParameters, &ts); > + > + char buffer[BUFSIZE]; You know how big this string is going to be, so I don't think there's any point in allocating a huge buffer. ::: src/SUTAgent.cpp @@ +100,5 @@ > + PRStatus status = acceptor->listen(acceptorAddr); > + if (status == PR_FAILURE) > + { > + std::cerr << "Failure to open socket: " << PR_GetError() << std::endl; > + exit(1); I find exit calls in random functions to be very confusing; I'd prefer this function return a bool and have main() return if there's an error. ::: src/Shell.cpp @@ +21,5 @@ > + if (!iface) > + continue; > + > + fgets(buffer, BUFSIZE, iface); > + buffer[strlen(buffer) - 1] = '\0'; // remove extra newline I get a compilation error here; you need to #include <string.h>. ::: src/SocketAcceptor.h @@ +23,4 @@ > > private: > PRFileDesc* mSocket; > + EventHandlerFactory* handlerFactory; Call this mHandlerFactory for consistency.
Attachment #661010 -
Flags: review?(mcote) → review+
Assignee | ||
Comment 9•12 years ago
|
||
https://github.com/mihneadb/Negatus/commit/51f997c9fdfd1e28a79d0ef232780966a49767f6
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Updated•6 years ago
|
Product: Testing → Testing Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•