Actually this patch is from Tushar Krishna. I did notice your recent style patch, but I failed to notice that Tushar's patch didn't follow that. I completely agree with your comments. I'll make the changes.
Brad -----Original Message----- From: [email protected] [mailto:[email protected]] On Behalf Of nathan binkert Sent: Thursday, March 18, 2010 4:28 PM To: M5 Developer List Subject: Re: [m5-dev] [PATCH 13 of 31] Fix bug in Ruby Event queue to avoid multiple wakeups of same consumer in same cycle All of the things below are things that I've fixed in my style pass and things that I hope we can stick to. Nate > #include "mem/ruby/common/Global.hh" > #include "mem/ruby/eventqueue/RubyEventQueue.hh" > +#include <set> Our style has us put system includes above local includes > + > +using namespace std; You should not do this in a header file. Put std:: where it's needed. You can do this in a .cc file if you like though. > > class MessageBuffer; > > @@ -61,12 +64,16 @@ > virtual void print(std::ostream& out) const = 0; > const Time& getLastScheduledWakeup() const { return > m_last_scheduled_wakeup; } > void setLastScheduledWakeup(const Time& time) { m_last_scheduled_wakeup = > time; } > + bool alreadyScheduled(Time time) { return (m_scheduled_wakeups.find(time) > != m_scheduled_wakeups.end()); } > + void insertScheduledWakeupTime(Time time) { > m_scheduled_wakeups.insert(time); } > + void removeScheduledWakeupTime(Time time) { > assert(alreadyScheduled(time)); m_scheduled_wakeups.erase(time); } These lines are too long. Style would be this: void removeScheduledWakeupTime(Time time) { assert(alreadyScheduled(time)); m_scheduled_wakeups.erase(time); } _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
