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

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.