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

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

 



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@xxxxxxxxxx>
> Signed-off-by: Stefan Berger <stefanb@xxxxxxxxxxxxxxxxxx>
> 

> +    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 ...

or

...%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@xxxxxxxxxx    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]