Re: Undefined int shift in ifwatchd

[Paul Goyette <paul%whooppee.com@localhost>]

ffs(3) returns the bit index, not the bit value, and the right-most bit 
is index 1.  So you need to use

		addrs &= ~(1 << (i - 1));

rather than

> +		addrs &= ~i;

:)


You could also use

	while ((i = ffs(addrs)) != 0) {
	...
	}

instead of the for(;;) loop.



On Wed, 27 Jan 2016, Martin Husemann wrote:

> On Wed, Jan 27, 2016 at 07:53:27PM +0100, Martin Husemann wrote:
> > How about defining a RTA_ALL_MASK (or however we'd call it, defined as
> > 0x1ff) and write:
> >
> > 	for (i = 1; i & RTA_ALL_MASK; i <<= 1) {
> >
> > ?
>
> Or without global header changes, just use ffs() ?
>
> Martin
>
> Index: ifwatchd.c
> ===================================================================
> RCS file: /cvsroot/src/usr.sbin/ifwatchd/ifwatchd.c,v
> retrieving revision 1.27
> diff -u -p -r1.27 ifwatchd.c
> --- ifwatchd.c	27 Jan 2016 18:55:51 -0000	1.27
> +++ ifwatchd.c	27 Jan 2016 19:31:10 -0000
> @@ -65,7 +65,7 @@ enum event { ARRIVAL, DEPARTURE, UP, DOW
> /* local functions */
> __dead static void usage(void);
> static void dispatch(void*, size_t);
> -static void check_addrs(char *cp, int addrs, enum event ev);
> +static void check_addrs(char *cp, unsigned int addrs, enum event ev);
> static void invoke_script(struct sockaddr *sa, struct sockaddr *dst, enum event ev, int ifindex, const char *ifname_hint);
> static void list_interfaces(const char *ifnames);
> static void check_announce(struct if_announcemsghdr *ifan);
> @@ -287,32 +287,44 @@ work:
> }
>
> static void
> -check_addrs(char *cp, int addrs, enum event ev)
> +check_addrs(char *cp, unsigned int addrs, enum event ev)
> {
> 	struct sockaddr *sa, *ifa = NULL, *brd = NULL;
> 	char ifname_buf[IFNAMSIZ];
> 	const char *ifname;
> 	int ifndx = 0;
> -	unsigned i;
> +	unsigned int i;
>
> 	if (addrs == 0)
> 		return;
> -	for (i = 1; i; i <<= 1) {
> -		if ((i & addrs) == 0)
> -			continue;
> +
> +	for (;;) {
> +		i = ffs(addrs);
> +		if (i == 0)
> +			break;
> +		addrs &= ~i;
> +
> 		sa = (struct sockaddr *)cp;
> -		if (i == RTA_IFP) {
> +		switch (i) {
> +		case RTA_IFP:
> +		{
> 			struct sockaddr_dl * li = (struct sockaddr_dl*)sa;
> 			ifndx = li->sdl_index;
> 			if (!find_interface(ifndx)) {
> 				if (verbose)
> -					printf("ignoring change on interface #%d\n", ifndx);
> +					printf("ignoring change on "
> +					    "interface #%d\n", ifndx);
> 				return;
> 			}
> -		} else if (i == RTA_IFA)
> +			break;
> +		}
> +		case RTA_IFA:
> 			ifa = sa;
> -		else if (i == RTA_BRD)
> +			break;
> +		case RTA_BRD:
> 			brd = sa;
> +			break;
> +		}
> 		RT_ADVANCE(cp, sa);
> 	}
> 	if (ifa != NULL) {
>
>
> !DSPAM:56a91bb7142006842815635!

+------------------+--------------------------+------------------------+
| Paul Goyette     | PGP Key fingerprint:     | E-mail addresses:      |
| (Retired)        | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com   |
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd.org |
+------------------+--------------------------+------------------------+