[libvirt] [PATCH 0/6] Misc cleanups to nwfilter code
This mini-series performs a few style cleanups on the nwfilter code. There should be no functional change in any of these patches. Daniel P. Berrange (6): Change 'int incoming' to 'bool incoming' in nwfilter code Remove pointless brackets around boolean Remove 'int stopOnError' parameters in nwfilter methods Remove pointless return values in nwfilter methods Change 'int isTempChain' to bool in nwfilter Change CMD_STOPONERR(1) to use true src/nwfilter/nwfilter_ebiptables_driver.c | 523 ++++++++++++++---------------- 1 file changed, 241 insertions(+), 282 deletions(-) -- 1.8.5.3
Many methods in the nwfilter code have an 'int incoming' parameter that only takes 0 or 1, so should use a bool instead. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/nwfilter/nwfilter_ebiptables_driver.c | 187 +++++++++++++++--------------- 1 file changed, 94 insertions(+), 93 deletions(-) diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 74a1f9d..2543854 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -642,7 +642,7 @@ static int iptablesCreateBaseChains(virBufferPtr buf) static int iptablesCreateTmpRootChain(virBufferPtr buf, char prefix, - int incoming, const char *ifname, + bool incoming, const char *ifname, int stopOnError) { char chain[MAX_CHAINNAME_LENGTH]; @@ -669,9 +669,9 @@ static int iptablesCreateTmpRootChains(virBufferPtr buf, const char *ifname) { - iptablesCreateTmpRootChain(buf, 'F', 0, ifname, 1); - iptablesCreateTmpRootChain(buf, 'F', 1, ifname, 1); - iptablesCreateTmpRootChain(buf, 'H', 1, ifname, 1); + iptablesCreateTmpRootChain(buf, 'F', false, ifname, 1); + iptablesCreateTmpRootChain(buf, 'F', true, ifname, 1); + iptablesCreateTmpRootChain(buf, 'H', true, ifname, 1); return 0; } @@ -679,7 +679,7 @@ iptablesCreateTmpRootChains(virBufferPtr buf, static int _iptablesRemoveRootChain(virBufferPtr buf, char prefix, - int incoming, const char *ifname, + bool incoming, const char *ifname, int isTempChain) { char chain[MAX_CHAINNAME_LENGTH]; @@ -709,7 +709,7 @@ _iptablesRemoveRootChain(virBufferPtr buf, static int iptablesRemoveRootChain(virBufferPtr buf, char prefix, - int incoming, + bool incoming, const char *ifname) { return _iptablesRemoveRootChain(buf, prefix, incoming, ifname, 0); @@ -719,7 +719,7 @@ iptablesRemoveRootChain(virBufferPtr buf, static int iptablesRemoveTmpRootChain(virBufferPtr buf, char prefix, - int incoming, + bool incoming, const char *ifname) { return _iptablesRemoveRootChain(buf, prefix, @@ -731,9 +731,9 @@ static int iptablesRemoveTmpRootChains(virBufferPtr buf, const char *ifname) { - iptablesRemoveTmpRootChain(buf, 'F', 0, ifname); - iptablesRemoveTmpRootChain(buf, 'F', 1, ifname); - iptablesRemoveTmpRootChain(buf, 'H', 1, ifname); + iptablesRemoveTmpRootChain(buf, 'F', false, ifname); + iptablesRemoveTmpRootChain(buf, 'F', true, ifname); + iptablesRemoveTmpRootChain(buf, 'H', true, ifname); return 0; } @@ -742,9 +742,9 @@ static int iptablesRemoveRootChains(virBufferPtr buf, const char *ifname) { - iptablesRemoveRootChain(buf, 'F', 0, ifname); - iptablesRemoveRootChain(buf, 'F', 1, ifname); - iptablesRemoveRootChain(buf, 'H', 1, ifname); + iptablesRemoveRootChain(buf, 'F', false, ifname); + iptablesRemoveRootChain(buf, 'F', true, ifname); + iptablesRemoveRootChain(buf, 'H', true, ifname); return 0; } @@ -753,7 +753,7 @@ static int iptablesLinkTmpRootChain(virBufferPtr buf, const char *basechain, char prefix, - int incoming, const char *ifname, + bool incoming, const char *ifname, int stopOnError) { char chain[MAX_CHAINNAME_LENGTH]; @@ -785,9 +785,9 @@ static int iptablesLinkTmpRootChains(virBufferPtr buf, const char *ifname) { - iptablesLinkTmpRootChain(buf, VIRT_OUT_CHAIN, 'F', 0, ifname, 1); - iptablesLinkTmpRootChain(buf, VIRT_IN_CHAIN, 'F', 1, ifname, 1); - iptablesLinkTmpRootChain(buf, HOST_IN_CHAIN, 'H', 1, ifname, 1); + iptablesLinkTmpRootChain(buf, VIRT_OUT_CHAIN, 'F', false, ifname, 1); + iptablesLinkTmpRootChain(buf, VIRT_IN_CHAIN, 'F', true, ifname, 1); + iptablesLinkTmpRootChain(buf, HOST_IN_CHAIN, 'H', true, ifname, 1); return 0; } @@ -831,7 +831,7 @@ static int _iptablesUnlinkRootChain(virBufferPtr buf, const char *basechain, char prefix, - int incoming, const char *ifname, + bool incoming, const char *ifname, int isTempChain) { char chain[MAX_CHAINNAME_LENGTH]; @@ -877,7 +877,7 @@ static int iptablesUnlinkRootChain(virBufferPtr buf, const char *basechain, char prefix, - int incoming, const char *ifname) + bool incoming, const char *ifname) { return _iptablesUnlinkRootChain(buf, basechain, prefix, incoming, ifname, 0); @@ -888,7 +888,7 @@ static int iptablesUnlinkTmpRootChain(virBufferPtr buf, const char *basechain, char prefix, - int incoming, const char *ifname) + bool incoming, const char *ifname) { return _iptablesUnlinkRootChain(buf, basechain, prefix, incoming, ifname, 1); @@ -899,9 +899,9 @@ static int iptablesUnlinkRootChains(virBufferPtr buf, const char *ifname) { - iptablesUnlinkRootChain(buf, VIRT_OUT_CHAIN, 'F', 0, ifname); - iptablesUnlinkRootChain(buf, VIRT_IN_CHAIN, 'F', 1, ifname); - iptablesUnlinkRootChain(buf, HOST_IN_CHAIN, 'H', 1, ifname); + iptablesUnlinkRootChain(buf, VIRT_OUT_CHAIN, 'F', false, ifname); + iptablesUnlinkRootChain(buf, VIRT_IN_CHAIN, 'F', true, ifname); + iptablesUnlinkRootChain(buf, HOST_IN_CHAIN, 'H', true, ifname); return 0; } @@ -911,9 +911,9 @@ static int iptablesUnlinkTmpRootChains(virBufferPtr buf, const char *ifname) { - iptablesUnlinkTmpRootChain(buf, VIRT_OUT_CHAIN, 'F', 0, ifname); - iptablesUnlinkTmpRootChain(buf, VIRT_IN_CHAIN, 'F', 1, ifname); - iptablesUnlinkTmpRootChain(buf, HOST_IN_CHAIN, 'H', 1, ifname); + iptablesUnlinkTmpRootChain(buf, VIRT_OUT_CHAIN, 'F', false, ifname); + iptablesUnlinkTmpRootChain(buf, VIRT_IN_CHAIN, 'F', true, ifname); + iptablesUnlinkTmpRootChain(buf, HOST_IN_CHAIN, 'H', true, ifname); return 0; } @@ -921,7 +921,7 @@ iptablesUnlinkTmpRootChains(virBufferPtr buf, static int iptablesRenameTmpRootChain(virBufferPtr buf, char prefix, - int incoming, + bool incoming, const char *ifname) { char tmpchain[MAX_CHAINNAME_LENGTH], chain[MAX_CHAINNAME_LENGTH]; @@ -951,9 +951,9 @@ static int iptablesRenameTmpRootChains(virBufferPtr buf, const char *ifname) { - iptablesRenameTmpRootChain(buf, 'F', 0, ifname); - iptablesRenameTmpRootChain(buf, 'F', 1, ifname); - iptablesRenameTmpRootChain(buf, 'H', 1, ifname); + iptablesRenameTmpRootChain(buf, 'F', false, ifname); + iptablesRenameTmpRootChain(buf, 'F', true, ifname); + iptablesRenameTmpRootChain(buf, 'H', true, ifname); return 0; } @@ -2869,7 +2869,7 @@ ebiptablesExecCLI(virBufferPtr buf, bool ignoreNonzero, char **outbuf) static int ebtablesCreateTmpRootChain(virBufferPtr buf, - int incoming, const char *ifname, + bool incoming, const char *ifname, int stopOnError) { char chain[MAX_CHAINNAME_LENGTH]; @@ -2891,7 +2891,7 @@ ebtablesCreateTmpRootChain(virBufferPtr buf, static int ebtablesLinkTmpRootChain(virBufferPtr buf, - int incoming, const char *ifname, + bool incoming, const char *ifname, int stopOnError) { char chain[MAX_CHAINNAME_LENGTH]; @@ -2917,7 +2917,7 @@ ebtablesLinkTmpRootChain(virBufferPtr buf, static int _ebtablesRemoveRootChain(virBufferPtr buf, - int incoming, const char *ifname, + bool incoming, const char *ifname, int isTempChain) { char chain[MAX_CHAINNAME_LENGTH]; @@ -2943,7 +2943,7 @@ _ebtablesRemoveRootChain(virBufferPtr buf, static int ebtablesRemoveRootChain(virBufferPtr buf, - int incoming, const char *ifname) + bool incoming, const char *ifname) { return _ebtablesRemoveRootChain(buf, incoming, ifname, 0); } @@ -2951,7 +2951,7 @@ ebtablesRemoveRootChain(virBufferPtr buf, static int ebtablesRemoveTmpRootChain(virBufferPtr buf, - int incoming, const char *ifname) + bool incoming, const char *ifname) { return _ebtablesRemoveRootChain(buf, incoming, ifname, 1); } @@ -2959,7 +2959,7 @@ ebtablesRemoveTmpRootChain(virBufferPtr buf, static int _ebtablesUnlinkRootChain(virBufferPtr buf, - int incoming, const char *ifname, + bool incoming, const char *ifname, int isTempChain) { char chain[MAX_CHAINNAME_LENGTH]; @@ -2988,7 +2988,7 @@ _ebtablesUnlinkRootChain(virBufferPtr buf, static int ebtablesUnlinkRootChain(virBufferPtr buf, - int incoming, const char *ifname) + bool incoming, const char *ifname) { return _ebtablesUnlinkRootChain(buf, incoming, ifname, 0); } @@ -2996,7 +2996,7 @@ ebtablesUnlinkRootChain(virBufferPtr buf, static int ebtablesUnlinkTmpRootChain(virBufferPtr buf, - int incoming, const char *ifname) + bool incoming, const char *ifname) { return _ebtablesUnlinkRootChain(buf, incoming, ifname, 1); } @@ -3005,7 +3005,7 @@ ebtablesUnlinkTmpRootChain(virBufferPtr buf, static int ebtablesCreateTmpSubChain(ebiptablesRuleInstPtr *inst, int *nRuleInstances, - int incoming, + bool incoming, const char *ifname, enum l3_proto_idx protoidx, const char *filtername, @@ -3145,7 +3145,7 @@ ebtablesRemoveTmpSubChains(virBufferPtr buf, static int ebtablesRenameTmpSubChain(virBufferPtr buf, - int incoming, + bool incoming, const char *ifname, const char *protocol) { @@ -3171,7 +3171,7 @@ ebtablesRenameTmpSubChain(virBufferPtr buf, static int ebtablesRenameTmpRootChain(virBufferPtr buf, - int incoming, + bool incoming, const char *ifname) { return ebtablesRenameTmpSubChain(buf, incoming, ifname, NULL); @@ -3208,8 +3208,8 @@ ebtablesRenameTmpSubAndRootChains(virBufferPtr buf, virBufferAddLit(buf, "rename_chains $chains\n"); - ebtablesRenameTmpRootChain(buf, 1, ifname); - ebtablesRenameTmpRootChain(buf, 0, ifname); + ebtablesRenameTmpRootChain(buf, true, ifname); + ebtablesRenameTmpRootChain(buf, false, ifname); return 0; } @@ -3275,7 +3275,7 @@ ebtablesApplyBasicRules(const char *ifname, NWFILTER_SET_EBTABLES_SHELLVAR(&buf); - ebtablesCreateTmpRootChain(&buf, 1, ifname, 1); + ebtablesCreateTmpRootChain(&buf, true, ifname, 1); PRINT_ROOT_CHAIN(chain, chainPrefix, ifname); virBufferAsprintf(&buf, @@ -3310,8 +3310,8 @@ ebtablesApplyBasicRules(const char *ifname, chain, CMD_STOPONERR(1)); - ebtablesLinkTmpRootChain(&buf, 1, ifname, 1); - ebtablesRenameTmpRootChain(&buf, 1, ifname); + ebtablesLinkTmpRootChain(&buf, true, ifname, 1); + ebtablesRenameTmpRootChain(&buf, true, ifname); if (ebiptablesExecCLI(&buf, false, NULL) < 0) goto tear_down_tmpebchains; @@ -3372,8 +3372,8 @@ ebtablesApplyDHCPOnlyRules(const char *ifname, NWFILTER_SET_EBTABLES_SHELLVAR(&buf); - ebtablesCreateTmpRootChain(&buf, 1, ifname, 1); - ebtablesCreateTmpRootChain(&buf, 0, ifname, 1); + ebtablesCreateTmpRootChain(&buf, true, ifname, 1); + ebtablesCreateTmpRootChain(&buf, false, ifname, 1); PRINT_ROOT_CHAIN(chain_in, CHAINPREFIX_HOST_IN_TEMP, ifname); PRINT_ROOT_CHAIN(chain_out, CHAINPREFIX_HOST_OUT_TEMP, ifname); @@ -3453,12 +3453,12 @@ ebtablesApplyDHCPOnlyRules(const char *ifname, chain_out, CMD_STOPONERR(1)); - ebtablesLinkTmpRootChain(&buf, 1, ifname, 1); - ebtablesLinkTmpRootChain(&buf, 0, ifname, 1); + ebtablesLinkTmpRootChain(&buf, true, ifname, 1); + ebtablesLinkTmpRootChain(&buf, false, ifname, 1); if (!leaveTemporary) { - ebtablesRenameTmpRootChain(&buf, 1, ifname); - ebtablesRenameTmpRootChain(&buf, 0, ifname); + ebtablesRenameTmpRootChain(&buf, true, ifname); + ebtablesRenameTmpRootChain(&buf, false, ifname); } if (ebiptablesExecCLI(&buf, false, NULL) < 0) @@ -3504,8 +3504,8 @@ ebtablesApplyDropAllRules(const char *ifname) NWFILTER_SET_EBTABLES_SHELLVAR(&buf); - ebtablesCreateTmpRootChain(&buf, 1, ifname, 1); - ebtablesCreateTmpRootChain(&buf, 0, ifname, 1); + ebtablesCreateTmpRootChain(&buf, true, ifname, 1); + ebtablesCreateTmpRootChain(&buf, false, ifname, 1); PRINT_ROOT_CHAIN(chain_in, CHAINPREFIX_HOST_IN_TEMP, ifname); PRINT_ROOT_CHAIN(chain_out, CHAINPREFIX_HOST_OUT_TEMP, ifname); @@ -3526,10 +3526,10 @@ ebtablesApplyDropAllRules(const char *ifname) chain_out, CMD_STOPONERR(1)); - ebtablesLinkTmpRootChain(&buf, 1, ifname, 1); - ebtablesLinkTmpRootChain(&buf, 0, ifname, 1); - ebtablesRenameTmpRootChain(&buf, 1, ifname); - ebtablesRenameTmpRootChain(&buf, 0, ifname); + ebtablesLinkTmpRootChain(&buf, true, ifname, 1); + ebtablesLinkTmpRootChain(&buf, false, ifname, 1); + ebtablesRenameTmpRootChain(&buf, true, ifname); + ebtablesRenameTmpRootChain(&buf, false, ifname); if (ebiptablesExecCLI(&buf, false, NULL) < 0) goto tear_down_tmpebchains; @@ -3563,17 +3563,17 @@ static int ebtablesCleanAll(const char *ifname) NWFILTER_SET_EBTABLES_SHELLVAR(&buf); - ebtablesUnlinkRootChain(&buf, 1, ifname); - ebtablesUnlinkRootChain(&buf, 0, ifname); + ebtablesUnlinkRootChain(&buf, true, ifname); + ebtablesUnlinkRootChain(&buf, false, ifname); ebtablesRemoveSubChains(&buf, ifname); - ebtablesRemoveRootChain(&buf, 1, ifname); - ebtablesRemoveRootChain(&buf, 0, ifname); + ebtablesRemoveRootChain(&buf, true, ifname); + ebtablesRemoveRootChain(&buf, false, ifname); - ebtablesUnlinkTmpRootChain(&buf, 1, ifname); - ebtablesUnlinkTmpRootChain(&buf, 0, ifname); + ebtablesUnlinkTmpRootChain(&buf, true, ifname); + ebtablesUnlinkTmpRootChain(&buf, false, ifname); ebtablesRemoveTmpSubChains(&buf, ifname); - ebtablesRemoveTmpRootChain(&buf, 1, ifname); - ebtablesRemoveTmpRootChain(&buf, 0, ifname); + ebtablesRemoveTmpRootChain(&buf, true, ifname); + ebtablesRemoveTmpRootChain(&buf, false, ifname); ebiptablesExecCLI(&buf, true, NULL); return 0; @@ -3682,7 +3682,8 @@ ebtablesGetProtoIdxByFiltername(const char *filtername) static int ebtablesCreateTmpRootAndSubChains(virBufferPtr buf, const char *ifname, - virHashTablePtr chains, int direction, + virHashTablePtr chains, + bool incoming, ebiptablesRuleInstPtr *inst, int *nRuleInstances) { @@ -3691,7 +3692,7 @@ ebtablesCreateTmpRootAndSubChains(virBufferPtr buf, virHashKeyValuePairPtr filter_names; const virNWFilterChainPriority *priority; - if (ebtablesCreateTmpRootChain(buf, direction, ifname, 1) < 0) + if (ebtablesCreateTmpRootChain(buf, incoming, ifname, 1) < 0) return -1; filter_names = virHashGetItems(chains, @@ -3706,7 +3707,7 @@ ebtablesCreateTmpRootAndSubChains(virBufferPtr buf, continue; priority = (const virNWFilterChainPriority *)filter_names[i].value; rc = ebtablesCreateTmpSubChain(inst, nRuleInstances, - direction, ifname, idx, + incoming, ifname, idx, filter_names[i].key, 1, *priority); if (rc < 0) @@ -3765,11 +3766,11 @@ ebiptablesApplyNewRules(const char *ifname, if (ebtables_cmd_path) { NWFILTER_SET_EBTABLES_SHELLVAR(&buf); - ebtablesUnlinkTmpRootChain(&buf, 1, ifname); - ebtablesUnlinkTmpRootChain(&buf, 0, ifname); + ebtablesUnlinkTmpRootChain(&buf, true, ifname); + ebtablesUnlinkTmpRootChain(&buf, false, ifname); ebtablesRemoveTmpSubChains(&buf, ifname); - ebtablesRemoveTmpRootChain(&buf, 1, ifname); - ebtablesRemoveTmpRootChain(&buf, 0, ifname); + ebtablesRemoveTmpRootChain(&buf, true, ifname); + ebtablesRemoveTmpRootChain(&buf, false, ifname); ebiptablesExecCLI(&buf, true, NULL); } @@ -3777,10 +3778,10 @@ ebiptablesApplyNewRules(const char *ifname, /* create needed chains */ if ((virHashSize(chains_in_set) > 0 && - ebtablesCreateTmpRootAndSubChains(&buf, ifname, chains_in_set, 1, + ebtablesCreateTmpRootAndSubChains(&buf, ifname, chains_in_set, true, &ebtChains, &nEbtChains) < 0) || (virHashSize(chains_out_set) > 0 && - ebtablesCreateTmpRootAndSubChains(&buf, ifname, chains_out_set, 0, + ebtablesCreateTmpRootAndSubChains(&buf, ifname, chains_out_set, false, &ebtChains, &nEbtChains) < 0)) { goto tear_down_tmpebchains; } @@ -3929,9 +3930,9 @@ ebiptablesApplyNewRules(const char *ifname, NWFILTER_SET_EBTABLES_SHELLVAR(&buf); if (virHashSize(chains_in_set) != 0) - ebtablesLinkTmpRootChain(&buf, 1, ifname, 1); + ebtablesLinkTmpRootChain(&buf, true, ifname, 1); if (virHashSize(chains_out_set) != 0) - ebtablesLinkTmpRootChain(&buf, 0, ifname, 1); + ebtablesLinkTmpRootChain(&buf, false, ifname, 1); if (ebiptablesExecCLI(&buf, false, &errmsg) < 0) goto tear_down_ebsubchains_and_unlink; @@ -3951,8 +3952,8 @@ tear_down_ebsubchains_and_unlink: if (ebtables_cmd_path) { NWFILTER_SET_EBTABLES_SHELLVAR(&buf); - ebtablesUnlinkTmpRootChain(&buf, 1, ifname); - ebtablesUnlinkTmpRootChain(&buf, 0, ifname); + ebtablesUnlinkTmpRootChain(&buf, true, ifname); + ebtablesUnlinkTmpRootChain(&buf, false, ifname); } tear_down_tmpip6tchains: @@ -3976,8 +3977,8 @@ tear_down_tmpebchains: NWFILTER_SET_EBTABLES_SHELLVAR(&buf); ebtablesRemoveTmpSubChains(&buf, ifname); - ebtablesRemoveTmpRootChain(&buf, 1, ifname); - ebtablesRemoveTmpRootChain(&buf, 0, ifname); + ebtablesRemoveTmpRootChain(&buf, true, ifname); + ebtablesRemoveTmpRootChain(&buf, false, ifname); } ebiptablesExecCLI(&buf, true, NULL); @@ -4025,12 +4026,12 @@ ebiptablesTearNewRules(const char *ifname) if (ebtables_cmd_path) { NWFILTER_SET_EBTABLES_SHELLVAR(&buf); - ebtablesUnlinkTmpRootChain(&buf, 1, ifname); - ebtablesUnlinkTmpRootChain(&buf, 0, ifname); + ebtablesUnlinkTmpRootChain(&buf, true, ifname); + ebtablesUnlinkTmpRootChain(&buf, false, ifname); ebtablesRemoveTmpSubChains(&buf, ifname); - ebtablesRemoveTmpRootChain(&buf, 1, ifname); - ebtablesRemoveTmpRootChain(&buf, 0, ifname); + ebtablesRemoveTmpRootChain(&buf, true, ifname); + ebtablesRemoveTmpRootChain(&buf, false, ifname); } ebiptablesExecCLI(&buf, true, NULL); @@ -4068,13 +4069,13 @@ ebiptablesTearOldRules(const char *ifname) if (ebtables_cmd_path) { NWFILTER_SET_EBTABLES_SHELLVAR(&buf); - ebtablesUnlinkRootChain(&buf, 1, ifname); - ebtablesUnlinkRootChain(&buf, 0, ifname); + ebtablesUnlinkRootChain(&buf, true, ifname); + ebtablesUnlinkRootChain(&buf, false, ifname); ebtablesRemoveSubChains(&buf, ifname); - ebtablesRemoveRootChain(&buf, 1, ifname); - ebtablesRemoveRootChain(&buf, 0, ifname); + ebtablesRemoveRootChain(&buf, true, ifname); + ebtablesRemoveRootChain(&buf, false, ifname); ebtablesRenameTmpSubAndRootChains(&buf, ifname); @@ -4157,13 +4158,13 @@ ebiptablesAllTeardown(const char *ifname) if (ebtables_cmd_path) { NWFILTER_SET_EBTABLES_SHELLVAR(&buf); - ebtablesUnlinkRootChain(&buf, 1, ifname); - ebtablesUnlinkRootChain(&buf, 0, ifname); + ebtablesUnlinkRootChain(&buf, true, ifname); + ebtablesUnlinkRootChain(&buf, false, ifname); ebtablesRemoveSubChains(&buf, ifname); - ebtablesRemoveRootChain(&buf, 1, ifname); - ebtablesRemoveRootChain(&buf, 0, ifname); + ebtablesRemoveRootChain(&buf, true, ifname); + ebtablesRemoveRootChain(&buf, false, ifname); } ebiptablesExecCLI(&buf, true, NULL); -- 1.8.5.3
On 03/18/2014 07:36 AM, Daniel P. Berrange wrote:
Many methods in the nwfilter code have an 'int incoming' parameter that only takes 0 or 1, so should use a bool instead.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/nwfilter/nwfilter_ebiptables_driver.c | 187 +++++++++++++++--------------- 1 file changed, 94 insertions(+), 93 deletions(-)
ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
A lot of methods have a 'bool incoming' parameter but then do (incoming) ? ... : .... The round brackets here add nothing to the code so can be removed. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/nwfilter/nwfilter_ebiptables_driver.c | 92 +++++++++++++++---------------- 1 file changed, 46 insertions(+), 46 deletions(-) diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 2543854..b3405e5 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -648,8 +648,8 @@ iptablesCreateTmpRootChain(virBufferPtr buf, char chain[MAX_CHAINNAME_LENGTH]; char chainPrefix[2] = { prefix, - (incoming) ? CHAINPREFIX_HOST_IN_TEMP - : CHAINPREFIX_HOST_OUT_TEMP + incoming ? CHAINPREFIX_HOST_IN_TEMP + : CHAINPREFIX_HOST_OUT_TEMP }; PRINT_IPT_ROOT_CHAIN(chain, chainPrefix, ifname); @@ -688,11 +688,11 @@ _iptablesRemoveRootChain(virBufferPtr buf, }; if (isTempChain) - chainPrefix[1] = (incoming) ? CHAINPREFIX_HOST_IN_TEMP - : CHAINPREFIX_HOST_OUT_TEMP; + chainPrefix[1] = incoming ? CHAINPREFIX_HOST_IN_TEMP + : CHAINPREFIX_HOST_OUT_TEMP; else - chainPrefix[1] = (incoming) ? CHAINPREFIX_HOST_IN - : CHAINPREFIX_HOST_OUT; + chainPrefix[1] = incoming ? CHAINPREFIX_HOST_IN + : CHAINPREFIX_HOST_OUT; PRINT_IPT_ROOT_CHAIN(chain, chainPrefix, ifname); @@ -759,11 +759,11 @@ iptablesLinkTmpRootChain(virBufferPtr buf, char chain[MAX_CHAINNAME_LENGTH]; char chainPrefix[2] = { prefix, - (incoming) ? CHAINPREFIX_HOST_IN_TEMP - : CHAINPREFIX_HOST_OUT_TEMP + incoming ? CHAINPREFIX_HOST_IN_TEMP + : CHAINPREFIX_HOST_OUT_TEMP }; - const char *match = (incoming) ? MATCH_PHYSDEV_IN - : MATCH_PHYSDEV_OUT; + const char *match = incoming ? MATCH_PHYSDEV_IN + : MATCH_PHYSDEV_OUT; PRINT_IPT_ROOT_CHAIN(chain, chainPrefix, ifname); @@ -839,15 +839,15 @@ _iptablesUnlinkRootChain(virBufferPtr buf, prefix, }; if (isTempChain) - chainPrefix[1] = (incoming) ? CHAINPREFIX_HOST_IN_TEMP - : CHAINPREFIX_HOST_OUT_TEMP; + chainPrefix[1] = incoming ? CHAINPREFIX_HOST_IN_TEMP + : CHAINPREFIX_HOST_OUT_TEMP; else - chainPrefix[1] = (incoming) ? CHAINPREFIX_HOST_IN - : CHAINPREFIX_HOST_OUT; - const char *match = (incoming) ? MATCH_PHYSDEV_IN - : MATCH_PHYSDEV_OUT; - const char *old_match = (incoming) ? NULL - : MATCH_PHYSDEV_OUT_OLD; + chainPrefix[1] = incoming ? CHAINPREFIX_HOST_IN + : CHAINPREFIX_HOST_OUT; + const char *match = incoming ? MATCH_PHYSDEV_IN + : MATCH_PHYSDEV_OUT; + const char *old_match = incoming ? NULL + : MATCH_PHYSDEV_OUT_OLD; PRINT_IPT_ROOT_CHAIN(chain, chainPrefix, ifname); @@ -927,13 +927,13 @@ iptablesRenameTmpRootChain(virBufferPtr buf, char tmpchain[MAX_CHAINNAME_LENGTH], chain[MAX_CHAINNAME_LENGTH]; char tmpChainPrefix[2] = { prefix, - (incoming) ? CHAINPREFIX_HOST_IN_TEMP - : CHAINPREFIX_HOST_OUT_TEMP + incoming ? CHAINPREFIX_HOST_IN_TEMP + : CHAINPREFIX_HOST_OUT_TEMP }; char chainPrefix[2] = { prefix, - (incoming) ? CHAINPREFIX_HOST_IN - : CHAINPREFIX_HOST_OUT + incoming ? CHAINPREFIX_HOST_IN + : CHAINPREFIX_HOST_OUT }; PRINT_IPT_ROOT_CHAIN(tmpchain, tmpChainPrefix, ifname); @@ -2873,8 +2873,8 @@ ebtablesCreateTmpRootChain(virBufferPtr buf, int stopOnError) { char chain[MAX_CHAINNAME_LENGTH]; - char chainPrefix = (incoming) ? CHAINPREFIX_HOST_IN_TEMP - : CHAINPREFIX_HOST_OUT_TEMP; + char chainPrefix = incoming ? CHAINPREFIX_HOST_IN_TEMP + : CHAINPREFIX_HOST_OUT_TEMP; PRINT_ROOT_CHAIN(chain, chainPrefix, ifname); @@ -2895,9 +2895,9 @@ ebtablesLinkTmpRootChain(virBufferPtr buf, int stopOnError) { char chain[MAX_CHAINNAME_LENGTH]; - char chainPrefix = (incoming) ? CHAINPREFIX_HOST_IN_TEMP - : CHAINPREFIX_HOST_OUT_TEMP; - char iodev = (incoming) ? 'i' : 'o'; + char chainPrefix = incoming ? CHAINPREFIX_HOST_IN_TEMP + : CHAINPREFIX_HOST_OUT_TEMP; + char iodev = incoming ? 'i' : 'o'; PRINT_ROOT_CHAIN(chain, chainPrefix, ifname); @@ -2905,8 +2905,8 @@ ebtablesLinkTmpRootChain(virBufferPtr buf, CMD_DEF("$EBT -t nat -A %s -%c %s -j %s") CMD_SEPARATOR CMD_EXEC "%s", - (incoming) ? EBTABLES_CHAIN_INCOMING - : EBTABLES_CHAIN_OUTGOING, + incoming ? EBTABLES_CHAIN_INCOMING + : EBTABLES_CHAIN_OUTGOING, iodev, ifname, chain, CMD_STOPONERR(stopOnError)); @@ -2923,11 +2923,11 @@ _ebtablesRemoveRootChain(virBufferPtr buf, char chain[MAX_CHAINNAME_LENGTH]; char chainPrefix; if (isTempChain) - chainPrefix = (incoming) ? CHAINPREFIX_HOST_IN_TEMP - : CHAINPREFIX_HOST_OUT_TEMP; + chainPrefix = incoming ? CHAINPREFIX_HOST_IN_TEMP + : CHAINPREFIX_HOST_OUT_TEMP; else - chainPrefix = (incoming) ? CHAINPREFIX_HOST_IN - : CHAINPREFIX_HOST_OUT; + chainPrefix = incoming ? CHAINPREFIX_HOST_IN + : CHAINPREFIX_HOST_OUT; PRINT_ROOT_CHAIN(chain, chainPrefix, ifname); @@ -2963,23 +2963,23 @@ _ebtablesUnlinkRootChain(virBufferPtr buf, int isTempChain) { char chain[MAX_CHAINNAME_LENGTH]; - char iodev = (incoming) ? 'i' : 'o'; + char iodev = incoming ? 'i' : 'o'; char chainPrefix; if (isTempChain) { - chainPrefix = (incoming) ? CHAINPREFIX_HOST_IN_TEMP - : CHAINPREFIX_HOST_OUT_TEMP; + chainPrefix = incoming ? CHAINPREFIX_HOST_IN_TEMP + : CHAINPREFIX_HOST_OUT_TEMP; } else { - chainPrefix = (incoming) ? CHAINPREFIX_HOST_IN - : CHAINPREFIX_HOST_OUT; + chainPrefix = incoming ? CHAINPREFIX_HOST_IN + : CHAINPREFIX_HOST_OUT; } PRINT_ROOT_CHAIN(chain, chainPrefix, ifname); virBufferAsprintf(buf, "$EBT -t nat -D %s -%c %s -j %s" CMD_SEPARATOR, - (incoming) ? EBTABLES_CHAIN_INCOMING - : EBTABLES_CHAIN_OUTGOING, + incoming ? EBTABLES_CHAIN_INCOMING + : EBTABLES_CHAIN_OUTGOING, iodev, ifname, chain); return 0; @@ -3016,8 +3016,8 @@ ebtablesCreateTmpSubChain(ebiptablesRuleInstPtr *inst, ebiptablesRuleInstPtr tmp = *inst; size_t count = *nRuleInstances; char rootchain[MAX_CHAINNAME_LENGTH], chain[MAX_CHAINNAME_LENGTH]; - char chainPrefix = (incoming) ? CHAINPREFIX_HOST_IN_TEMP - : CHAINPREFIX_HOST_OUT_TEMP; + char chainPrefix = incoming ? CHAINPREFIX_HOST_IN_TEMP + : CHAINPREFIX_HOST_OUT_TEMP; char *protostr = NULL; PRINT_ROOT_CHAIN(rootchain, chainPrefix, ifname); @@ -3150,10 +3150,10 @@ ebtablesRenameTmpSubChain(virBufferPtr buf, const char *protocol) { char tmpchain[MAX_CHAINNAME_LENGTH], chain[MAX_CHAINNAME_LENGTH]; - char tmpChainPrefix = (incoming) ? CHAINPREFIX_HOST_IN_TEMP - : CHAINPREFIX_HOST_OUT_TEMP; - char chainPrefix = (incoming) ? CHAINPREFIX_HOST_IN - : CHAINPREFIX_HOST_OUT; + char tmpChainPrefix = incoming ? CHAINPREFIX_HOST_IN_TEMP + : CHAINPREFIX_HOST_OUT_TEMP; + char chainPrefix = incoming ? CHAINPREFIX_HOST_IN + : CHAINPREFIX_HOST_OUT; if (protocol) { PRINT_CHAIN(tmpchain, tmpChainPrefix, ifname, protocol); -- 1.8.5.3
On 03/18/2014 07:36 AM, Daniel P. Berrange wrote:
A lot of methods have a 'bool incoming' parameter but then do (incoming) ? ... : .... The round brackets here add nothing to the code so can be removed.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/nwfilter/nwfilter_ebiptables_driver.c | 92 +++++++++++++++---------------- 1 file changed, 46 insertions(+), 46 deletions(-)
ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Many nwfilter methods have an 'int stopOnError' parameter but with 1 exception, the callers always pass '1'. The parameter can therefore be removed from all except one method. That method will be changed to 'bool stopOnError' Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/nwfilter/nwfilter_ebiptables_driver.c | 99 ++++++++++++++----------------- 1 file changed, 46 insertions(+), 53 deletions(-) diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index b3405e5..640c5fe 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -585,8 +585,7 @@ ebtablesHandleEthHdr(virBufferPtr buf, static int iptablesLinkIPTablesBaseChain(virBufferPtr buf, const char *udchain, const char *syschain, - unsigned int pos, - int stopOnError) + unsigned int pos) { virBufferAsprintf(buf, "res=$($IPT -L %s -n --line-number | %s '%s')\n" @@ -612,10 +611,10 @@ static int iptablesLinkIPTablesBaseChain(virBufferPtr buf, pos, syschain, pos, udchain, - CMD_STOPONERR(stopOnError), + CMD_STOPONERR(true), syschain, - CMD_STOPONERR(stopOnError)); + CMD_STOPONERR(true)); return 0; } @@ -627,13 +626,13 @@ static int iptablesCreateBaseChains(virBufferPtr buf) "$IPT -N " VIRT_IN_POST_CHAIN CMD_SEPARATOR "$IPT -N " HOST_IN_CHAIN CMD_SEPARATOR); iptablesLinkIPTablesBaseChain(buf, - VIRT_IN_CHAIN, "FORWARD", 1, 1); + VIRT_IN_CHAIN, "FORWARD", 1); iptablesLinkIPTablesBaseChain(buf, - VIRT_OUT_CHAIN, "FORWARD", 2, 1); + VIRT_OUT_CHAIN, "FORWARD", 2); iptablesLinkIPTablesBaseChain(buf, - VIRT_IN_POST_CHAIN, "FORWARD", 3, 1); + VIRT_IN_POST_CHAIN, "FORWARD", 3); iptablesLinkIPTablesBaseChain(buf, - HOST_IN_CHAIN, "INPUT", 1, 1); + HOST_IN_CHAIN, "INPUT", 1); return 0; } @@ -642,8 +641,7 @@ static int iptablesCreateBaseChains(virBufferPtr buf) static int iptablesCreateTmpRootChain(virBufferPtr buf, char prefix, - bool incoming, const char *ifname, - int stopOnError) + bool incoming, const char *ifname) { char chain[MAX_CHAINNAME_LENGTH]; char chainPrefix[2] = { @@ -659,7 +657,7 @@ iptablesCreateTmpRootChain(virBufferPtr buf, CMD_EXEC "%s", chain, - CMD_STOPONERR(stopOnError)); + CMD_STOPONERR(true)); return 0; } @@ -669,9 +667,9 @@ static int iptablesCreateTmpRootChains(virBufferPtr buf, const char *ifname) { - iptablesCreateTmpRootChain(buf, 'F', false, ifname, 1); - iptablesCreateTmpRootChain(buf, 'F', true, ifname, 1); - iptablesCreateTmpRootChain(buf, 'H', true, ifname, 1); + iptablesCreateTmpRootChain(buf, 'F', false, ifname); + iptablesCreateTmpRootChain(buf, 'F', true, ifname); + iptablesCreateTmpRootChain(buf, 'H', true, ifname); return 0; } @@ -753,8 +751,7 @@ static int iptablesLinkTmpRootChain(virBufferPtr buf, const char *basechain, char prefix, - bool incoming, const char *ifname, - int stopOnError) + bool incoming, const char *ifname) { char chain[MAX_CHAINNAME_LENGTH]; char chainPrefix[2] = { @@ -775,7 +772,7 @@ iptablesLinkTmpRootChain(virBufferPtr buf, basechain, match, ifname, chain, - CMD_STOPONERR(stopOnError)); + CMD_STOPONERR(true)); return 0; } @@ -785,9 +782,9 @@ static int iptablesLinkTmpRootChains(virBufferPtr buf, const char *ifname) { - iptablesLinkTmpRootChain(buf, VIRT_OUT_CHAIN, 'F', false, ifname, 1); - iptablesLinkTmpRootChain(buf, VIRT_IN_CHAIN, 'F', true, ifname, 1); - iptablesLinkTmpRootChain(buf, HOST_IN_CHAIN, 'H', true, ifname, 1); + iptablesLinkTmpRootChain(buf, VIRT_OUT_CHAIN, 'F', false, ifname); + iptablesLinkTmpRootChain(buf, VIRT_IN_CHAIN, 'F', true, ifname); + iptablesLinkTmpRootChain(buf, HOST_IN_CHAIN, 'H', true, ifname); return 0; } @@ -960,15 +957,14 @@ iptablesRenameTmpRootChains(virBufferPtr buf, static void iptablesInstCommand(virBufferPtr buf, - const char *templ, char cmd, int pos, - int stopOnError) + const char *templ, char cmd, int pos) { char position[10] = { 0 }; if (pos >= 0) snprintf(position, sizeof(position), "%d", pos); virBufferAsprintf(buf, templ, cmd, position); virBufferAsprintf(buf, CMD_SEPARATOR "%s", - CMD_STOPONERR(stopOnError)); + CMD_STOPONERR(true)); } @@ -2869,8 +2865,7 @@ ebiptablesExecCLI(virBufferPtr buf, bool ignoreNonzero, char **outbuf) static int ebtablesCreateTmpRootChain(virBufferPtr buf, - bool incoming, const char *ifname, - int stopOnError) + bool incoming, const char *ifname) { char chain[MAX_CHAINNAME_LENGTH]; char chainPrefix = incoming ? CHAINPREFIX_HOST_IN_TEMP @@ -2883,7 +2878,7 @@ ebtablesCreateTmpRootChain(virBufferPtr buf, CMD_EXEC "%s", chain, - CMD_STOPONERR(stopOnError)); + CMD_STOPONERR(true)); return 0; } @@ -2891,8 +2886,7 @@ ebtablesCreateTmpRootChain(virBufferPtr buf, static int ebtablesLinkTmpRootChain(virBufferPtr buf, - bool incoming, const char *ifname, - int stopOnError) + bool incoming, const char *ifname) { char chain[MAX_CHAINNAME_LENGTH]; char chainPrefix = incoming ? CHAINPREFIX_HOST_IN_TEMP @@ -2909,7 +2903,7 @@ ebtablesLinkTmpRootChain(virBufferPtr buf, : EBTABLES_CHAIN_OUTGOING, iodev, ifname, chain, - CMD_STOPONERR(stopOnError)); + CMD_STOPONERR(true)); return 0; } @@ -3009,7 +3003,6 @@ ebtablesCreateTmpSubChain(ebiptablesRuleInstPtr *inst, const char *ifname, enum l3_proto_idx protoidx, const char *filtername, - int stopOnError, virNWFilterChainPriority priority) { virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -3057,11 +3050,11 @@ ebtablesCreateTmpSubChain(ebiptablesRuleInstPtr *inst, chain, chain, - CMD_STOPONERR(stopOnError), + CMD_STOPONERR(true), rootchain, protostr, chain, - CMD_STOPONERR(stopOnError)); + CMD_STOPONERR(true)); VIR_FREE(protostr); @@ -3217,7 +3210,7 @@ ebtablesRenameTmpSubAndRootChains(virBufferPtr buf, static void ebiptablesInstCommand(virBufferPtr buf, const char *templ, char cmd, int pos, - int stopOnError) + bool stopOnError) { char position[10] = { 0 }; if (pos >= 0) @@ -3275,7 +3268,7 @@ ebtablesApplyBasicRules(const char *ifname, NWFILTER_SET_EBTABLES_SHELLVAR(&buf); - ebtablesCreateTmpRootChain(&buf, true, ifname, 1); + ebtablesCreateTmpRootChain(&buf, true, ifname); PRINT_ROOT_CHAIN(chain, chainPrefix, ifname); virBufferAsprintf(&buf, @@ -3310,7 +3303,7 @@ ebtablesApplyBasicRules(const char *ifname, chain, CMD_STOPONERR(1)); - ebtablesLinkTmpRootChain(&buf, true, ifname, 1); + ebtablesLinkTmpRootChain(&buf, true, ifname); ebtablesRenameTmpRootChain(&buf, true, ifname); if (ebiptablesExecCLI(&buf, false, NULL) < 0) @@ -3372,8 +3365,8 @@ ebtablesApplyDHCPOnlyRules(const char *ifname, NWFILTER_SET_EBTABLES_SHELLVAR(&buf); - ebtablesCreateTmpRootChain(&buf, true, ifname, 1); - ebtablesCreateTmpRootChain(&buf, false, ifname, 1); + ebtablesCreateTmpRootChain(&buf, true, ifname); + ebtablesCreateTmpRootChain(&buf, false, ifname); PRINT_ROOT_CHAIN(chain_in, CHAINPREFIX_HOST_IN_TEMP, ifname); PRINT_ROOT_CHAIN(chain_out, CHAINPREFIX_HOST_OUT_TEMP, ifname); @@ -3453,8 +3446,8 @@ ebtablesApplyDHCPOnlyRules(const char *ifname, chain_out, CMD_STOPONERR(1)); - ebtablesLinkTmpRootChain(&buf, true, ifname, 1); - ebtablesLinkTmpRootChain(&buf, false, ifname, 1); + ebtablesLinkTmpRootChain(&buf, true, ifname); + ebtablesLinkTmpRootChain(&buf, false, ifname); if (!leaveTemporary) { ebtablesRenameTmpRootChain(&buf, true, ifname); @@ -3504,8 +3497,8 @@ ebtablesApplyDropAllRules(const char *ifname) NWFILTER_SET_EBTABLES_SHELLVAR(&buf); - ebtablesCreateTmpRootChain(&buf, true, ifname, 1); - ebtablesCreateTmpRootChain(&buf, false, ifname, 1); + ebtablesCreateTmpRootChain(&buf, true, ifname); + ebtablesCreateTmpRootChain(&buf, false, ifname); PRINT_ROOT_CHAIN(chain_in, CHAINPREFIX_HOST_IN_TEMP, ifname); PRINT_ROOT_CHAIN(chain_out, CHAINPREFIX_HOST_OUT_TEMP, ifname); @@ -3526,8 +3519,8 @@ ebtablesApplyDropAllRules(const char *ifname) chain_out, CMD_STOPONERR(1)); - ebtablesLinkTmpRootChain(&buf, true, ifname, 1); - ebtablesLinkTmpRootChain(&buf, false, ifname, 1); + ebtablesLinkTmpRootChain(&buf, true, ifname); + ebtablesLinkTmpRootChain(&buf, false, ifname); ebtablesRenameTmpRootChain(&buf, true, ifname); ebtablesRenameTmpRootChain(&buf, false, ifname); @@ -3692,7 +3685,7 @@ ebtablesCreateTmpRootAndSubChains(virBufferPtr buf, virHashKeyValuePairPtr filter_names; const virNWFilterChainPriority *priority; - if (ebtablesCreateTmpRootChain(buf, incoming, ifname, 1) < 0) + if (ebtablesCreateTmpRootChain(buf, incoming, ifname) < 0) return -1; filter_names = virHashGetItems(chains, @@ -3708,7 +3701,7 @@ ebtablesCreateTmpRootAndSubChains(virBufferPtr buf, priority = (const virNWFilterChainPriority *)filter_names[i].value; rc = ebtablesCreateTmpSubChain(inst, nRuleInstances, incoming, ifname, idx, - filter_names[i].key, 1, + filter_names[i].key, *priority); if (rc < 0) break; @@ -3823,11 +3816,11 @@ ebiptablesApplyNewRules(const char *ifname, ebtChains[j].priority <= inst[i]->priority) { ebiptablesInstCommand(&buf, ebtChains[j++].commandTemplate, - 'A', -1, 1); + 'A', -1, true); } ebiptablesInstCommand(&buf, inst[i]->commandTemplate, - 'A', -1, 1); + 'A', -1, true); break; case RT_IPTABLES: haveIptables = true; @@ -3841,7 +3834,7 @@ ebiptablesApplyNewRules(const char *ifname, while (j < nEbtChains) ebiptablesInstCommand(&buf, ebtChains[j++].commandTemplate, - 'A', -1, 1); + 'A', -1, true); if (ebiptablesExecCLI(&buf, false, &errmsg) < 0) goto tear_down_tmpebchains; @@ -3878,7 +3871,7 @@ ebiptablesApplyNewRules(const char *ifname, if (inst[i]->ruleType == RT_IPTABLES) iptablesInstCommand(&buf, inst[i]->commandTemplate, - 'A', -1, 1); + 'A', -1); } if (ebiptablesExecCLI(&buf, false, &errmsg) < 0) @@ -3918,7 +3911,7 @@ ebiptablesApplyNewRules(const char *ifname, if (inst[i]->ruleType == RT_IP6TABLES) iptablesInstCommand(&buf, inst[i]->commandTemplate, - 'A', -1, 1); + 'A', -1); } if (ebiptablesExecCLI(&buf, false, &errmsg) < 0) @@ -3930,9 +3923,9 @@ ebiptablesApplyNewRules(const char *ifname, NWFILTER_SET_EBTABLES_SHELLVAR(&buf); if (virHashSize(chains_in_set) != 0) - ebtablesLinkTmpRootChain(&buf, true, ifname, 1); + ebtablesLinkTmpRootChain(&buf, true, ifname); if (virHashSize(chains_out_set) != 0) - ebtablesLinkTmpRootChain(&buf, false, ifname, 1); + ebtablesLinkTmpRootChain(&buf, false, ifname); if (ebiptablesExecCLI(&buf, false, &errmsg) < 0) goto tear_down_ebsubchains_and_unlink; @@ -4113,7 +4106,7 @@ ebiptablesRemoveRules(const char *ifname ATTRIBUTE_UNUSED, ebiptablesInstCommand(&buf, inst[i]->commandTemplate, 'D', -1, - 0); + false); if (ebiptablesExecCLI(&buf, true, NULL) < 0) goto cleanup; -- 1.8.5.3
Many nwfilter methods have an int return value but only ever return 0 and their callers never check the return value either. These methods can all be void. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/nwfilter/nwfilter_ebiptables_driver.c | 157 ++++++++++++------------------ 1 file changed, 61 insertions(+), 96 deletions(-) diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 640c5fe..63ccfe6 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -206,7 +206,7 @@ static const char *m_physdev_out_old_str = "-m physdev " PHYSDEV_OUT_OLD; static int ebtablesRemoveBasicRules(const char *ifname); static int ebiptablesDriverInit(bool privileged); static void ebiptablesDriverShutdown(void); -static int ebtablesCleanAll(const char *ifname); +static void ebtablesCleanAll(const char *ifname); static int ebiptablesAllTeardown(const char *ifname); static virMutex execCLIMutex; @@ -582,10 +582,11 @@ ebtablesHandleEthHdr(virBufferPtr buf, /************************ iptables support ************************/ -static int iptablesLinkIPTablesBaseChain(virBufferPtr buf, - const char *udchain, - const char *syschain, - unsigned int pos) +static void +iptablesLinkIPTablesBaseChain(virBufferPtr buf, + const char *udchain, + const char *syschain, + unsigned int pos) { virBufferAsprintf(buf, "res=$($IPT -L %s -n --line-number | %s '%s')\n" @@ -615,11 +616,11 @@ static int iptablesLinkIPTablesBaseChain(virBufferPtr buf, syschain, CMD_STOPONERR(true)); - return 0; } -static int iptablesCreateBaseChains(virBufferPtr buf) +static void +iptablesCreateBaseChains(virBufferPtr buf) { virBufferAddLit(buf, "$IPT -N " VIRT_IN_CHAIN CMD_SEPARATOR "$IPT -N " VIRT_OUT_CHAIN CMD_SEPARATOR @@ -633,12 +634,10 @@ static int iptablesCreateBaseChains(virBufferPtr buf) VIRT_IN_POST_CHAIN, "FORWARD", 3); iptablesLinkIPTablesBaseChain(buf, HOST_IN_CHAIN, "INPUT", 1); - - return 0; } -static int +static void iptablesCreateTmpRootChain(virBufferPtr buf, char prefix, bool incoming, const char *ifname) @@ -658,23 +657,20 @@ iptablesCreateTmpRootChain(virBufferPtr buf, "%s", chain, CMD_STOPONERR(true)); - - return 0; } -static int +static void iptablesCreateTmpRootChains(virBufferPtr buf, const char *ifname) { iptablesCreateTmpRootChain(buf, 'F', false, ifname); iptablesCreateTmpRootChain(buf, 'F', true, ifname); iptablesCreateTmpRootChain(buf, 'H', true, ifname); - return 0; } -static int +static void _iptablesRemoveRootChain(virBufferPtr buf, char prefix, bool incoming, const char *ifname, @@ -699,55 +695,51 @@ _iptablesRemoveRootChain(virBufferPtr buf, "$IPT -X %s" CMD_SEPARATOR, chain, chain); - - return 0; } -static int +static void iptablesRemoveRootChain(virBufferPtr buf, char prefix, bool incoming, const char *ifname) { - return _iptablesRemoveRootChain(buf, prefix, incoming, ifname, 0); + _iptablesRemoveRootChain(buf, prefix, incoming, ifname, 0); } -static int +static void iptablesRemoveTmpRootChain(virBufferPtr buf, char prefix, bool incoming, const char *ifname) { - return _iptablesRemoveRootChain(buf, prefix, - incoming, ifname, 1); + _iptablesRemoveRootChain(buf, prefix, + incoming, ifname, 1); } -static int +static void iptablesRemoveTmpRootChains(virBufferPtr buf, const char *ifname) { iptablesRemoveTmpRootChain(buf, 'F', false, ifname); iptablesRemoveTmpRootChain(buf, 'F', true, ifname); iptablesRemoveTmpRootChain(buf, 'H', true, ifname); - return 0; } -static int +static void iptablesRemoveRootChains(virBufferPtr buf, const char *ifname) { iptablesRemoveRootChain(buf, 'F', false, ifname); iptablesRemoveRootChain(buf, 'F', true, ifname); iptablesRemoveRootChain(buf, 'H', true, ifname); - return 0; } -static int +static void iptablesLinkTmpRootChain(virBufferPtr buf, const char *basechain, char prefix, @@ -773,24 +765,20 @@ iptablesLinkTmpRootChain(virBufferPtr buf, match, ifname, chain, CMD_STOPONERR(true)); - - return 0; } -static int +static void iptablesLinkTmpRootChains(virBufferPtr buf, const char *ifname) { iptablesLinkTmpRootChain(buf, VIRT_OUT_CHAIN, 'F', false, ifname); iptablesLinkTmpRootChain(buf, VIRT_IN_CHAIN, 'F', true, ifname); iptablesLinkTmpRootChain(buf, HOST_IN_CHAIN, 'H', true, ifname); - - return 0; } -static int +static void iptablesSetupVirtInPost(virBufferPtr buf, const char *ifname) { @@ -808,11 +796,10 @@ iptablesSetupVirtInPost(virBufferPtr buf, PHYSDEV_IN, ifname, match, ifname, CMD_STOPONERR(1)); - return 0; } -static int +static void iptablesClearVirtInPost(virBufferPtr buf, const char *ifname) { @@ -821,10 +808,9 @@ iptablesClearVirtInPost(virBufferPtr buf, "$IPT -D " VIRT_IN_POST_CHAIN " %s %s -j ACCEPT" CMD_SEPARATOR, match, ifname); - return 0; } -static int +static void _iptablesUnlinkRootChain(virBufferPtr buf, const char *basechain, char prefix, @@ -865,57 +851,52 @@ _iptablesUnlinkRootChain(virBufferPtr buf, "%s %s -g %s" CMD_SEPARATOR, basechain, old_match, ifname, chain); - - return 0; } -static int +static void iptablesUnlinkRootChain(virBufferPtr buf, const char *basechain, char prefix, bool incoming, const char *ifname) { - return _iptablesUnlinkRootChain(buf, - basechain, prefix, incoming, ifname, 0); + _iptablesUnlinkRootChain(buf, + basechain, prefix, incoming, ifname, 0); } -static int +static void iptablesUnlinkTmpRootChain(virBufferPtr buf, const char *basechain, char prefix, bool incoming, const char *ifname) { - return _iptablesUnlinkRootChain(buf, - basechain, prefix, incoming, ifname, 1); + _iptablesUnlinkRootChain(buf, + basechain, prefix, incoming, ifname, 1); } -static int +static void iptablesUnlinkRootChains(virBufferPtr buf, const char *ifname) { iptablesUnlinkRootChain(buf, VIRT_OUT_CHAIN, 'F', false, ifname); iptablesUnlinkRootChain(buf, VIRT_IN_CHAIN, 'F', true, ifname); iptablesUnlinkRootChain(buf, HOST_IN_CHAIN, 'H', true, ifname); - - return 0; } -static int +static void iptablesUnlinkTmpRootChains(virBufferPtr buf, const char *ifname) { iptablesUnlinkTmpRootChain(buf, VIRT_OUT_CHAIN, 'F', false, ifname); iptablesUnlinkTmpRootChain(buf, VIRT_IN_CHAIN, 'F', true, ifname); iptablesUnlinkTmpRootChain(buf, HOST_IN_CHAIN, 'H', true, ifname); - return 0; } -static int +static void iptablesRenameTmpRootChain(virBufferPtr buf, char prefix, bool incoming, @@ -940,18 +921,16 @@ iptablesRenameTmpRootChain(virBufferPtr buf, "$IPT -E %s %s" CMD_SEPARATOR, tmpchain, chain); - return 0; } -static int +static void iptablesRenameTmpRootChains(virBufferPtr buf, const char *ifname) { iptablesRenameTmpRootChain(buf, 'F', false, ifname); iptablesRenameTmpRootChain(buf, 'F', true, ifname); iptablesRenameTmpRootChain(buf, 'H', true, ifname); - return 0; } @@ -2863,7 +2842,7 @@ ebiptablesExecCLI(virBufferPtr buf, bool ignoreNonzero, char **outbuf) } -static int +static void ebtablesCreateTmpRootChain(virBufferPtr buf, bool incoming, const char *ifname) { @@ -2880,11 +2859,10 @@ ebtablesCreateTmpRootChain(virBufferPtr buf, chain, CMD_STOPONERR(true)); - return 0; } -static int +static void ebtablesLinkTmpRootChain(virBufferPtr buf, bool incoming, const char *ifname) { @@ -2904,12 +2882,10 @@ ebtablesLinkTmpRootChain(virBufferPtr buf, iodev, ifname, chain, CMD_STOPONERR(true)); - - return 0; } -static int +static void _ebtablesRemoveRootChain(virBufferPtr buf, bool incoming, const char *ifname, int isTempChain) @@ -2930,28 +2906,26 @@ _ebtablesRemoveRootChain(virBufferPtr buf, "$EBT -t nat -X %s" CMD_SEPARATOR, chain, chain); - - return 0; } -static int +static void ebtablesRemoveRootChain(virBufferPtr buf, bool incoming, const char *ifname) { - return _ebtablesRemoveRootChain(buf, incoming, ifname, 0); + _ebtablesRemoveRootChain(buf, incoming, ifname, 0); } -static int +static void ebtablesRemoveTmpRootChain(virBufferPtr buf, bool incoming, const char *ifname) { - return _ebtablesRemoveRootChain(buf, incoming, ifname, 1); + _ebtablesRemoveRootChain(buf, incoming, ifname, 1); } -static int +static void _ebtablesUnlinkRootChain(virBufferPtr buf, bool incoming, const char *ifname, int isTempChain) @@ -2975,24 +2949,22 @@ _ebtablesUnlinkRootChain(virBufferPtr buf, incoming ? EBTABLES_CHAIN_INCOMING : EBTABLES_CHAIN_OUTGOING, iodev, ifname, chain); - - return 0; } -static int +static void ebtablesUnlinkRootChain(virBufferPtr buf, bool incoming, const char *ifname) { - return _ebtablesUnlinkRootChain(buf, incoming, ifname, 0); + _ebtablesUnlinkRootChain(buf, incoming, ifname, 0); } -static int +static void ebtablesUnlinkTmpRootChain(virBufferPtr buf, bool incoming, const char *ifname) { - return _ebtablesUnlinkRootChain(buf, incoming, ifname, 1); + _ebtablesUnlinkRootChain(buf, incoming, ifname, 1); } @@ -3077,7 +3049,7 @@ ebtablesCreateTmpSubChain(ebiptablesRuleInstPtr *inst, return 0; } -static int +static void _ebtablesRemoveSubChains(virBufferPtr buf, const char *ifname, const char *chains) @@ -3106,11 +3078,9 @@ _ebtablesRemoveSubChains(virBufferPtr buf, rootchain); } virBufferAddLit(buf, "rm_chains $chains\n"); - - return 0; } -static int +static void ebtablesRemoveSubChains(virBufferPtr buf, const char *ifname) { @@ -3120,10 +3090,10 @@ ebtablesRemoveSubChains(virBufferPtr buf, 0 }; - return _ebtablesRemoveSubChains(buf, ifname, chains); + _ebtablesRemoveSubChains(buf, ifname, chains); } -static int +static void ebtablesRemoveTmpSubChains(virBufferPtr buf, const char *ifname) { @@ -3133,10 +3103,10 @@ ebtablesRemoveTmpSubChains(virBufferPtr buf, 0 }; - return _ebtablesRemoveSubChains(buf, ifname, chains); + _ebtablesRemoveSubChains(buf, ifname, chains); } -static int +static void ebtablesRenameTmpSubChain(virBufferPtr buf, bool incoming, const char *ifname, @@ -3159,18 +3129,17 @@ ebtablesRenameTmpSubChain(virBufferPtr buf, virBufferAsprintf(buf, "$EBT -t nat -E %s %s" CMD_SEPARATOR, tmpchain, chain); - return 0; } -static int +static void ebtablesRenameTmpRootChain(virBufferPtr buf, bool incoming, const char *ifname) { - return ebtablesRenameTmpSubChain(buf, incoming, ifname, NULL); + ebtablesRenameTmpSubChain(buf, incoming, ifname, NULL); } -static int +static void ebtablesRenameTmpSubAndRootChains(virBufferPtr buf, const char *ifname) { @@ -3203,8 +3172,6 @@ ebtablesRenameTmpSubAndRootChains(virBufferPtr buf, ebtablesRenameTmpRootChain(buf, true, ifname); ebtablesRenameTmpRootChain(buf, false, ifname); - - return 0; } static void @@ -3543,16 +3510,18 @@ tear_down_tmpebchains: static int ebtablesRemoveBasicRules(const char *ifname) { - return ebtablesCleanAll(ifname); + ebtablesCleanAll(ifname); + return 0; } -static int ebtablesCleanAll(const char *ifname) +static void +ebtablesCleanAll(const char *ifname) { virBuffer buf = VIR_BUFFER_INITIALIZER; if (!ebtables_cmd_path) - return 0; + return; NWFILTER_SET_EBTABLES_SHELLVAR(&buf); @@ -3569,7 +3538,6 @@ static int ebtablesCleanAll(const char *ifname) ebtablesRemoveTmpRootChain(&buf, false, ifname); ebiptablesExecCLI(&buf, true, NULL); - return 0; } @@ -3685,8 +3653,7 @@ ebtablesCreateTmpRootAndSubChains(virBufferPtr buf, virHashKeyValuePairPtr filter_names; const virNWFilterChainPriority *priority; - if (ebtablesCreateTmpRootChain(buf, incoming, ifname) < 0) - return -1; + ebtablesCreateTmpRootChain(buf, incoming, ifname); filter_names = virHashGetItems(chains, ebiptablesFilterOrderSort); @@ -4246,7 +4213,7 @@ err_exit: return ret; } -static int +static void ebiptablesDriverInitCLITools(void) { ebtables_cmd_path = virFindFileInPath("ebtables"); @@ -4260,8 +4227,6 @@ ebiptablesDriverInitCLITools(void) ip6tables_cmd_path = virFindFileInPath("ip6tables"); if (!ip6tables_cmd_path) VIR_WARN("Could not find 'ip6tables' executable"); - - return 0; } /* -- 1.8.5.3
On 03/18/2014 07:36 AM, Daniel P. Berrange wrote:
Many nwfilter methods have an int return value but only ever return 0 and their callers never check the return value either. These methods can all be void.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/nwfilter/nwfilter_ebiptables_driver.c | 157 ++++++++++++------------------ 1 file changed, 61 insertions(+), 96 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
The 'int isTempChain' parameter to various nwfilter methods only takes two values so should be a bool type. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/nwfilter/nwfilter_ebiptables_driver.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 63ccfe6..0927552 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -674,7 +674,7 @@ static void _iptablesRemoveRootChain(virBufferPtr buf, char prefix, bool incoming, const char *ifname, - int isTempChain) + bool isTempChain) { char chain[MAX_CHAINNAME_LENGTH]; char chainPrefix[2] = { @@ -704,7 +704,7 @@ iptablesRemoveRootChain(virBufferPtr buf, bool incoming, const char *ifname) { - _iptablesRemoveRootChain(buf, prefix, incoming, ifname, 0); + _iptablesRemoveRootChain(buf, prefix, incoming, ifname, false); } @@ -715,7 +715,7 @@ iptablesRemoveTmpRootChain(virBufferPtr buf, const char *ifname) { _iptablesRemoveRootChain(buf, prefix, - incoming, ifname, 1); + incoming, ifname, true); } @@ -815,7 +815,7 @@ _iptablesUnlinkRootChain(virBufferPtr buf, const char *basechain, char prefix, bool incoming, const char *ifname, - int isTempChain) + bool isTempChain) { char chain[MAX_CHAINNAME_LENGTH]; char chainPrefix[2] = { @@ -861,7 +861,7 @@ iptablesUnlinkRootChain(virBufferPtr buf, bool incoming, const char *ifname) { _iptablesUnlinkRootChain(buf, - basechain, prefix, incoming, ifname, 0); + basechain, prefix, incoming, ifname, false); } @@ -872,7 +872,7 @@ iptablesUnlinkTmpRootChain(virBufferPtr buf, bool incoming, const char *ifname) { _iptablesUnlinkRootChain(buf, - basechain, prefix, incoming, ifname, 1); + basechain, prefix, incoming, ifname, true); } @@ -2888,7 +2888,7 @@ ebtablesLinkTmpRootChain(virBufferPtr buf, static void _ebtablesRemoveRootChain(virBufferPtr buf, bool incoming, const char *ifname, - int isTempChain) + bool isTempChain) { char chain[MAX_CHAINNAME_LENGTH]; char chainPrefix; @@ -2913,7 +2913,7 @@ static void ebtablesRemoveRootChain(virBufferPtr buf, bool incoming, const char *ifname) { - _ebtablesRemoveRootChain(buf, incoming, ifname, 0); + _ebtablesRemoveRootChain(buf, incoming, ifname, false); } @@ -2921,14 +2921,14 @@ static void ebtablesRemoveTmpRootChain(virBufferPtr buf, bool incoming, const char *ifname) { - _ebtablesRemoveRootChain(buf, incoming, ifname, 1); + _ebtablesRemoveRootChain(buf, incoming, ifname, true); } static void _ebtablesUnlinkRootChain(virBufferPtr buf, bool incoming, const char *ifname, - int isTempChain) + bool isTempChain) { char chain[MAX_CHAINNAME_LENGTH]; char iodev = incoming ? 'i' : 'o'; @@ -2956,7 +2956,7 @@ static void ebtablesUnlinkRootChain(virBufferPtr buf, bool incoming, const char *ifname) { - _ebtablesUnlinkRootChain(buf, incoming, ifname, 0); + _ebtablesUnlinkRootChain(buf, incoming, ifname, false); } @@ -2964,7 +2964,7 @@ static void ebtablesUnlinkTmpRootChain(virBufferPtr buf, bool incoming, const char *ifname) { - _ebtablesUnlinkRootChain(buf, incoming, ifname, 1); + _ebtablesUnlinkRootChain(buf, incoming, ifname, true); } -- 1.8.5.3
On 03/18/2014 07:36 AM, Daniel P. Berrange wrote:
The 'int isTempChain' parameter to various nwfilter methods only takes two values so should be a bool type.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/nwfilter/nwfilter_ebiptables_driver.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)
This touches a lot of the same lines as 1/6 when you changed incoming to bool. Should you squash the two patches to minimize the churn? ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Tue, Mar 18, 2014 at 09:21:14AM -0600, Eric Blake wrote:
On 03/18/2014 07:36 AM, Daniel P. Berrange wrote:
The 'int isTempChain' parameter to various nwfilter methods only takes two values so should be a bool type.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/nwfilter/nwfilter_ebiptables_driver.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)
This touches a lot of the same lines as 1/6 when you changed incoming to bool. Should you squash the two patches to minimize the churn?
I just found it easier to review the change when it was only doing one variable at a time, so I think its nicer to keep them separate Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 03/18/2014 09:47 AM, Daniel P. Berrange wrote:
On Tue, Mar 18, 2014 at 09:21:14AM -0600, Eric Blake wrote:
On 03/18/2014 07:36 AM, Daniel P. Berrange wrote:
The 'int isTempChain' parameter to various nwfilter methods only takes two values so should be a bool type.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/nwfilter/nwfilter_ebiptables_driver.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)
This touches a lot of the same lines as 1/6 when you changed incoming to bool. Should you squash the two patches to minimize the churn?
I just found it easier to review the change when it was only doing one variable at a time, so I think its nicer to keep them separate
And after reading patch 3/6, I also see where doing one thing at a time helps because that patch changed the number of parameters. Okay, I agree: no squashing necessary. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
The CMD_STOPONERR macro uses its parameter as a boolean, so should be passed true rather than 1. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/nwfilter/nwfilter_ebiptables_driver.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 0927552..842d2d2 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -795,7 +795,7 @@ iptablesSetupVirtInPost(virBufferPtr buf, "fi\n", PHYSDEV_IN, ifname, match, ifname, - CMD_STOPONERR(1)); + CMD_STOPONERR(true)); } @@ -3244,7 +3244,7 @@ ebtablesApplyBasicRules(const char *ifname, "%s", chain, macaddr_str, - CMD_STOPONERR(1)); + CMD_STOPONERR(true)); virBufferAsprintf(&buf, CMD_DEF("$EBT -t nat -A %s -p IPv4 -j ACCEPT") CMD_SEPARATOR @@ -3252,7 +3252,7 @@ ebtablesApplyBasicRules(const char *ifname, "%s", chain, - CMD_STOPONERR(1)); + CMD_STOPONERR(true)); virBufferAsprintf(&buf, CMD_DEF("$EBT -t nat -A %s -p ARP -j ACCEPT") CMD_SEPARATOR @@ -3260,7 +3260,7 @@ ebtablesApplyBasicRules(const char *ifname, "%s", chain, - CMD_STOPONERR(1)); + CMD_STOPONERR(true)); virBufferAsprintf(&buf, CMD_DEF("$EBT -t nat -A %s -j DROP") CMD_SEPARATOR @@ -3268,7 +3268,7 @@ ebtablesApplyBasicRules(const char *ifname, "%s", chain, - CMD_STOPONERR(1)); + CMD_STOPONERR(true)); ebtablesLinkTmpRootChain(&buf, true, ifname); ebtablesRenameTmpRootChain(&buf, true, ifname); @@ -3349,7 +3349,7 @@ ebtablesApplyDHCPOnlyRules(const char *ifname, chain_in, macaddr_str, - CMD_STOPONERR(1)); + CMD_STOPONERR(true)); virBufferAsprintf(&buf, CMD_DEF("$EBT -t nat -A %s -j DROP") CMD_SEPARATOR @@ -3357,7 +3357,7 @@ ebtablesApplyDHCPOnlyRules(const char *ifname, "%s", chain_in, - CMD_STOPONERR(1)); + CMD_STOPONERR(true)); num_dhcpsrvrs = (dhcpsrvrs != NULL) ? virNWFilterVarValueGetCardinality(dhcpsrvrs) @@ -3394,7 +3394,7 @@ ebtablesApplyDHCPOnlyRules(const char *ifname, chain_out, (ctr == 0) ? macaddr_str : "ff:ff:ff:ff:ff:ff", srcIPParam != NULL ? srcIPParam : "", - CMD_STOPONERR(1)); + CMD_STOPONERR(true)); } VIR_FREE(srcIPParam); @@ -3411,7 +3411,7 @@ ebtablesApplyDHCPOnlyRules(const char *ifname, "%s", chain_out, - CMD_STOPONERR(1)); + CMD_STOPONERR(true)); ebtablesLinkTmpRootChain(&buf, true, ifname); ebtablesLinkTmpRootChain(&buf, false, ifname); @@ -3476,7 +3476,7 @@ ebtablesApplyDropAllRules(const char *ifname) "%s", chain_in, - CMD_STOPONERR(1)); + CMD_STOPONERR(true)); virBufferAsprintf(&buf, CMD_DEF("$EBT -t nat -A %s -j DROP") CMD_SEPARATOR @@ -3484,7 +3484,7 @@ ebtablesApplyDropAllRules(const char *ifname) "%s", chain_out, - CMD_STOPONERR(1)); + CMD_STOPONERR(true)); ebtablesLinkTmpRootChain(&buf, true, ifname); ebtablesLinkTmpRootChain(&buf, false, ifname); @@ -4180,7 +4180,7 @@ ebiptablesDriverInitWithFirewallD(void) CMD_DEF("$FWC --state") CMD_SEPARATOR CMD_EXEC "%s", - CMD_STOPONERR(1)); + CMD_STOPONERR(true)); if (ebiptablesExecCLI(&buf, false, &output) < 0) { VIR_INFO("firewalld support disabled for nwfilter"); @@ -4249,7 +4249,7 @@ ebiptablesDriverTestCLITools(void) CMD_DEF("$EBT -t nat -L") CMD_SEPARATOR CMD_EXEC "%s", - CMD_STOPONERR(1)); + CMD_STOPONERR(true)); if (ebiptablesExecCLI(&buf, false, &errmsg) < 0) { VIR_FREE(ebtables_cmd_path); @@ -4266,7 +4266,7 @@ ebiptablesDriverTestCLITools(void) CMD_DEF("$IPT -n -L FORWARD") CMD_SEPARATOR CMD_EXEC "%s", - CMD_STOPONERR(1)); + CMD_STOPONERR(true)); if (ebiptablesExecCLI(&buf, false, &errmsg) < 0) { VIR_FREE(iptables_cmd_path); @@ -4283,7 +4283,7 @@ ebiptablesDriverTestCLITools(void) CMD_DEF("$IPT -n -L FORWARD") CMD_SEPARATOR CMD_EXEC "%s", - CMD_STOPONERR(1)); + CMD_STOPONERR(true)); if (ebiptablesExecCLI(&buf, false, &errmsg) < 0) { VIR_FREE(ip6tables_cmd_path); -- 1.8.5.3
On 03/18/2014 07:36 AM, Daniel P. Berrange wrote:
The CMD_STOPONERR macro uses its parameter as a boolean, so should be passed true rather than 1.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/nwfilter/nwfilter_ebiptables_driver.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-)
Umm, ALL callers of CMD_STOPONERR pass 1 (now true). Why not just delete the parameter altogether? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 03/18/2014 09:25 AM, Eric Blake wrote:
On 03/18/2014 07:36 AM, Daniel P. Berrange wrote:
The CMD_STOPONERR macro uses its parameter as a boolean, so should be passed true rather than 1.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/nwfilter/nwfilter_ebiptables_driver.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-)
Umm, ALL callers of CMD_STOPONERR pass 1 (now true). Why not just delete the parameter altogether?
Oh, never mind. Patch 3/6 was missing in my inbox (but I see it in list archives - wonder why some messages are getting delayed), but with that patch, I see uses of CMD_STOPONERR(stopOnError), so we DO pass false in some places. ACK as-is. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 03/18/2014 07:36 AM, Daniel P. Berrange wrote:
This mini-series performs a few style cleanups on the nwfilter code. There should be no functional change in any of these patches.
Daniel P. Berrange (6): Change 'int incoming' to 'bool incoming' in nwfilter code Remove pointless brackets around boolean Remove 'int stopOnError' parameters in nwfilter methods
I'm not sure why mailman hasn't sent 3/6 my way yet, but I reviewed the copy in the archives. ACK.
Remove pointless return values in nwfilter methods Change 'int isTempChain' to bool in nwfilter Change CMD_STOPONERR(1) to use true
src/nwfilter/nwfilter_ebiptables_driver.c | 523 ++++++++++++++---------------- 1 file changed, 241 insertions(+), 282 deletions(-)
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Daniel P. Berrange -
Eric Blake