Changeset 55580 in vbox for trunk/src/VBox/Additions/common/VBoxService

Timestamp:

May 1, 2015 10:19:37 AM (10 years ago)

Author:

vboxsync

Message:

VBoxServiceControlSessionForkInit: Some minor cleanup and a couple of code review comments.

File:

• trunk/src/VBox/Additions/common/VBoxService/VBoxServiceControlSession.cpp (modified) (9 diffs)

trunk/src/VBox/Additions/common/VBoxService/VBoxServiceControlSession.cpp

@@ -2267 +2267 @@
 
 /**
- * Close all formerly opened guest session threads.
- * Note: Caller is responsible for locking!
+ * Close all open guest session threads.
+ *
+ * @note    Caller is responsible for locking!
  *
  * @return  IPRT status code.
@@ -2284 +2285 @@
             VBoxServiceError("Cancelling pending waits failed; rc=%Rrc\n", rc);*/
 
-    PVBOXSERVICECTRLSESSIONTHREAD pSessionThread
-         = RTListGetFirst(pList, VBOXSERVICECTRLSESSIONTHREAD, Node);
+/** @todo r=bird: Why don't you use RTListForEachSafe here?? */
+    PVBOXSERVICECTRLSESSIONTHREAD pSessionThread = RTListGetFirst(pList, VBOXSERVICECTRLSESSIONTHREAD, Node);
     while (pSessionThread)
     {
         PVBOXSERVICECTRLSESSIONTHREAD pSessionThreadNext =
             RTListGetNext(pList, pSessionThread, VBOXSERVICECTRLSESSIONTHREAD, Node);
-        bool fLast = RTListNodeIsLast(pList, &pSessionThread->Node);
+        bool fLast = RTListNodeIsLast(pList, &pSessionThread->Node); /** @todo r=bird: This isn't necessary, pSessionThreadNext will be NULL! */
 
         int rc2 = GstCntlSessionThreadDestroy(pSessionThread, uFlags);
@@ -2310 +2311 @@
 }
 
+/** @todo r=bird: This isn't a fork in the tranditional unix sense, so please
+ * don't confuse any unix guys by using the term.
+ * GstCntlSessionChildMain would be a good name.  */
 RTEXITCODE VBoxServiceControlSessionForkInit(int argc, char **argv)
 {
@@ -2341 +2345 @@
     g_Session.StartupInfo.uSessionID = UINT32_MAX;
 
-    int rc = VINF_SUCCESS;
-
-    while (   (ch = RTGetOpt(&GetState, &ValueUnion))
-           && RT_SUCCESS(rc))
+    while ((ch = RTGetOpt(&GetState, &ValueUnion)) != 0)
     {
         /* For options that require an argument, ValueUnion has received the value. */
@@ -2350 +2351 @@
         {
             case VBOXSERVICESESSIONOPT_LOG_FILE:
-                rc = RTStrCopy(g_szLogFile, sizeof(g_szLogFile), ValueUnion.psz);
+            {
+                int rc = RTStrCopy(g_szLogFile, sizeof(g_szLogFile), ValueUnion.psz);
                 if (RT_FAILURE(rc))
                     return RTMsgErrorExit(RTEXITCODE_FAILURE, "Error copying log file name: %Rrc", rc);
                 break;
+            }
 #ifdef DEBUG
             case VBOXSERVICESESSIONOPT_DUMP_STDOUT:
@@ -2364 +2367 @@
 #endif
             case VBOXSERVICESESSIONOPT_USERNAME:
-                /** @todo Information not needed right now, skip. */
+                /* Information not needed right now, skip. */
                 break;
 
@@ -2377 +2380 @@
 #ifdef DEBUG
             case VBOXSERVICESESSIONOPT_THREAD_ID:
-                /* Not handled. */
+                /* Not handled. Mainly for processs listing. */
                 break;
 #endif
@@ -2388 +2391 @@
             case VINF_GETOPT_NOT_OPTION:
                 /* Ignore; might be "guestsession" main command. */
+                /** @todo r=bird: We DO NOT ignore stuff on the command line! */
                 break;
 
             default:
                 return RTMsgErrorExit(RTEXITCODE_SYNTAX, "Unknown command '%s'", ValueUnion.psz);
-                break; /* Never reached. */
-        }
-    }
-
-    if (RT_FAILURE(rc))
-        return RTMsgErrorExit(RTEXITCODE_FAILURE, "Initialization failed with rc=%Rrc", rc);
-
+        }
+    }
+
+    /* Check that we've got all the required options. */
     if (g_Session.StartupInfo.uProtocol == UINT32_MAX)
         return RTMsgErrorExit(RTEXITCODE_SYNTAX, "No protocol version specified");
@@ -2406 +2407 @@
 
     /* Init the session object. */
-    rc = GstCntlSessionInit(&g_Session, uSessionFlags);
+    int rc = GstCntlSessionInit(&g_Session, uSessionFlags);
     if (RT_FAILURE(rc))
         return RTMsgErrorExit(RTEXITCODE_INIT, "Failed to initialize session object, rc=%Rrc\n", rc);
 
-    rc = VBoxServiceLogCreate(strlen(g_szLogFile) ? g_szLogFile : NULL);
+    rc = VBoxServiceLogCreate(g_szLogFile[0] ? g_szLogFile : NULL);
     if (RT_FAILURE(rc))
         return RTMsgErrorExit(RTEXITCODE_INIT, "Failed to create log file \"%s\", rc=%Rrc\n",
-                              strlen(g_szLogFile) ? g_szLogFile : "<None>", rc);
+                              g_szLogFile[0] ? g_szLogFile : "<None>", rc);
 
     RTEXITCODE rcExit = gstcntlSessionForkWorker(&g_Session);