[libvirt] [PATCH V11 3/7] nwfilter: Fix support for trusted DHCP servers

[Daniel Veillard]

On Thu, Apr 19, 2012 at 06:30:59AM -0400, Stefan Berger wrote:
> On 04/19/2012 05:27 AM, Daniel Veillard wrote:
> >On Tue, Apr 17, 2012 at 10:44:04AM -0400, Stefan Berger wrote:
> >>Fix the support for trusted DHCP server in the ebtables code's
> >>hard-coded function applying DHCP only filtering rules:
> >>Rather than using a char * use the more flexible
> >>virNWFilterVarValuePtr that contains the trusted DHCP server(s)
> >>IP address. Process all entries.
> >>
> >>Since all callers so far provided NULL as parameter, no changes
> >>are necessary in any other code.
> >>
> [...]
> >>+    while (true) {
> >>+        char *srcIPParam = NULL;
> >>+
> >>+        if (idx<  num_dhcpsrvrs) {
> >>+            const char *dhcpserver;
> >>+
> >>+            dhcpserver = virNWFilterVarValueGetNthValue(dhcpsrvrs, idx);
> >>+
> >>+            if (virAsprintf(&srcIPParam, "--ip-src %s", dhcpserver)<  0) {
> >>+                virReportOOMError();
> >>+                goto tear_down_tmpebchains;
> >>+            }
> >>+        }
> >>+
> >>+        virBufferAsprintf(&buf,
> >>+                          CMD_DEF("$EBT -t nat -A %s"
> >>+                                  " -d %s"
> >>+                                  " -p ipv4 --ip-protocol udp"
> >>+                                  " %s"
> >>+                                  " --ip-sport 67 --ip-dport 68"
> >>+                                  " -j ACCEPT") CMD_SEPARATOR
> >>+                          CMD_EXEC
> >>+                          "%s",
> >>+
> >>+                          chain_out,
> >>+                          macaddr_str,
> >>+                          srcIPParam != NULL ? srcIPParam : "",
> >>+                          CMD_STOPONERR(1));
> >>+
> >>+        VIR_FREE(srcIPParam);
> >>+
> >>+        if (idx == num_dhcpsrvrs)
> >>+            break;
> >>+
> >>+        idx++;
> >>+    }
> >   There is something I don't understand in that loop, you repetedly
> >write to buf, but you don't seems to use buf in the loop. This looks
> >fishy to me, or are you using side effect execution in the Asprintf
> >argument evaluation. Too cryptic to my taste, I'm lost !
> 
> I am accumulating (shell) commands in the buffer and then test the
> buffer for error before finally executing the commands.

  Okay, virBufferAsprintf contrary to virAsprintf accumulates the
  writes, makes sense now ...

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/