Debug stub: associate NaClAppThread with IThread

src/trusted/debug_stub/posix/thread_impl.cc

Index: src/trusted/debug_stub/posix/thread_impl.cc
diff --git a/src/trusted/debug_stub/posix/thread_impl.cc b/src/trusted/debug_stub/posix/thread_impl.cc
index 1d0abc79d0a74ad54400452df2bd0706ab063597..4ba75e7e0b85376f169bf7457f13ffe82d9a6899 100644
--- a/src/trusted/debug_stub/posix/thread_impl.cc
+++ b/src/trusted/debug_stub/posix/thread_impl.cc
@@ -41,7 +41,8 @@ static IThread::ThreadMap_t *ThreadGetMap() {
 
 class Thread : public IThread {
  public:
-  explicit Thread(uint32_t id) : ref_(1), id_(id), state_(DEAD) {}
+  Thread(uint32_t id, struct NaClAppThread *natp)
+      : ref_(1), id_(id), natp_(natp), state_(DEAD) {}
   ~Thread() {}
 
   uint32_t GetId() {
@@ -124,34 +125,42 @@ class Thread : public IThread {
  private:
   uint32_t ref_;
   uint32_t id_;
+  struct NaClAppThread *natp_;
   State  state_;
   struct NaClSignalContext context_;
 
   friend class IThread;
 };
 
-// TODO(mseaborn): This is duplicated in the Windows version.

[Mark Seaborn, 2012/05/07 21:56:49]

This TODO should be kept with Create(), because it still applies.

[eaeltsin, 2012/05/08 11:10:19]

Done.

-IThread* IThread::Acquire(uint32_t id, bool create) {
+IThread* IThread::Create(uint32_t id, struct NaClAppThread* natp) {

[Mark Seaborn, 2012/05/07 21:56:49]

'*' spacing style differs between here and above.  If there's a doubt, I'd
suggest using " *", which is the prevailing NaCl style.

[eaeltsin, 2012/05/08 11:10:19]

But if I change this, it will differ from below (which seems a bit closer as it
is the same function :)) I'll try clean the style in the whole file when
refactoring this in the subsequent changes.

   MutexLock lock(ThreadGetLock());
   Thread* thread;
   ThreadMap_t &map = *ThreadGetMap();
 
-  // Check if we have that thread
   if (map.count(id)) {
-    thread = static_cast<Thread*>(map[id]);
-    thread->ref_++;
-    return thread;
+    NaClLog(LOG_ERROR, "IThread::Create: thread 0x%x already exists\n", id);

[Mark Seaborn, 2012/05/07 21:56:49]

This should be LOG_FATAL because the callers are not checking for NULL.

[eaeltsin, 2012/05/08 11:10:19]

Done.

+    return NULL;
   }
 
-  // If not, can we create it?
-  if (create) {
-    // If not add it to the map
-    thread = new Thread(id);
-    map[id] = thread;
-    return thread;
+  thread = new Thread(id, natp);
+  map[id] = thread;
+  return thread;
+}
+
+// TODO(mseaborn): This is duplicated in the Windows version.
+IThread* IThread::Acquire(uint32_t id) {
+  MutexLock lock(ThreadGetLock());
+  Thread* thread;
+  ThreadMap_t &map = *ThreadGetMap();
+
+  if (map.count(id) == 0) {
+    NaClLog(LOG_ERROR, "IThread::Acquire: thread 0x%x does not exist\n", id);

[Mark Seaborn, 2012/05/07 21:56:49]

Shouldn't this be LOG_FATAL too?  The callers aren't checking for NULL.

[eaeltsin, 2012/05/08 11:10:19]

Done.

+    return NULL;
   }
 
-  return NULL;
+  thread = static_cast<Thread*>(map[id]);
+  thread->ref_++;
+  return thread;
 }
 
 // TODO(mseaborn): This is duplicated in the Windows version.