Changeset 85264 in vbox for trunk/src/VBox/Main/src-server

Timestamp:

Jul 12, 2020 12:56:45 AM (5 years ago)

Author:

vboxsync

Message:

Main/NetIf-generic.cpp: Made NetIfAdpCtl() return failure when the exit code is non-zero or it terminated abnormally (judging from the error messages, this is what the callers expects). Tried to untangle status code mess in NetIfCreateHostOnlyNetworkInterface(), adding missing error handling for popen() and adjusting some of the copy&paste error messages a little. NetIfRemoveHostOnlyNetworkInterface() should not call i_notifyComplete on an uninitialized progress object, and the return of Progress::init() is not a vbox status code that this function can return unconverted. createObject can fail. bugref:9790

File:

• trunk/src/VBox/Main/src-server/generic/NetIf-generic.cpp (modified) (11 diffs)

trunk/src/VBox/Main/src-server/generic/NetIf-generic.cpp

@@ -39 +39 @@
 #include "ProgressImpl.h"
 #include "VirtualBoxImpl.h"
+#include "Global.h"
 #include "netif.h"
 
@@ -69 +70 @@
         RTPROCSTATUS Status;
         rc = RTProcWait(pid, 0, &Status);
-        if (   RT_SUCCESS(rc)
-            && Status.iStatus == 0
-            && Status.enmReason == RTPROCEXITREASON_NORMAL)
-            return VINF_SUCCESS;
+        if (RT_SUCCESS(rc))
+        {
+            if (   Status.iStatus == 0
+                && Status.enmReason == RTPROCEXITREASON_NORMAL)
+                return VINF_SUCCESS;
+            LogRel(("NetIfAdpCtl: failed to create process for %s: iStats=%d enmReason=%d\n",
+                    szAdpCtl, Status.iStatus, Status.enmReason));
+            rc = VERR_GENERAL_FAILURE;
+        }
     }
     else
@@ -188 +194 @@
     /* create a progress object */
     ComObjPtr<Progress> progress;
-    progress.createObject();
-
+    HRESULT hrc = progress.createObject();
+    AssertComRCReturn(hrc, Global::vboxStatusCodeFromCOM(hrc));
+
+    /* Note vrc and hrc are competing about tracking the error state here. */
+    int vrc = VINF_SUCCESS;
     ComPtr<IHost> host;
     HRESULT hrc = pVirtualBox->COMGETTER(Host)(host.asOutParam());
@@ -202 +211 @@
 
             char szAdpCtl[RTPATH_MAX];
-            int rc = RTPathExecDir(szAdpCtl, sizeof(szAdpCtl) - sizeof("/" VBOXNETADPCTL_NAME " add"));
-            if (RT_FAILURE(rc))
+            vrc = RTPathExecDir(szAdpCtl, sizeof(szAdpCtl) - sizeof("/" VBOXNETADPCTL_NAME " add"));
+            if (RT_FAILURE(vrc))
             {
                 progress->i_notifyComplete(E_FAIL,
                                            COM_IIDOF(IHostNetworkInterface),
                                            HostNetworkInterface::getStaticComponentName(),
-                                           "Failed to get program path, rc=%Rrc\n", rc);
-                return rc;
+                                           "Failed to get program path, vrc=%Rrc\n", vrc);
+                return vrc;
             }
             strcat(szAdpCtl, "/" VBOXNETADPCTL_NAME " ");
@@ -221 +230 @@
             if (strlen(szAdpCtl) < RTPATH_MAX - sizeof(" 2>&1"))
                 strcat(szAdpCtl, " 2>&1");
+
             FILE *fp = popen(szAdpCtl, "r");
-
             if (fp)
             {
@@ -240 +249 @@
                                                    "%s", szBuf);
                         pclose(fp);
-                        return E_FAIL;
+                        return Global::vboxStatusCodeFromCOM(E_FAIL);
                     }
 
@@ -246 +255 @@
                     PNETIFINFO pInfo = (PNETIFINFO)RTMemAllocZ(RT_UOFFSETOF_DYN(NETIFINFO, szName[cbNameLen]));
                     if (!pInfo)
-                        rc = VERR_NO_MEMORY;
+                        vrc = VERR_NO_MEMORY;
                     else
                     {
                         strcpy(pInfo->szShortName, szBuf);
                         strcpy(pInfo->szName, szBuf);
-                        rc = NetIfGetConfigByName(pInfo);
-                        if (RT_FAILURE(rc))
+                        vrc = NetIfGetConfigByName(pInfo);
+                        if (RT_FAILURE(vrc))
                         {
                             progress->i_notifyComplete(E_FAIL,
@@ -271 +280 @@
                         RTMemFree(pInfo);
                     }
-                    if ((rc = pclose(fp)) != 0)
+                    if ((vrc = pclose(fp)) != 0)
                     {
                         progress->i_notifyComplete(E_FAIL,
                                                    COM_IIDOF(IHostNetworkInterface),
                                                    HostNetworkInterface::getStaticComponentName(),
-                                                   "Failed to execute '" VBOXNETADPCTL_NAME " add' (exit status: %d)", rc);
-                        rc = VERR_INTERNAL_ERROR;
+                                                   "Failed to execute '%s' - exit status: %d", szAdpCtl, vrc);
+                        vrc = VERR_INTERNAL_ERROR;
                     }
                 }
@@ -283 +292 @@
                 {
                     /* Failed to add an interface */
-                    rc = VERR_PERMISSION_DENIED;
                     progress->i_notifyComplete(E_FAIL,
                                                COM_IIDOF(IHostNetworkInterface),
                                                HostNetworkInterface::getStaticComponentName(),
-                                               "Failed to execute '" VBOXNETADPCTL_NAME " add' (exit status: %d). Check permissions!", rc);
+                                               "Failed to execute '%s' (errno %d). Check permissions!",
+                                               szAdpCtl, errno);
                     pclose(fp);
+                    vrc = VERR_PERMISSION_DENIED;
                 }
             }
-            if (RT_SUCCESS(rc))
-                progress->i_notifyComplete(rc);
+            else
+            {
+                vrc = RTErrConvertFromErrno(errno);
+                progress->i_notifyComplete(E_FAIL,
+                                           COM_IIDOF(IHostNetworkInterface),
+                                           HostNetworkInterface::getStaticComponentName(),
+                                           "Failed to execute '%s' (errno %d / %Rrc). Check permissions!",
+                                           szAdpCtl, errno, vrc);
+            }
+            if (RT_SUCCESS(vrc))
+                progress->i_notifyComplete(S_OK);
             else
                 hrc = E_FAIL;
@@ -298 +317 @@
     }
 
-    return hrc;
+    return RT_FAILURE(vrc) ? vrc : SUCCEEDED(hrc) ? VINF_SUCCESS : Global::vboxStatusCodeFromCOM(hrc);
 
 #else
@@ -315 +334 @@
     /* create a progress object */
     ComObjPtr<Progress> progress;
-    progress.createObject();
+    HRESULT hrc = progress.createObject();
+    AssertComRCReturn(hrc, Global::vboxStatusCodeFromCOM(hrc));
+
     ComPtr<IHost> host;
-    int rc = VINF_SUCCESS;
-    HRESULT hr = pVirtualBox->COMGETTER(Host)(host.asOutParam());
-    if (SUCCEEDED(hr))
-    {
-        Bstr ifname;
+    int vrc = VINF_SUCCESS;
+    hrc = pVirtualBox->COMGETTER(Host)(host.asOutParam());
+    if (SUCCEEDED(hrc))
+    {
         ComPtr<IHostNetworkInterface> iface;
         if (FAILED(host->FindHostNetworkInterfaceById(aId.toUtf16().raw(), iface.asOutParam())))
             return VERR_INVALID_PARAMETER;
+
+        Bstr ifname;
         iface->COMGETTER(Name)(ifname.asOutParam());
         if (ifname.isEmpty())
             return VERR_INTERNAL_ERROR;
-
-        rc = progress->init(pVirtualBox, host,
-                            "Removing host network interface",
-                            FALSE /* aCancelable */);
-        if (SUCCEEDED(rc))
+        Utf8Str strIfName(ifname);
+
+        hrc = progress->init(pVirtualBox, host, "Removing host network interface", FALSE /* aCancelable */);
+        if (SUCCEEDED(hrc))
         {
             progress.queryInterfaceTo(aProgress);
-            rc = NetIfAdpCtl(Utf8Str(ifname).c_str(), "remove", NULL, NULL);
-            if (RT_FAILURE(rc))
+            vrc = NetIfAdpCtl(strIfName.c_str(), "remove", NULL, NULL);
+            if (RT_FAILURE(vrc))
                 progress->i_notifyComplete(E_FAIL,
                                            COM_IIDOF(IHostNetworkInterface),
                                            HostNetworkInterface::getStaticComponentName(),
-                                           "Failed to execute '" VBOXNETADPCTL_NAME "' (exit status: %d)", rc);
+                                           "Failed to execute '" VBOXNETADPCTL_NAME " %s remove' (%Rrc)",
+                                           strIfName.c_str(), vrc);
             else
                 progress->i_notifyComplete(S_OK);
         }
-    }
-    else
-    {
-        progress->i_notifyComplete(hr);
-        rc = VERR_INTERNAL_ERROR;
-    }
-    return rc;
+        else
+            vrc = Global::vboxStatusCodeFromCOM(hrc);
+    }
+    else
+        vrc = Global::vboxStatusCodeFromCOM(hrc);
+    return vrc;
 #else
     NOREF(pVirtualBox);