[hipl-dev] Re: [Merge] lp:~christian-roeller/hipl/whitelisting into lp:hipl
- From: Diego Biurrun <diego@xxxxxxxxxx>
- To: hipl-dev@xxxxxxxxxxxxx
- Date: Thu, 11 Aug 2011 14:31:37 +0200
review needs-fixing
On Thu, Aug 11, 2011 at 11:48:47AM +0000, Christian Röller wrote:
> Christian Röller has proposed merging lp:~christian-roeller/hipl/whitelisting
> into lp:hipl.
>
> --- hipd/netdev.c 2011-05-25 09:22:19 +0000
> +++ hipd/netdev.c 2011-08-11 11:48:38 +0000
> @@ -99,19 +99,27 @@
> 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);
This is an opportunity for breaking a line, as is
> @@ -119,17 +127,20 @@
>
> -static int hip_netdev_is_in_white_list(int if_index)
> +static int hip_netdev_is_in_white_list(const unsigned int if_index, const
> char *const device_name)
this one
> + if (hip_netdev_white_list_count > 0) {
> + for (unsigned int i = 0; i < hip_netdev_white_list_count; i++) {
> + if (hip_netdev_white_list[i].if_index == if_index &&
> + !strncmp(hip_netdev_white_list[i].if_label, device_name,
> sizeof(hip_netdev_white_list[0].if_label))) {
and this one.
> @@ -1122,6 +1132,40 @@
>
> /**
> + * This functions gives you the interface label for a given IPv4 or IPv6
> address.
This sentence explains why starting a sentence with "this sentence" is
redundant and weird.
> + * @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
> + * if the given addr exists in the system
address
The usual convention is to return zero on success, not one.
> +int hip_find_label_for_address(const struct sockaddr *const addr, char
> *const label)
long line
> +{
> + int res = 0;
> + 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)) {
long line
> @@ -1213,6 +1252,18 @@
>
> + /* find interface label for address and check if it is
> whitelisted or not */
> + if (is_add) {
> + char label[IF_NAMESIZE];
> + if (hip_find_label_for_address(addr, label)) {
> + if ((!hip_netdev_is_in_white_list(ifa->ifa_index,
> label))) {
> + HIP_DEBUG("Interface:<%s> is not whitelisted\n",
> label);
> + continue;
The conditions can be merged.
> --- hipd/netdev.h 2011-05-18 15:12:07 +0000
> +++ hipd/netdev.h 2011-08-11 11:48:38 +0000
> @@ -56,4 +56,6 @@
>
> +int hip_find_label_for_address(const struct sockaddr *const addr, char
> *const label);
long line
Diego
Other related posts:
