Re: [B.A.T.M.A.N.] [PATCH v4] alfred: IPv4 multicast distribution support.

[Sven Eckelmann <sven@narfation.org>]

[-- Attachment #1: Type: text/plain, Size: 2085 bytes --]

On Donnerstag, 19. Januar 2017 21:01:04 CET Jonathan Haws wrote:
> @@ -273,10 +276,15 @@ static struct globals *alfred_init(int argc, char *argv[])
>  			return NULL;
>  		case 'p':
>  			sync_period = strtod(optarg, NULL);
> -			globals->sync_period.tv_sec = (int) sync_period;
> -			globals->sync_period.tv_nsec = (double) (sync_period - (int) sync_period) * 1e9;
> +			globals->sync_period.tv_sec = (int)sync_period;
> +			globals->sync_period.tv_nsec = (double)(sync_period - (int)sync_period) * 1e9;
>  			printf(" ** Setting sync interval to: %.9f seconds (%ld.%09ld)\n", sync_period, globals->sync_period.tv_sec, globals->sync_period.tv_nsec);
>  			break;

This doesn't seem to belong in this patch.

> +static int netsock_open4(struct interface *interface)
> +{
> +	int sock;
> +	int sock_mc;
> +	struct sockaddr_in sin4, sin_mc;
> +	struct ip_mreq mreq;
> +	struct ifreq ifr;
> +	int ret;
> +
> +	interface->netsock = -1;
> +	interface->netsock_mcast = -1;
> +
> +	sock = socket(PF_INET, SOCK_DGRAM, IPPROTO_UDP);
> +	if (sock  < 0) {
> +		perror("ipv4: can't open socket");
> +		return -1;
> +	}
> +
> +	sock_mc = socket(PF_INET, SOCK_DGRAM, IPPROTO_UDP);
> +	if (sock_mc  < 0) {
> +		perror("ipv4: can't open mc socket");
> +		return -1;
> +	}

You don't sock in this situation (on errors)?

> +	memset(&ifr, 0, sizeof(ifr));
> +	strncpy(ifr.ifr_name, interface->interface, IFNAMSIZ);
> +	ifr.ifr_name[IFNAMSIZ - 1] = '\0';
> +	if (ioctl(sock_mc, SIOCGIFHWADDR, &ifr) == -1) {
> +		perror("ipv4: can't get MAC address");
> +		goto err4;

And now (see err4) you only close sock_mc? This applies to basically the rest
of the function. So I will skip it and not mention every place which looks
suspicious.

[....]
> +
> +	interface->netsock = sock;
> +	interface->netsock_mcast = sock_mc;
> +
> +	return 0;
> +err4:
> +	close(sock_mc);
> +	return -1;
> +}
> +

You only close sock_mc on errors but not sock?

And please chose something better [1] than err4.

Kind regards,
	Sven

[1] https://static.lwn.net/kerneldoc/process/coding-style.html#centralized-exiting-of-functions

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]