*** empty log message ***

1 files changed, 138 insertions, 103 deletions

diff --git a/examples/Bounded_Packet_Relay/Thread_Bounded_Packet_Relay.cpp b/examples/Bounded_Packet_Relay/Thread_Bounded_Packet_Relay.cpp
index 17d9f2d947e..55475bc13d9 100644
--- a/examples/Bounded_Packet_Relay/Thread_Bounded_Packet_Relay.cpp
+++ b/examples/Bounded_Packet_Relay/Thread_Bounded_Packet_Relay.cpp
@@ -9,7 +9,7 @@
 //    Thread_Bounded_Packet_Relay.cpp
 //
 // = DESCRIPTION
-//    Method definitions for a threaded bounded packet relay  
+//    Method definitions for the threaded-bounded packet relay class.
 //
 // = AUTHORS
 //    Chris Gill          <cdgill@cs.wustl.edu>
@@ -21,14 +21,13 @@
 //
 // ============================================================================
 
-ACE_RCSID(Bounded_Packet_Relay, Thread_Bounded_Packet_Relay, "$Id$")
-
 #include "ace/Task.h"
 #include "ace/Timer_Heap_T.h"
 #include "ace/Timer_Queue_Adapters.h"
-
 #include "Thread_Bounded_Packet_Relay.h"
 
+ACE_RCSID(Bounded_Packet_Relay, Thread_Bounded_Packet_Relay, "$Id$")
+
 // constructor
 
 template <class RECEIVER, class ACTION>
@@ -47,10 +46,17 @@ Command<RECEIVER, ACTION>::execute (void *arg)
   return (receiver_.*action_) (arg);
 }
 
-
-Text_Input_Device_Wrapper::Text_Input_Device_Wrapper (ACE_Thread_Manager * input_task_mgr,
-                                                      size_t read_length, const char* text)
+Text_Input_Device_Wrapper::Text_Input_Device_Wrapper (ACE_Thread_Manager *input_task_mgr,
+                                                      size_t read_length, 
+                                                      const char* text)
   : Input_Device_Wrapper_Base (input_task_mgr)
+  // @@ Chris, your coding style here is not consistent with the rest of ACE.  this
+  // should be 
+  // : Input_Device.... (....),
+  //   read_length_ (....),
+  //   text_ (....), 
+  //   index_ (0)
+  // Please be follow this in the future.
   , read_length_ (read_length)
   , text_ (text)
   , index_ (0)
@@ -58,15 +64,17 @@ Text_Input_Device_Wrapper::Text_Input_Device_Wrapper (ACE_Thread_Manager * input
 }
   // ctor
 
-Text_Input_Device_Wrapper::~Text_Input_Device_Wrapper ()
+Text_Input_Device_Wrapper::~Text_Input_Device_Wrapper (void)
 {
 }
   // dtor
 
-
 ACE_Message_Block *
-Text_Input_Device_Wrapper::create_input_message ()
+Text_Input_Device_Wrapper::create_input_message (void)
 {
+  // @@ Chris, make sure to use the proper capitalization and
+  // punctuation for these and all comments...
+
   // construct a new message block to send
   ACE_Message_Block *message;
   ACE_NEW_RETURN (message, 
@@ -80,23 +88,25 @@ Text_Input_Device_Wrapper::create_input_message ()
   // loop through the text, filling in data to copy into
   // the read buffer (leaving room for a terminating zero)
   for (size_t i = 0; i < read_length_ - 1; ++i)
-  {
-    read_buf [i] = text_ [index_];
-    index_ = (index_ + 1) % ACE_OS::strlen (text_);
-  }
+    {
+      read_buf [i] = text_ [index_];
+      index_ = (index_ + 1) % ACE_OS::strlen (text_);
+    }
 
   // Copy buf into the Message_Block and update the wr_ptr ().
   if (mb->copy (read_buf, read_length_) < 0) 
-  {
-    delete message;
-    ACE_ERROR_RETURN ((LM_ERROR, "read buffer copy failed"), 0);
-  }
+    {
+      delete message;
+      ACE_ERROR_RETURN ((LM_ERROR,
+                         "read buffer copy failed"),
+                        0);
+    }
 
   return message;
 }
-  // creates a new message block, carrying data
-  // read from the underlying input device  
 
+// creates a new message block, carrying data
+// read from the underlying input device  
 
 Text_Output_Driver_Wrapper::Text_Output_Driver_Wrapper (int logging)
   : logging_ (logging)
@@ -109,20 +119,33 @@ Text_Output_Driver_Wrapper::write_output_message (void *message)
 {
   ACE_Message_Block *message;
 }
-  // consume and possibly print out the passed message
+
+// consume and possibly print out the passed message
 
 int 
 Text_Output_Driver_Wrapper::modify_device_settings (void *logging)
 {
+  // @@ Chris, your indentation throughout this file is not consistent
+  // with ACE coding guidelines.  Please make sure you use the
+  // indentation format that GNU emacs provide.  If you have any
+  // questions, please let me know.
   if (logging)
   {
-    logging_ = * ACE_static_cast (int *, logging);
+    logging_ = *ACE_static_cast (int *, logging);
   }
   else
   {
-    ACE_ERROR_RETURN ((LM_ERROR, "null logging level pointer"), -1);
+    ACE_ERROR_RETURN ((LM_ERROR,
+                       "null logging level pointer"),
+                      -1);
   }
 }
+
+// @@ Chris, you are not intenting your comments correctly.  The
+// comments before each method should be flush against the left column
+// and there should be only one blank line between the comments and
+// the method.
+
   // modifies device settings based on passed pointer to a u_long
   // turns logging on if u_long is non-zero, off if u_long is zero,
   // and does nothing if the pointer is null.
@@ -136,6 +159,7 @@ User_Input_Task::User_Input_Task (Thread_Timer_Queue *queue,
     driver_ (tbprd)
 {
 }
+
   // ctor
 
 int 
@@ -150,9 +174,12 @@ User_Input_Task::svc (void)
   // we are done.
   this->relay_->end_transmission (Bounded_Packet_Relay_Driver::CANCELLED);
   this->queue_->deactivate ();
-  ACE_DEBUG ((LM_DEBUG, "terminating input thread\n"));
+  ACE_DEBUG ((LM_DEBUG,
+              "terminating input thread\n"));
   return 0;
-}
+} // @@ Chris, you are putting the comments in the wrong place...
+ // They should go BEFORE the method, not after it...  It's only in
+ // the header file that they must go after the method...
   // This method runs the event loop in the new thread.
 
   // = Some helper methods.
@@ -162,7 +189,7 @@ User_Input_Task::set_packet_count (void *argument)
 {
   if (argument)
   {
-    packet_count_ = * ACE_static_cast (int *, argument);
+    packet_count_ = *ACE_static_cast (int *, argument);
     return 0;
   }
 
@@ -177,7 +204,7 @@ User_Input_Task::set_arrival_period (void *argument)
 {
   if (argument)
   {
-    arrival_period_ = * ACE_static_cast (int *, argument);
+    arrival_period_ = *ACE_static_cast (int *, argument);
     return 0;
   }
 
@@ -193,7 +220,7 @@ User_Input_Task::set_send_period (void *argument)
 {
   if (argument)
   {
-    send_period_ = *  ACE_static_cast (int *, argument);
+    send_period_ = *ACE_static_cast (int *, argument);
     return 0;
   }
 
@@ -209,7 +236,7 @@ User_Input_Task::set_duration_limit (void *argument)
 {
   if (argument)
   {
-    duration_limit_ = * ACE_static_cast (int *, argument);
+    duration_limit_ = *ACE_static_cast (int *, argument);
     return 0;
   }
 
@@ -224,7 +251,7 @@ User_Input_Task::set_logging_level (void *argument)
 {
   if (argument)
   {
-    logging_level_ = * ACE_static_cast (int *, argument);
+    logging_level_ = *ACE_static_cast (int *, argument);
     return 0;
   }
 
@@ -248,12 +275,13 @@ User_Input_Task::run_transmission (void *argument)
                                         logging_level_))
     {
       case 1:
+        // @@ Chris, please don't use ACE_OS::fprintf() for things
+        // like this, use ACE_DEBUG or ACE_ERROR instead.
         ACE_OS::fprintf (ACE_STDERR, 
                          "\nRun transmission: "
                          "transmission already in progress\n");
         return 0;
-         /* not reached */
-
+        /* NOT REACHED */
       case 0:
       {
         ACE_Time_Value now = ACE_OS::gettimeofday ();
@@ -342,11 +370,12 @@ User_Input_Task::end_transmission (void *argument)
 
         case 0: 
           // cancel any remaining timers
-          ACE_Timer_Node_T <ACE_Event_Handler *> *node;
-          while ((node = queue_->timer_queue ().get_first ()))
-          {
+
+          for (ACE_Timer_Node_T <ACE_Event_Handler *> *node;
+               (node = queue_->timer_queue ().get_first ()) != 0;
+               )
             queue->cancel (node->get_timer_id (), 0);
-          }
+
           return 0;
            /* not reached */
   
@@ -403,7 +432,7 @@ User_Input_Task::report_stats (void *argument)
   // reports statistics for the previous transmission 
   // (if one is not in progress)
 
- int 
+int 
 User_Input_Task::shutdown (void *argument)
 {
   // Macro to avoid "warning: unused parameter" type warning.
@@ -422,11 +451,11 @@ User_Input_Task::shutdown (void *argument)
 }
   // Shutdown task.
 
-
 Send_Handler::Send_Handler (u_long send_count, 
-                const ACE_Time_Value &duration,
-                Bounded_Packet_Relay<ACE_Thread_Mutex> &relay,
-                Thread_Timer_Queue &queue)
+                            const ACE_Time_Value &duration,
+                            Bounded_Packet_Relay<ACE_Thread_Mutex> &relay,
+                            Thread_Timer_Queue &queue)
+  // @@ Chris, please fix the formatting here...
   : send_count_ (send_count)
   , duration_ (duration)
   , relay_ (relay)
@@ -445,42 +474,44 @@ Send_Handler::handle_timeout (const ACE_Time_Value &current_time,
 			      const void *arg)
 {
   switch (relay_->send_input ())
-  {
+    {
     case 0:
       // decrement count of packets to relay
       --send_count_;
       /* fall through to next case */
     case 1:
       if (send_count_ > 0)
-      {
-        // re-register the handler for a new timeout
-        if (queue_->schedule (this, 0, 
-                              duration_ + ACE_OS::gettimeofday ()) < 0)
         {
-          ACE_ERROR_RETURN ((LM_ERROR, 
-                             "Send_Handler::handle_timeout: "
-                             "failed to reschedule send handler"), 
-                            -1);
+          // re-register the handler for a new timeout
+          if (queue_->schedule (this, 0, 
+                                duration_ + ACE_OS::gettimeofday ()) < 0)
+            ACE_ERROR_RETURN ((LM_ERROR, 
+                               "Send_Handler::handle_timeout: "
+                               "failed to reschedule send handler"), 
+                              -1);
+          return 0;
         }
-        return 0;
-      }
       else
-      {
-        // all packets are sent, time to cancel any other
-        // timers, end the transmission, and go away
-        ACE_Timer_Node_T <ACE_Event_Handler *> *node;
-        while ((node = queue_->timer_queue ().get_first ()))
         {
-          queue->cancel (node->get_timer_id (), 0);
+          // @@ Chris, I think you repeat this code a number of times.
+          // Can you please abstract it out into a helper method and
+          // call it, rather than writing it redundantly?
+
+          // all packets are sent, time to cancel any other
+          // timers, end the transmission, and go away
+          for (ACE_Timer_Node_T <ACE_Event_Handler *> *node;
+               (node = queue_->timer_queue ().get_first ()) != 0;
+               )
+            queue->cancel (node->get_timer_id (), 0);
+
+          relay_->end_transmission (Bounded_Packet_Relay::COMPLETED);
+          delete this;
+          return 0;
         }
-        relay_->end_transmission (Bounded_Packet_Relay::COMPLETED);
-        delete this;
-        return 0;
-      }
-    /* not reached */
+      /* NOT REACHED */
     default:
       return -1;
-  }
+    }
 }
   // Call back hook.
 
@@ -492,9 +523,8 @@ Send_Handler::cancelled (void)
 }
   // Cancellation hook
 
-
 Termination_Handler::Termination_Handler (Bounded_Packet_Relay<ACE_Thread_Mutex> &relay,
-                       Thread_Timer_Queue &queue)
+                                          Thread_Timer_Queue &queue)
   : relay_ (relay)
   , queue_ (queue)
 {
@@ -508,15 +538,15 @@ Termination_Handler::~Termination_Handler (void)
 
 int 
 Termination_Handler::handle_timeout (const ACE_Time_Value &current_time,
-			      const void *arg)
+                                     const void *arg)
 {
   // transmission timed out, cancel any other
   // timers, end the transmission, and go away
-  ACE_Timer_Node_T <ACE_Event_Handler *> *node;
-  while ((node = queue_->timer_queue ().get_first ()))
-  {
+  for (ACE_Timer_Node_T <ACE_Event_Handler *> *node;
+       (node = queue_->timer_queue ().get_first ()) != 0;
+       )
     queue->cancel (node->get_timer_id (), 0);
-  }
+
   relay_->end_transmission (Bounded_Packet_Relay::TIMED_OUT);
   delete this;
   return 0;
@@ -531,19 +561,19 @@ Termination_Handler::cancelled (void)
 }
   // Cancellation hook
 
-Thread_Bounded_Packet_Relay_Driver::Thread_Bounded_Packet_Relay_Driver (void);
+Thread_Bounded_Packet_Relay_Driver::Thread_Bounded_Packet_Relay_Driver (void)
   : input_task_ (&timer_queue_, *this)
 {
 }
 // ctor
 
-Thread_Bounded_Packet_Relay_Driver::~Thread_Bounded_Packet_Relay_Driver (void);
+Thread_Bounded_Packet_Relay_Driver::~Thread_Bounded_Packet_Relay_Driver (void)
 {
 }
 // dtor
 
 int 
-Thread_Bounded_Packet_Relay_Driver::display_menu (void);
+Thread_Bounded_Packet_Relay_Driver::display_menu (void)
 {
   static char menu[] =
     "\n\n  Options:\n"
@@ -563,7 +593,8 @@ Thread_Bounded_Packet_Relay_Driver::display_menu (void);
     "  8 - report statistics from the most recent transmission\n"
     "  9 - quit the program\n";
 
-  ACE_OS::fprintf(ACE_STDERR, "%s", menu);
+  // @@ Chris, don't use ACE_OS::printf...
+  ACE_OS::fprintf (ACE_STDERR, "%s", menu);
 
   return 0;
 }
@@ -572,68 +603,72 @@ Thread_Bounded_Packet_Relay_Driver::display_menu (void);
 int 
 Thread_Bounded_Packet_Relay_Driver::init (void);
 {
+  // @@ Chris, I suspect that putting a typedef here may break some
+  // compilers.  Can you please move it to outside of the method?
   typedef Command<Input_Task, Input_Task::ACTION> CMD;
 
   // initialize the <Command> objects with their corresponding
   // methods from <Input_Task>
   ACE_NEW_RETURN (packet_count_cmd_,
-                  CMD (input_task_, &User_Input_Task::set_packet_count),
+                  CMD (input_task_,
+                       &User_Input_Task::set_packet_count),
                   -1);
-
   ACE_NEW_RETURN (arrival_period_cmd_,
-                  CMD (input_task_, &User_Input_Task::set_arrival_period),
+                  CMD (input_task_,
+                       &User_Input_Task::set_arrival_period),
                   -1);
-
   ACE_NEW_RETURN (transmit_period_cmd_,
-                  CMD (input_task_, &User_Input_Task::set_send_period),
+                  CMD (input_task_,
+                       &User_Input_Task::set_send_period),
                   -1);
-
   ACE_NEW_RETURN (duration_limit_cmd_,
-                  CMD (input_task_, &User_Input_Task::set_duration_limit),
+                  CMD (input_task_,
+                       &User_Input_Task::set_duration_limit),
                   -1);
-
   ACE_NEW_RETURN (logging_level_cmd_,
-                  CMD (input_task_, &User_Input_Task::set_logging_level),
+                  CMD (input_task_,
+                       &User_Input_Task::set_logging_level),
                   -1);
-
   ACE_NEW_RETURN (run_transmission_cmd_,
-                  CMD (input_task_, &User_Input_Task::run_transmission),
+                  CMD (input_task_,
+                       &User_Input_Task::run_transmission),
                   -1);
-
   ACE_NEW_RETURN (cancel_transmission_cmd_,
-                  CMD (input_task_, &User_Input_Task::end_transmission),
+                  CMD (input_task_,
+                       &User_Input_Task::end_transmission),
                   -1);
-
   ACE_NEW_RETURN (report_stats_cmd_,
-                  CMD (input_task_, &User_Input_Task::report_stats),
+                  CMD (input_task_,
+                       &User_Input_Task::report_stats),
                   -1);
-
   ACE_NEW_RETURN (shutdown_cmd_,
-                  CMD (input_task_, &User_Input_Task::shutdown),
+                  CMD (input_task_,
+                       &User_Input_Task::shutdown),
                   -1);
-
   if (this->input_task_.activate () == -1)
-    ACE_ERROR_RETURN ((LM_ERROR, "cannot activate input task"), -1);
-
-  if (this->timer_queue_.activate () == -1)
-    ACE_ERROR_RETURN ((LM_ERROR, "cannot activate timer queue"), -1);
-
-  if (ACE_Thread_Manager::instance ()->wait () == -1)
-    ACE_ERROR_RETURN ((LM_ERROR, "wait on Thread_Manager failed"),-1);
-
+    ACE_ERROR_RETURN ((LM_ERROR,
+                       "cannot activate input task"),
+                      -1);
+  else if (this->timer_queue_.activate () == -1)
+    ACE_ERROR_RETURN ((LM_ERROR,
+                       "cannot activate timer queue"),
+                      -1);
+  else if (ACE_Thread_Manager::instance ()->wait () == -1)
+    ACE_ERROR_RETURN ((LM_ERROR,
+                       "wait on Thread_Manager failed"),
+                      -1);
   return 0;
 }
 // initialize the driver
 
 int 
-Thread_Bounded_Packet_Relay_Driver::run (void);
+Thread_Bounded_Packet_Relay_Driver::run (void)
 {
   this->init ();
   return 0;
 }
 // run the driver
 
-
 #if defined (ACE_HAS_EXPLICIT_TEMPLATE_INSTANTIATION)
 template class ACE_Thread_Timer_Queue_Adapter<Timer_Heap>;
 template class Bounded_Packet_Relay_Driver<Thread_Timer_Queue,