> On Wed, Aug 10, 2011 at 03:28:27PM +0000, Christian Röller wrote:
> > Christian Röller has proposed merging lp:~christian-
> roeller/hipl/whitelisting into lp:hipl.
> 
> Did you run
> 
>   make alltests
>   tools/hipl_autobuild.sh

Yes I run make alltests. It seems to be OK, I get no errors and it results with "archives ready for distribution".
I am not really sure how to use the autobuild script. How and from which machine can I run this script manually to build my branch? 
Can it come to problems, although make and make alltests run without ones?

> 
> with this branch?
> 
> > --- hipd/netdev.c     2011-05-25 09:22:19 +0000
> > +++ hipd/netdev.c     2011-08-10 15:28:20 +0000
> > @@ -99,19 +99,26 @@
> > -static void hip_netdev_white_list_add_index(int if_index)
> > +static void hip_netdev_white_list_add_index_and_name(const unsigned int
> if_index, const char *const device_name)
> 
> Please keep lines below 80 characters where easily possible, like here.

Done...

> 
> More examples below...
> 
> > @@ -1122,6 +1131,68 @@
> >
> >  /**
> > + * This functions gives you the interface label for a given ip4 or ip6
> addr.
> 
> IPv4, IPv6 address

Done...

> 
> > + * @param addr address for which you want to know the label
> > + * @param label pointer where the function stores the label
> > + * @return one on success, zero on error and write the label in param label
> 
> Is the label written in both cases?

Done...only if the address exists in the system

> 
> > +int hip_find_label_for_address(const struct sockaddr *const addr, char
> *const label)
> > +{
> > +
> > +    if (addr->sa_family == AF_INET) {
> > +        s4_in = (const struct sockaddr_in *const) (addr);
> > +    }
> > +    if (addr->sa_family == AF_INET6) {
> > +        s6_in = (const struct sockaddr_in6 *const) (addr);
> > +    }
> 
> An else will save you a comparison here.
> 
> > +    getifaddrs(&myaddrs);
> > +    for (ifa = myaddrs; ifa != NULL; ifa = ifa->ifa_next) {
> > +        if (ifa->ifa_addr == NULL) {
> > +            continue;
> > +        }
> > +        if (ifa->ifa_addr->sa_family != AF_INET && ifa->ifa_addr->sa_family
> != AF_INET6) {
> > +            continue;
> > +        }
> 
> These two could be merged.
> 
> > +        if (ifa->ifa_addr->sa_family == AF_INET) {
> > +            s4 = (struct sockaddr_in *) (ifa->ifa_addr);
> 
> pointless ()
> 
> > +        if (ifa->ifa_addr->sa_family == AF_INET6) {
> > +            s6 = (struct sockaddr_in6 *) (ifa->ifa_addr);
> 
> pointless ()
> 
> As above, if you use else here, you could save a comparison.
> 
> > +            if (!memcmp(s4_in, s4, sizeof(s4))) {
> > +                if (strlen(ifa->ifa_name) <= sizeof(label) - 1) {
> > +                    strncpy(label, ifa->ifa_name, sizeof(label) - 1);
> > +                    res = 1;
> > +                } else {
> > +                    res = 0;
> > +                }
> > +                break;
> 
> > +            if (!memcmp(s6_in, s6, sizeof(s6))) {
> > +                if (strlen(ifa->ifa_name) <= sizeof(label) - 1) {
> > +                    strncpy(label, ifa->ifa_name, sizeof(label) - 1);
> > +                    res = 1;
> > +                } else {
> > +                    res = 0;
> > +                }
> > +                break;
> 
> Isn't there a way to avoid this code duplication?

I shorten the entire function, you were right there was a lot overhead in it.

> 
> Diego

Christian