[PATCH 1/4] hnetcfg: Don't use _GetValue() if text element is missing in get_xml_elements().

[Paul Gofman]

Signed-off-by: Paul Gofman 
---
 dlls/hnetcfg/port.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/dlls/hnetcfg/port.c b/dlls/hnetcfg/port.c
index def77621764..611a652ce5e 100644
--- a/dlls/hnetcfg/port.c
+++ b/dlls/hnetcfg/port.c
@@ -265,7 +265,10 @@ static BOOL get_xml_elements( const char *desc_xml, struct xml_value_desc *value
             if (node_type == XmlNodeType_EndElement) value = L"";
             else                                     goto done;
         }
-        if (FAILED(IXmlReader_GetValue( reader, &value, NULL ))) goto done;
+        else
+        {
+            if (FAILED(IXmlReader_GetValue( reader, &value, NULL ))) goto done;
+        }
         if (values[i].value)
         {
             WARN( "Duplicate value %s.\n", debugstr_w(values[i].name) );

[PATCH 2/4] hnetcfg: Handle NULL strings in update_mapping_list().

[Paul Gofman]

Signed-off-by: Paul Gofman 
---
 dlls/hnetcfg/port.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/dlls/hnetcfg/port.c b/dlls/hnetcfg/port.c
index 611a652ce5e..47bfc16be79 100644
--- a/dlls/hnetcfg/port.c
+++ b/dlls/hnetcfg/port.c
@@ -540,6 +540,22 @@ static LONG long_from_bstr( BSTR s )
     return _wtoi( s );
 }

+static BSTR mapping_move_bstr( BSTR *s )
+{
+    BSTR ret;
+
+    if (*s)
+    {
+        ret = *s;
+        *s = NULL;
+    }
+    else if (!(ret = SysAllocString( L"" )))
+    {
+        ERR( "No memory.\n" );
+    }
+    return ret;
+}
+
 static void update_mapping_list(void)
 {
     struct xml_value_desc mapping_desc[ARRAY_SIZE(port_mapping_template)];
@@ -579,20 +595,17 @@ static void update_mapping_list(void)
             }
             break;
         }
-        new_mappings[index].external_ip = mapping_desc[PM_EXTERNAL_IP].value;
-        mapping_desc[PM_EXTERNAL_IP].value = NULL;
+        new_mappings[index].external_ip = mapping_move_bstr( &mapping_desc[PM_EXTERNAL_IP].value );
         new_mappings[index].external = long_from_bstr( mapping_desc[PM_EXTERNAL].value );
-        new_mappings[index].protocol = mapping_desc[PM_PROTOCOL].value;
-        mapping_desc[PM_PROTOCOL].value = NULL;
+        new_mappings[index].protocol = mapping_move_bstr( &mapping_desc[PM_PROTOCOL].value );
         new_mappings[index].internal = long_from_bstr( mapping_desc[PM_INTERNAL].value );
-        new_mappings[index].client = mapping_desc[PM_CLIENT].value;
-        mapping_desc[PM_CLIENT].value = NULL;
-        if (!wcsicmp( mapping_desc[PM_ENABLED].value, L"true" ) || long_from_bstr( mapping_desc[PM_ENABLED].value ))
+        new_mappings[index].client = mapping_move_bstr( &mapping_desc[PM_CLIENT].value );
+        if (mapping_desc[PM_ENABLED].value && (!wcsicmp( mapping_desc[PM_ENABLED].value, L"true" )
+                                               || long_from_bstr( mapping_desc[PM_ENABLED].value )))
             new_mappings[index].enabled = VARIANT_TRUE;
         else
             new_mappings[index].enabled = VARIANT_FALSE;
-        new_mappings[index].descr = mapping_desc[PM_DESC].value;
-        mapping_desc[PM_DESC].value = NULL;
+        new_mappings[index].descr = mapping_move_bstr( &mapping_desc[PM_DESC].value );

TRACE( "%s %s %s:%u -> %s:%u, enabled %d.\n", debugstr_w(new_mappings[index].descr),
                debugstr_w(new_mappings[index].protocol), debugstr_w(new_mappings[index].external_ip),

[PATCH 3/4] hnetcfg/tests: Don't leak IEnumVARIANT reference in test_static_port_mapping_collection().

[Paul Gofman]

Signed-off-by: Paul Gofman 
---
 dlls/hnetcfg/tests/policy.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/dlls/hnetcfg/tests/policy.c b/dlls/hnetcfg/tests/policy.c
index b6b353ac87d..f344019d1d3 100644
--- a/dlls/hnetcfg/tests/policy.c
+++ b/dlls/hnetcfg/tests/policy.c
@@ -188,6 +188,7 @@ static void test_static_port_mapping_collection( IStaticPortMappingCollection *p

hr = IUnknown_QueryInterface(unk, &IID_IEnumVARIANT, (void **)&enum_ports);
     ok(hr == S_OK, "Got unexpected hr %#x.\n", hr);
+    IUnknown_Release( unk );

refcount2 = get_refcount((IUnknown *)ports);
     ok(refcount2 == refcount, "Got unexpected refcount %u, refcount2 %u.\n", refcount, refcount2);

[PATCH 4/4] hnetcfg/tests: Don't test removing the same mapping twice.

[Paul Gofman]

Signed-off-by: Paul Gofman 
---
     Different routers may return either success or error when deleting nonexistent mapping,
     so no point testing it.

dlls/hnetcfg/tests/policy.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/dlls/hnetcfg/tests/policy.c b/dlls/hnetcfg/tests/policy.c
index f344019d1d3..3b614e6e05a 100644
--- a/dlls/hnetcfg/tests/policy.c
+++ b/dlls/hnetcfg/tests/policy.c
@@ -278,8 +278,6 @@ static void test_static_port_mapping_collection( IStaticPortMappingCollection *p
     hr = IEnumVARIANT_Next(enum_ports, 1, &var, &fetched);
     ok(hr == S_FALSE, "Got unexpected hr %#x.\n", hr);

-    hr = IStaticPortMappingCollection_Remove(ports, 12345, (BSTR)L"UDP");
-    ok(hr == S_OK, "Got unexpected hr %#x.\n", hr);
     hr = IStaticPortMappingCollection_Remove(ports, 12345, (BSTR)L"UDP");
     ok(hr == S_OK, "Got unexpected hr %#x.\n", hr);