[Openvpn-devel] [M] Change in openvpn[master]: tapctl: make output of 'list' and 'create' commands more verbose

[stipa (C. Review) <ge...@op...>]

Attention is currently required from: cron2, plaisthos, stipa.

Hello cron2, plaisthos, 

I'd like you to reexamine a change. Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1372?usp=email

to look at the new patch set (#2).

The following approvals got outdated and were removed:
Code-Review-1 by cron2


Change subject: tapctl: make output of 'list' and 'create' commands more verbose
......................................................................

tapctl: make output of 'list' and 'create' commands more verbose

Print adapter GUID, name and hwid.

Change-Id: Iac6bcf8b5434aa414e86cb4b9742e7dd591dc796
Signed-off-by: Lev Stipakov <le...@op...>
---
M src/tapctl/main.c
1 file changed, 35 insertions(+), 20 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/72/1372/2

diff --git a/src/tapctl/main.c b/src/tapctl/main.c
index 4d25998..15c25ae 100644
--- a/src/tapctl/main.c
+++ b/src/tapctl/main.c
@@ -80,7 +80,8 @@
     L"\n"
     L"Output:\n"
     L"\n"
-    L"This command prints newly created VPN network adapter's GUID to stdout.        \n"
+    L"This command prints newly created VPN network adapter's GUID, name and         \n"
+    L"hardware ID to stdout.                                                         \n"
 ;
 
 static const WCHAR usage_message_list[] =
@@ -99,7 +100,7 @@
     L"\n"
     L"Output:\n"
     L"\n"
-    L"This command prints all VPN network adapters to stdout.                        \n"
+    L"This command prints VPN network adapter GUID, name and hardware ID to stdout.  \n"
 ;
 
 static const WCHAR usage_message_delete[] =
@@ -211,10 +212,12 @@
     LPCWSTR szName = NULL;
     LPCWSTR szHwId = L"root\\" _L(TAP_WIN_COMPONENT_ID);
     struct tap_adapter_node *adapter_list = NULL;
-    LPWSTR adapter_name = NULL;
+    LPWSTR rename_name = NULL;
+    LPWSTR final_name = NULL;
     LPOLESTR adapter_id = NULL;
     GUID guidAdapter;
     int result = 1;
+    BOOL delete_created_adapter = FALSE;
 
     for (int i = 2; i < argc; i++)
     {
@@ -247,45 +250,56 @@
     if (dwResult != ERROR_SUCCESS)
     {
         fwprintf(stderr, L"Enumerating adapters failed (error 0x%x).\n", dwResult);
-        tap_delete_adapter(NULL, &guidAdapter, bRebootRequired);
-        return result;
+        delete_created_adapter = TRUE;
+        goto cleanup;
     }
 
-    adapter_name = szName ? wcsdup(szName) : get_unique_adapter_name(szHwId, adapter_list);
-    if (adapter_name)
+    rename_name = szName ? wcsdup(szName) : get_unique_adapter_name(szHwId, adapter_list);
+    if (rename_name)
     {
-        if (szName && !is_adapter_name_available(adapter_name, adapter_list, TRUE))
+        if (szName && !is_adapter_name_available(rename_name, adapter_list, TRUE))
         {
-            tap_delete_adapter(NULL, &guidAdapter, bRebootRequired);
+            delete_created_adapter = TRUE;
             goto cleanup;
         }
 
-        dwResult = tap_set_adapter_name(&guidAdapter, adapter_name, FALSE);
-        if (dwResult != ERROR_SUCCESS)
+        dwResult = tap_set_adapter_name(&guidAdapter, rename_name, FALSE);
+        if (dwResult == ERROR_SUCCESS)
+        {
+            final_name = rename_name;
+            rename_name = NULL;
+        }
+        else
         {
             StringFromIID((REFIID)&guidAdapter, &adapter_id);
             fwprintf(stderr,
                      L"Renaming TUN/TAP adapter %ls"
                      L" to \"%ls\" failed (error 0x%x).\n",
-                     adapter_id, adapter_name, dwResult);
+                     adapter_id, rename_name, dwResult);
             CoTaskMemFree(adapter_id);
-            goto cleanup;
         }
     }
 
     result = 0;
 
 cleanup:
-    free(adapter_name);
     tap_free_adapter_list(adapter_list);
+    free(rename_name);
 
-    if (result == 0)
+    if (result == 0 && final_name)
     {
         StringFromIID((REFIID)&guidAdapter, &adapter_id);
-        fwprintf(stdout, L"%ls\n", adapter_id);
+        fwprintf(stdout, L"%ls\t%ls\t%ls\n", adapter_id, final_name, szHwId ? szHwId : L"");
         CoTaskMemFree(adapter_id);
     }
 
+    free(final_name);
+
+    if (result != 0 && delete_created_adapter)
+    {
+        tap_delete_adapter(NULL, &guidAdapter, bRebootRequired);
+    }
+
     return result;
 }
 
@@ -327,10 +341,11 @@
     {
         LPOLESTR adapter_id = NULL;
         StringFromIID((REFIID)&adapter->guid, &adapter_id);
-        fwprintf(stdout,
-                 L"%ls\t%"
-                 L"ls\n",
-                 adapter_id, adapter->szName);
+        const WCHAR *name = adapter->szName ? adapter->szName : L"";
+        const WCHAR *hwid = (adapter->szzHardwareIDs && adapter->szzHardwareIDs[0])
+                                ? adapter->szzHardwareIDs
+                                : L"";
+        fwprintf(stdout, L"%ls\t%ls\t%ls\n", adapter_id, name, hwid);
         CoTaskMemFree(adapter_id);
     }
 

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1372?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Iac6bcf8b5434aa414e86cb4b9742e7dd591dc796
Gerrit-Change-Number: 1372
Gerrit-PatchSet: 2
Gerrit-Owner: stipa <lst...@gm...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: cron2 <ge...@gr...>
Gerrit-Attention: stipa <lst...@gm...>