[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH V1 4/9] Add a mac chain

On 10/26/2011 09:12 AM, Stefan Berger wrote:
> With hunks borrowed from one of David Steven's previous patches, we now
> add the capability of having a 'mac' chain which is useful to filter
> for multiple valid MAC addresses.
> Signed-off-by: David L Stevens <dlstevens us ibm com>
> Signed-off-by: Stefan Berger <stefanb linux vnet ibm com>

> +    char protostr[16] = { 0, };

A bit oversized...

>      PRINT_ROOT_CHAIN(rootchain, chainPrefix, ifname);
>      PRINT_CHAIN(chain, chainPrefix, ifname,
>                  (filtername) ? filtername : l3_protocols[protoidx].val);
> +    switch (protoidx) {
> +    case L2_PROTO_MAC_IDX:
> +        break;
> +    default:
> +        snprintf(protostr, sizeof(protostr), "-p 0x%04x ",

for a max of 11 bytes (including trailing NUL) ever printed into it, but
not the end of the world.  And since you didn't check snprintf results
for failure, if we ever change the size of protostr or the length of
what we print into it, it's a slight maintenance risk we are taking on,
compared to dynamic allocation that always gets the right length.  But I
don't know if it is worth replacing this snprintf with
virAsprintf/VIR_FREE overhead, so I can live with it as is.

> @@ -2918,7 +2930,7 @@ ebtablesCreateTmpSubChain(ebiptablesRule
>                        CMD_DEF("%s -t %s -N %s") CMD_SEPARATOR
>                        CMD_EXEC
>                        "%s"
> -                      CMD_DEF("%s -t %s -%%c %s %%s -p 0x%x -j %s")
> +                      CMD_DEF("%s -t %s -%%c %s %%s %s -j %s")

This results in output with a double space, either:

...%s  -j ...


...%s -p 0xnnnn  -j ...

Also not the end of the world, but you may want to remove the extra
space before the -j.

ACK.  There is some conflict resolution needed in nwfilter.rng, but that
should be trivial.

Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]