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

Timestamp:

Jul 21, 2010 1:07:34 PM (15 years ago)

Author:

vboxsync

Message:

VBoxService: Review comments for Andy. Changed -pagefusionfork to --pagefusionfork and removed the negative comments as this is the best approach and we use it in several other places as well (VBoxSVC comes to mind). Some nits.

Location:

trunk/src/VBox/Additions/common/VBoxService

Files:

• VBoxService.cpp (modified) (4 diffs)
• VBoxServicePageSharing.cpp (modified) (1 diff)

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

@@ -496 +496 @@
 int main(int argc, char **argv)
 {
-    int rc = VINF_SUCCESS;
     /*
      * Init globals and such.
@@ -503 +502 @@
 
     /*
-     * Connect to the kernel part before daemonizing so we can fail
-     * and complain if there is some kind of problem. We need to initialize
-     * the guest lib *before* we do the pre-init just in case one of services
-     * needs do to some initial stuff with it.
+     * Connect to the kernel part before daemonizing so we can fail and
+     * complain if there is some kind of problem.  We need to initialize the
+     * guest lib *before* we do the pre-init just in case one of services needs
+     * do to some initial stuff with it.
      */
     VBoxServiceVerbose(2, "Calling VbgR3Init()\n");
-    rc = VbglR3Init();
+    int rc = VbglR3Init();
     if (RT_FAILURE(rc))
         return VBoxServiceError("VbglR3Init failed with rc=%Rrc.\n", rc);
 
 #ifdef RT_OS_WINDOWS
-    /* Special forked VBoxService.exe process for handling page fusion. */
-    /* Note: ugly hack, but annoying to install additional executables. */
+    /*
+     * Check if we're the specially spawned VBoxService.exe process that
+     * handles page fusion.  This saves an extra executable.
+     */
     if (    argc == 2
-        &&  !strcmp(argv[1], "-pagefusionfork"))
-    {
+        &&  !strcmp(argv[1], "--pagefusionfork"))
         return VBoxServicePageSharingInitFork();
-    }
-#endif
-
-    /* Do pre-init of services. */
+#endif
+
+    /*
+     * Do pre-init of services.
+     */
     g_pszProgName = RTPathFilename(argv[0]);
     for (unsigned j = 0; j < RT_ELEMENTS(g_aServices); j++)
@@ -533 +534 @@
 
 #ifdef RT_OS_WINDOWS
-    /* Make sure only one instance of VBoxService runs at a time. Create a global mutex for that.
-       Do not use a global namespace ("Global\\") for mutex name here, will blow up NT4 compatibility! */
-    HANDLE hMutexAppRunning = CreateMutex (NULL, FALSE, VBOXSERVICE_NAME);
+    /*
+     * Make sure only one instance of VBoxService runs at a time.  Create a
+     * global mutex for that.  Do not use a global namespace ("Global\\") for
+     * mutex name here, will blow up NT4 compatibility!
+     */
+    /** @todo r=bird: Use Global\\ prefix or this serves no purpose on terminal servers. */
+    HANDLE hMutexAppRunning = CreateMutex(NULL, FALSE, VBOXSERVICE_NAME);
     if (   hMutexAppRunning != NULL
         && GetLastError() == ERROR_ALREADY_EXISTS)
@@ -544 +549 @@
         CloseHandle(hMutexAppRunning);
         hMutexAppRunning = NULL;
+
+        /** @todo r=bird: How does this cause us to terminate?  Btw. Why do
+         *        we do this before parsing parameters?  'VBoxService --help'
+         *        and 'VBoxService --version' won't work now when the service
+         *        is running... */
     }
 #endif

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

@@ -681 +681 @@
         VBoxServiceVerbose(3, "VBoxServicePageSharingWorkerProcess: enabled=%d\n", fEnabled);
 
+        /*
+         * Start a 2nd VBoxService process to deal with page fusion as we do
+         * not wish to dummy load dlls into this process.  (First load with
+         * DONT_RESOLVE_DLL_REFERENCES, 2nd normal -> dll init routines not called!)
+         */
         if (    fEnabled
             &&  hProcess == NIL_RTPROCESS)
         {
             char szExeName[256];
-            char *pszExeName;
-            char *pszArgs[3];
-
-            pszExeName = RTProcGetExecutableName(szExeName, sizeof(szExeName));
-
+            char *pszExeName = RTProcGetExecutableName(szExeName, sizeof(szExeName));
             if (pszExeName)
             {
+                char const *papszArgs[3];
                 pszArgs[0] = pszExeName;
-                pszArgs[1] = "-pagefusionfork";
+                pszArgs[1] = "--pagefusionfork";
                 pszArgs[2] = NULL;
-                /* Start a 2nd VBoxService process to deal with page fusion as we do not wish to dummy load
-                 * dlls into this process. (first load with DONT_RESOLVE_DLL_REFERENCES, 2nd normal -> dll init routines not called!)
-                 */
-                rc = RTProcCreate(pszExeName, pszArgs, RTENV_DEFAULT, 0 /* normal child */, &hProcess);
-                if (rc != VINF_SUCCESS)
+                rc = RTProcCreate(pszExeName, papszArgs, RTENV_DEFAULT, 0 /* normal child */, &hProcess);
+                if (RT_FAILURE(rc))
                     VBoxServiceError("RTProcCreate %s failed; rc=%Rrc\n", pszExeName, rc);
             }