On Tue, May 03, 2016 at 09:07:35AM +0200, Sven Eckelmann wrote: > [...] > > +ifeq ($(origin LIBNL_GENL_CFLAGS) $(origin LIBNL_GENL_LDLIBS), undefined > > undefined) > > + LIBNL_GENL_NAME ?= libnl-genl-3.0 > > + ifeq ($(shell $(PKG_CONFIG) --modversion $(LIBNL_GENL_NAME) > > 2>/dev/null),) > > + $(error No $(LIBNL_GENL_NAME) development libraries found!) > > + endif > > + LIBNL_GENL_CFLAGS += $(shell $(PKG_CONFIG) --cflags $(LIBNL_GENL_NAME)) > > + LIBNL_GENL_LDLIBS += $(shell $(PKG_CONFIG) --libs $(LIBNL_GENL_NAME)) > > +endif > > +CFLAGS += $(LIBNL_GENL_CFLAGS) > > +LDLIBS += $(LIBNL_GENL_LDLIBS) > > @Simon, @Matthias, @Antonio, @Marek: Should the header file be included in > batctl like nl80211.h in iw. Or should we always require the current kernel > header files installed to build batctl?
Once the code is mostly complete and stable, i think a local copy would be good. It does seems to be the common way. While doing development work, it saved me copying the header file around for each change. I can make this change in the next version. > [...] > > +struct mandatory_attr { > > + int attr; > > + int datalen; > > +}; > > + > > +static int last_err; > > + > > +static int invalidate_mandatory_attrs(struct nlattr *attrs[], > > + const struct mandatory_attr *mandatory[], > > + int num) > > +{ > > + int i; > > + int len; > > + > > + for (i = 0; i < num; i++) { > > + if (!attrs[mandatory[i]->attr]) > > + return EINVAL; > > + len = nla_len(attrs[mandatory[i]->attr]); > > + if (mandatory[i]->datalen && (len != mandatory[i]->datalen)) > > + return EINVAL; > > + } > > + > > + return 0; > > +} > > + > > +static const struct mandatory_attr mandatory_attr_version= { > > + BATADV_ATTR_VERSION, 0 }; > [...] > > +static const struct mandatory_attr *info_hard_mandatory[] = { > > + &mandatory_attr_version, > > + &mandatory_attr_algo_name, > > + &mandatory_attr_hard_ifname, > > + &mandatory_attr_hard_address, > > +}; > > + > > +static int info_callback(struct nl_msg *msg, void *arg __unused) > > +{ > [...] > > + if (nla_parse(attrs, BATADV_ATTR_MAX, genlmsg_attrdata(ghdr, 0), > > + genlmsg_len(ghdr), NULL)) { > > + fputs("Received invalid data from kernel.", stderr); > > + exit(1); > > + } > > + > > + if (invalidate_mandatory_attrs(attrs, info_mandatory, > > + ARRAY_SIZE(info_mandatory))) { > > + fputs("Missing/invalid attributes from kernel\n", stderr); > > + exit(1); > > + } > > Interesting idea to check the mandatory attributes with a common function. But > shouldn't be the length checked by the nl_parse(..., policy) [1]? This is > especially important because you don't check the type. I could be missing something, but i don't see how to check the type, i.e. string, u8, u16, etc. The header file is defining attribute types: enum { BATADV_ATTR_UNSPEC, BATADV_ATTR_VERSION, BATADV_ATTR_ALGO_NAME, BATADV_ATTR_MESH_IFINDEX, There does not seem to be a way to say that BATADV_ATTR_VERSION is also an NLA_STRING. However, yes, i should do the length checking as part of nl_parse. Andrew