Hi,

I just got one issue that was raised by Stefan before, but has not been fixed so far (more inline).

On 22.08.2011, at 21:05, Christian Röller wrote:
> Christian Röller has proposed merging lp:~christian-roeller/hipl/whitelisting into lp:hipl.
> 
> Requested reviews:
>  Stefan Götz (stefan.goetz)
>  Diego Biurrun (diego-biurrun)
> 
> For more details, see:
> https://code.launchpad.net/~christian-roeller/hipl/whitelisting/+merge/72487

> /**
> - * Add a network interface index number to the list of white listed
> + * Add a network interface index number plus label to the list of whitelisted
>  * network interfaces.
>  *
> - * @param if_index the network interface index to be white listed
> + * @param if_index the network interface index to be whitelisted
> + * @param device_name the network interface label to be whitelisted
>  */
> -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)
> {
>     if (hip_netdev_white_list_count < HIP_NETDEV_MAX_WHITE_LIST) {
> -        hip_netdev_white_list[hip_netdev_white_list_count++] = if_index;
> +        hip_netdev_white_list[hip_netdev_white_list_count].if_index = if_index;
> +        strncpy(hip_netdev_white_list[hip_netdev_white_list_count].if_label,
> +                device_name, sizeof(hip_netdev_white_list[0].if_label) - 1);

Why don't you strncpy the complete sizeof? Either the string is null-terminated, in which case you should copy the 0 as well, or it is not, in which case you will miss the last letter of the label. What happens with label is of length 1?

> /**
> - * Add a network interface index number to the list of white listed
> + * Add a network interface index number to the list of whitelisted
>  * network interfaces by name.
>  *
> - * @param device_name the name of the device to be white listed
> + * @param device_name the name of the device to be whitelisted
>  * @return 1 on success, 0 on error
>  */
> int hip_netdev_white_list_add(const char *const device_name)
> @@ -146,19 +160,18 @@
>     int          sock = 0;
>     int          ret  = 0;
> 
> -    strncpy(ifr.ifr_name, device_name, IF_NAMESIZE);
> +    strncpy(ifr.ifr_name, device_name, sizeof(ifr.ifr_name) - 1);

Same here.

> /**
> + * Gives you the interface label for a given IPv4 or IPv6 address.
> + *
> + * @param addr address for which you want to know the label
> + * @param label pointer where the function stores the label
> + * @return zero on success, one on error and write the label
> + *         in param label if the given address exists in the system
> + */
> +int hip_find_label_for_address(const struct sockaddr *const addr,
> +                               char *const label)
> +{
> +    int             res = 1;
> +    struct ifaddrs *myaddrs, *ifa = NULL;
> +
> +    getifaddrs(&myaddrs);
> +    for (ifa = myaddrs; ifa != NULL; ifa = ifa->ifa_next) {
> +        if (ifa->ifa_addr == NULL ||
> +            (ifa->ifa_addr->sa_family != AF_INET &&
> +             ifa->ifa_addr->sa_family != AF_INET6)) {
> +            continue;
> +        }
> +
> +        if (!memcmp(addr, ifa->ifa_addr, sizeof(addr))) {
> +            if (strlen(ifa->ifa_name) <= sizeof(label) - 1) {
> +                strncpy(label, ifa->ifa_name, sizeof(label) - 1);

And here.



--
Dipl.-Inform. Rene Hummen, Ph.D. Student
Chair of Communication and Distributed Systems
RWTH Aachen University, Germany
tel: +49 241 80 20772
web: http://www.comsys.rwth-aachen.de/team/rene-hummen/