Comments and fixes that came up during JAWS code review.

3 files changed, 120 insertions, 109 deletions

diff --git a/ace/Filecache.cpp b/ace/Filecache.cpp
index ff3a0fe8a79..ecbde7255f0 100644
--- a/ace/Filecache.cpp
+++ b/ace/Filecache.cpp
@@ -8,6 +8,7 @@ static const int R_MASK = S_IRUSR|S_IRGRP|S_IROTH;
 static const int W_MASK = S_IRUSR|S_IRGRP|S_IROTH|S_IWUSR|S_IWGRP|S_IWOTH;
 
 #if defined (ACE_WIN32)
+// See if you can get rid of some of these.
 static const int READ_FLAGS = (FILE_FLAG_SEQUENTIAL_SCAN |
                                FILE_FLAG_OVERLAPPED |
                                O_RDONLY);
diff --git a/ace/Filecache.h b/ace/Filecache.h
index 19fc3551cc3..3aff3402e4a 100644
--- a/ace/Filecache.h
+++ b/ace/Filecache.h
@@ -25,114 +25,7 @@
 
 // = Forward declarations.
 class ACE_Filecache;
-
-
-class ACE_Filecache_Object
-  // = TITLE
-  //     Abstraction over a real file.  This is what the Virtual
-  //     Filesystem contains.  This class is not intended for general
-  //     consumption.  Please consult a physician before attempting to
-  //     use this class.
-{
-  friend class ACE_Filecache;
-
-public:
-  ACE_Filecache_Object (const char *filename,
-                        ACE_SYNCH_RW_MUTEX &lock);
-  // Creates a file for reading.
-
-  ACE_Filecache_Object (const char *filename,
-                        int size,
-                        ACE_SYNCH_RW_MUTEX &lock);
-  // Creates a file for writing.
-
-  ~ACE_Filecache_Object (void);
-  // Only if reference count is zero should this be called.
-
-  int acquire (void);
-  // Increment the reference_count_.
-
-  int release (void);
-  // Decrement the reference_count_.
-
-  // = error_ accessors
-  int error (void) const;
-  int error (int error_value,
-	     const char *s = "ACE_Filecache_Object");
-
-  const char *filename (void) const;
-  // filename_ accessor
-
-  ACE_HANDLE handle (void) const;
-  // handle_ accessor.
-
-  void *address (void) const;
-  // Base memory address for memory mapped file.
-
-  size_t size (void) const;
-  // size_ accessor.
-
-  int update (void) const;
-  // True if file on disk is newer than cached file.
-
-protected:
-  ACE_Filecache_Object (void);
-  // Prevent from being called.
-
-  void init (void);
-  // Common initialization code,
-
-private:
-  int error_i (int error_value,
-	       const char *s = "ACE_Filecache_Object");
-  // Internal error logging method, no locking.
-
-public:
-
-  enum 
-  {
-    READING = 1,
-    WRITING = 2
-  };
-
-  enum 
-  {
-    SUCCESS = 0,
-    ACCESS_FAILED,
-    OPEN_FAILED,
-    COPY_FAILED,
-    STAT_FAILED,
-    MEMMAP_FAILED,
-    WRITE_FAILED 
-  };
-
-private:
-  char *tempname_;
-  char filename_[MAXPATHLEN + 1];
-  // The temporary file name and the real file name.  The real file is
-  // copied into the temporary file for safety reasons.
-
-  ACE_Mem_Map mmap_;
-  ACE_HANDLE handle_;
-  // mmap_ holds the memory mapped version of the temporary file.
-  // handle_ is the descriptor to the temporary file.
-
-  struct stat stat_;
-  size_t size_;
-  // Used to compare against the real file to test if an update is needed.
-
-  int action_;
-  int error_;
-  // Status indicators.
-
-  int stale_;
-  // If set to 1, means the object is flagged for removal.
-
-  ACE_SYNCH_RW_MUTEX junklock_;
-  ACE_SYNCH_RW_MUTEX &lock_;
-  // lock_ provides a bookkeeping mechanism for users of this object.
-  // junklock_ is the default initializer
-};
+class ACE_Filecache_Object;
 
 
 class ACE_Export ACE_Filecache_Handle
@@ -321,10 +214,118 @@ private:
   static ACE_Filecache *cvf_;
   // The reference to the instance
 
+  // = Synchronization variables.
   static ACE_SYNCH_RW_MUTEX lock_;
   static ACE_SYNCH_RW_MUTEX hash_lock_[DEFAULT_VIRTUAL_FILESYSTEM_TABLE_SIZE];
   static ACE_SYNCH_RW_MUTEX file_lock_[DEFAULT_VIRTUAL_FILESYSTEM_TABLE_SIZE];
-  // Synchronization variables
 };
 
+class ACE_Filecache_Object
+  // = TITLE
+  //     Abstraction over a real file.  This is what the Virtual
+  //     Filesystem contains.  This class is not intended for general
+  //     consumption.  Please consult a physician before attempting to
+  //     use this class.
+{
+  friend class ACE_Filecache;
+
+public:
+  ACE_Filecache_Object (const char *filename,
+                        ACE_SYNCH_RW_MUTEX &lock);
+  // Creates a file for reading.
+
+  ACE_Filecache_Object (const char *filename,
+                        int size,
+                        ACE_SYNCH_RW_MUTEX &lock);
+  // Creates a file for writing.
+
+  ~ACE_Filecache_Object (void);
+  // Only if reference count is zero should this be called.
+
+  int acquire (void);
+  // Increment the reference_count_.
+
+  int release (void);
+  // Decrement the reference_count_.
+
+  // = error_ accessors
+  int error (void) const;
+  int error (int error_value,
+	     const char *s = "ACE_Filecache_Object");
+
+  const char *filename (void) const;
+  // filename_ accessor
+
+  ACE_HANDLE handle (void) const;
+  // handle_ accessor.
+
+  void *address (void) const;
+  // Base memory address for memory mapped file.
+
+  size_t size (void) const;
+  // size_ accessor.
+
+  int update (void) const;
+  // True if file on disk is newer than cached file.
+
+protected:
+  ACE_Filecache_Object (void);
+  // Prevent from being called.
+
+  void init (void);
+  // Common initialization code,
+
+private:
+  int error_i (int error_value,
+	       const char *s = "ACE_Filecache_Object");
+  // Internal error logging method, no locking.
+
+public:
+
+  enum Creation_States
+  {
+    READING = 1,
+    WRITING = 2
+  };
+
+  enum Error_Conditions
+  {
+    SUCCESS = 0,
+    ACCESS_FAILED,
+    OPEN_FAILED,
+    COPY_FAILED,
+    STAT_FAILED,
+    MEMMAP_FAILED,
+    WRITE_FAILED 
+  };
+
+private:
+  char *tempname_;
+  char filename_[MAXPATHLEN + 1];
+  // The temporary file name and the real file name.  The real file is
+  // copied into the temporary file for safety reasons.
+
+  ACE_Mem_Map mmap_;
+  ACE_HANDLE handle_;
+  // mmap_ holds the memory mapped version of the temporary file.
+  // handle_ is the descriptor to the temporary file.
+
+  struct stat stat_;
+  size_t size_;
+  // Used to compare against the real file to test if an update is needed.
+
+  int action_;
+  int error_;
+  // Status indicators.
+
+  int stale_;
+  // If set to 1, means the object is flagged for removal.
+
+  ACE_SYNCH_RW_MUTEX junklock_;
+  ACE_SYNCH_RW_MUTEX &lock_;
+  // lock_ provides a bookkeeping mechanism for users of this object.
+  // junklock_ is the default initializer
+};
+
+
 #endif /* ACE_FILECACHE_H */
diff --git a/ace/Thread_Manager.cpp b/ace/Thread_Manager.cpp
index 37e89e14f75..ddb5374a288 100644
--- a/ace/Thread_Manager.cpp
+++ b/ace/Thread_Manager.cpp
@@ -445,6 +445,9 @@ ace_thread_manager_adapter (void *args)
 // Call the appropriate OS routine to spawn a thread.  Should *not* be
 // called with the lock_ held...
 
+// HEY!  Consider duping the handles so that users can have something
+// to manipulate if they want.
+
 int 
 ACE_Thread_Manager::spawn_i (ACE_THR_FUNC func, 
 			     void *args, 
@@ -722,6 +725,9 @@ ACE_Thread_Manager::remove_thr (int i)
 
   this->current_count_--;
 
+  // This compaction strategy should be removed so that we can use the
+  // TSS trick.
+
   if (this->current_count_ > 0)
     // Compact the table by moving the last item into the slot vacated
     // by the index being removed (this is a structure assignment).
@@ -1132,6 +1138,9 @@ ACE_Thread_Manager::exit (void *status, int do_thr_exit)
 int
 ACE_Thread_Manager::wait (const ACE_Time_Value *timeout)
 {
+  // HEY!  What we should do is build a table of threads which have
+  // been removed so that we can ``join'' with them later.
+
   ACE_TRACE ("ACE_Thread_Manager::wait");
 
 #if defined (ACE_HAS_THREADS)