[avahi] [PATCH] [RFC] Untested interface limitation patch

[Lennart Poettering]

On Sun, 11.05.08 23:27, Stefan de Konink (avahi at ml.kinkrsoftware.nl) wrote:

Hi!

> Yesterday I proposed a patch on to limit avahi to a specific interface 
> based on the source address when opening each socket. As a real Michael 
> Niedermayer, Sjoerd shot the patch on sight and suggested an interface 
> based approach.

I am confused.

> This patch tries to accomplish to limit the used interfaces based on a user 
> defined list in the daemon configuration file. The idea would be if the 
> list is not defined, it will just process every found interface, if the 
> list is present, it will iterate over the list for every found interface. A 
> todo is clean the list from duplicates.

I am not sure it is really necessary to filter out dupes. If there are
dupes it doesn't have any impact on the logic, so if the user likes to
waste some memory he is welcome to do so, but i wouldn't really shed a
tear about that. I mean, if I as the maintainer of Avahi would have
coded this I'd have been too lazy to add the filtering code here, and you
could even argue that having this complicates the code for no real
benefit.

I am mostly happy for the patch. Before I'll merge I'd however ask you
to rename the option to something to "interface-allow" or
"allow-interfaces" or so. I'd consider "bind-interfaces" misleading,
since this suggests that some kind of bind() is involved here, which
however is not the case.

I'd prefer if we'd also get an interface blacklist at the same time
as a whitelist, but that wouldn't hinder me to merge your
patch. (i.e. "deny-interfaces" would be cool in addition to
"allow-interface").

> -int avahi_interface_is_relevant(AvahiInterface *i) {
> +static int avahi_interface_is_relevant_iter(AvahiInterface *i) {

Hmm, why did you call this "_iter"? I see no iterator involved here */

>      AvahiInterfaceAddress *a;
>      
> -    assert(i);
> +    assert(i); // Not really required

Hehe, *no* assert is really required. Also /me doesn't like C++ style
comments. So please leave this as it is right now. /me likes paranoid asserts.

/* This is C */

// this is C++


Hmm, avahi_server_config_free() needs updating, too.

And finally, man/avahi-daemon.conf.5.xml.in and
avahi-daemon/avahi-daemon.conf need updating as well.

Otherwise I am happy, no further nitpicking ;-)

Thanks for your patch!

Lennart

-- 
Lennart Poettering                        Red Hat, Inc.
lennart [at] poettering [dot] net         ICQ# 11060553
http://0pointer.net/lennart/           GnuPG 0x1A015CC4