From: "Daniel P. Berrange" <berrange@xxxxxxxxxx> The virNetlinkCommand() method takes an 'unsigned char **' parameter to be filled with the received netlink message. The callers then immediately cast this to 'struct nlmsghdr', triggering (bogus) warnings about increasing alignment requirements util/virnetdev.c: In function 'virNetDevLinkDump': util/virnetdev.c:1300:12: warning: cast increases required alignment of target type [-Wcast-align] resp = (struct nlmsghdr *)*recvbuf; ^ util/virnetdev.c: In function 'virNetDevSetVfConfig': util/virnetdev.c:1429:12: warning: cast increases required alignment of target type [-Wcast-align] resp = (struct nlmsghdr *)recvbuf; Since all callers cast to 'struct nlmsghdr' we can avoid the warning problem entirely by simply changing the signature of virNetlinkCommand to return a 'struct nlmsghdr **' instead of 'unsigned char **'. The way we do the cast inside virNetlinkCommand does not have any alignment issues. Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx> --- src/util/virnetdev.c | 33 +++++++++------------------------ src/util/virnetdev.h | 1 - src/util/virnetdevmacvlan.c | 30 +++++++++++------------------- src/util/virnetdevvportprofile.c | 23 ++++++----------------- src/util/virnetlink.c | 14 ++++++++------ src/util/virnetlink.h | 9 +++++++-- 6 files changed, 41 insertions(+), 69 deletions(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 00e0f94..7ffaac1 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -1226,8 +1226,6 @@ static struct nla_policy ifla_vf_policy[IFLA_VF_MAX+1] = { * @ifindex: The interface index; may be < 0 if ifname is given * @nlattr: pointer to a pointer of netlink attributes that will contain * the results - * @recvbuf: Pointer to the buffer holding the returned netlink response - * message; free it, once not needed anymore * @src_pid: pid used for nl_pid of the local end of the netlink message * (0 == "use getpid()") * @dst_pid: pid of destination nl_pid if the kernel @@ -1241,11 +1239,10 @@ static struct nla_policy ifla_vf_policy[IFLA_VF_MAX+1] = { int virNetDevLinkDump(const char *ifname, int ifindex, struct nlattr **tb, - unsigned char **recvbuf, uint32_t src_pid, uint32_t dst_pid) { int rc = -1; - struct nlmsghdr *resp; + struct nlmsghdr *resp = NULL; struct nlmsgerr *err; struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC, @@ -1254,8 +1251,6 @@ virNetDevLinkDump(const char *ifname, int ifindex, unsigned int recvbuflen; struct nl_msg *nl_msg; - *recvbuf = NULL; - if (ifname && ifindex <= 0 && virNetDevGetIndex(ifname, &ifindex) < 0) return -1; @@ -1290,15 +1285,13 @@ virNetDevLinkDump(const char *ifname, int ifindex, } # endif - if (virNetlinkCommand(nl_msg, recvbuf, &recvbuflen, + if (virNetlinkCommand(nl_msg, &resp, &recvbuflen, src_pid, dst_pid, NETLINK_ROUTE, 0) < 0) goto cleanup; - if (recvbuflen < NLMSG_LENGTH(0) || *recvbuf == NULL) + if (recvbuflen < NLMSG_LENGTH(0) || resp == NULL) goto malformed_resp; - resp = (struct nlmsghdr *)*recvbuf; - switch (resp->nlmsg_type) { case NLMSG_ERROR: err = (struct nlmsgerr *)NLMSG_DATA(resp); @@ -1326,9 +1319,8 @@ virNetDevLinkDump(const char *ifname, int ifindex, } rc = 0; cleanup: - if (rc < 0) - VIR_FREE(*recvbuf); nlmsg_free(nl_msg); + VIR_FREE(resp); return rc; malformed_resp: @@ -1348,9 +1340,8 @@ virNetDevSetVfConfig(const char *ifname, int ifindex, int vf, int vlanid, uint32_t (*getPidFunc)(void)) { int rc = -1; - struct nlmsghdr *resp; + struct nlmsghdr *resp = NULL; struct nlmsgerr *err; - unsigned char *recvbuf = NULL; unsigned int recvbuflen = 0; uint32_t pid = 0; struct nl_msg *nl_msg; @@ -1419,15 +1410,13 @@ virNetDevSetVfConfig(const char *ifname, int ifindex, int vf, } } - if (virNetlinkCommand(nl_msg, &recvbuf, &recvbuflen, 0, pid, + if (virNetlinkCommand(nl_msg, &resp, &recvbuflen, 0, pid, NETLINK_ROUTE, 0) < 0) goto cleanup; - if (recvbuflen < NLMSG_LENGTH(0) || recvbuf == NULL) + if (recvbuflen < NLMSG_LENGTH(0) || resp == NULL) goto malformed_resp; - resp = (struct nlmsghdr *)recvbuf; - switch (resp->nlmsg_type) { case NLMSG_ERROR: err = (struct nlmsgerr *)NLMSG_DATA(resp); @@ -1453,7 +1442,7 @@ virNetDevSetVfConfig(const char *ifname, int ifindex, int vf, rc = 0; cleanup: nlmsg_free(nl_msg); - VIR_FREE(recvbuf); + VIR_FREE(resp); return rc; malformed_resp: @@ -1528,18 +1517,15 @@ virNetDevGetVfConfig(const char *ifname, int vf, virMacAddrPtr mac, int *vlanid) { int rc = -1; - unsigned char *recvbuf = NULL; struct nlattr *tb[IFLA_MAX + 1] = {NULL, }; int ifindex = -1; - rc = virNetDevLinkDump(ifname, ifindex, tb, &recvbuf, 0, 0); + rc = virNetDevLinkDump(ifname, ifindex, tb, 0, 0); if (rc < 0) return rc; rc = virNetDevParseVfConfig(tb, vf, mac, vlanid); - VIR_FREE(recvbuf); - return rc; } @@ -1689,7 +1675,6 @@ int virNetDevLinkDump(const char *ifname ATTRIBUTE_UNUSED, int ifindex ATTRIBUTE_UNUSED, struct nlattr **tb ATTRIBUTE_UNUSED, - unsigned char **recvbuf ATTRIBUTE_UNUSED, uint32_t src_pid ATTRIBUTE_UNUSED, uint32_t dst_pid ATTRIBUTE_UNUSED) { diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h index 06d0650..551ea22 100644 --- a/src/util/virnetdev.h +++ b/src/util/virnetdev.h @@ -111,7 +111,6 @@ int virNetDevGetVirtualFunctions(const char *pfname, int virNetDevLinkDump(const char *ifname, int ifindex, struct nlattr **tb, - unsigned char **recvbuf, uint32_t src_pid, uint32_t dst_pid) ATTRIBUTE_RETURN_CHECK; diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index 2578ff0..e76e94c 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -110,11 +110,10 @@ virNetDevMacVLanCreate(const char *ifname, int *retry) { int rc = -1; - struct nlmsghdr *resp; + struct nlmsghdr *resp = NULL; struct nlmsgerr *err; struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC }; int ifindex; - unsigned char *recvbuf = NULL; unsigned int recvbuflen; struct nl_msg *nl_msg; struct nlattr *linkinfo, *info_data; @@ -163,16 +162,14 @@ virNetDevMacVLanCreate(const char *ifname, nla_nest_end(nl_msg, linkinfo); - if (virNetlinkCommand(nl_msg, &recvbuf, &recvbuflen, 0, 0, + if (virNetlinkCommand(nl_msg, &resp, &recvbuflen, 0, 0, NETLINK_ROUTE, 0) < 0) { goto cleanup; } - if (recvbuflen < NLMSG_LENGTH(0) || recvbuf == NULL) + if (recvbuflen < NLMSG_LENGTH(0) || resp == NULL) goto malformed_resp; - resp = (struct nlmsghdr *)recvbuf; - switch (resp->nlmsg_type) { case NLMSG_ERROR: err = (struct nlmsgerr *)NLMSG_DATA(resp); @@ -206,7 +203,7 @@ virNetDevMacVLanCreate(const char *ifname, rc = 0; cleanup: nlmsg_free(nl_msg); - VIR_FREE(recvbuf); + VIR_FREE(resp); return rc; malformed_resp: @@ -232,10 +229,9 @@ buffer_too_small: int virNetDevMacVLanDelete(const char *ifname) { int rc = -1; - struct nlmsghdr *resp; + struct nlmsghdr *resp = NULL; struct nlmsgerr *err; struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC }; - unsigned char *recvbuf = NULL; unsigned int recvbuflen; struct nl_msg *nl_msg; @@ -252,16 +248,14 @@ int virNetDevMacVLanDelete(const char *ifname) if (nla_put(nl_msg, IFLA_IFNAME, strlen(ifname)+1, ifname) < 0) goto buffer_too_small; - if (virNetlinkCommand(nl_msg, &recvbuf, &recvbuflen, 0, 0, + if (virNetlinkCommand(nl_msg, &resp, &recvbuflen, 0, 0, NETLINK_ROUTE, 0) < 0) { goto cleanup; } - if (recvbuflen < NLMSG_LENGTH(0) || recvbuf == NULL) + if (recvbuflen < NLMSG_LENGTH(0) || resp == NULL) goto malformed_resp; - resp = (struct nlmsghdr *)recvbuf; - switch (resp->nlmsg_type) { case NLMSG_ERROR: err = (struct nlmsgerr *)NLMSG_DATA(resp); @@ -286,7 +280,7 @@ int virNetDevMacVLanDelete(const char *ifname) rc = 0; cleanup: nlmsg_free(nl_msg); - VIR_FREE(recvbuf); + VIR_FREE(resp); return rc; malformed_resp: @@ -477,7 +471,7 @@ static int instance2str(const unsigned char *p, char *dst, size_t size) /** * virNetDevMacVLanVPortProfileCallback: * - * @msg: The buffer containing the received netlink message + * @hdr: The buffer containing the received netlink header + payload * @length: The length of the received netlink message. * @peer: The netling sockaddr containing the peer information * @handled: Contains information if the message has been replied to yet @@ -489,8 +483,8 @@ static int instance2str(const unsigned char *p, char *dst, size_t size) */ static void -virNetDevMacVLanVPortProfileCallback(unsigned char *msg, - int length, +virNetDevMacVLanVPortProfileCallback(struct nlmsghdr *hdr, + unsigned int length, struct sockaddr_nl *peer, bool *handled, void *opaque) @@ -510,7 +504,6 @@ virNetDevMacVLanVPortProfileCallback(unsigned char *msg, *tb_vfinfo[IFLA_VF_MAX + 1], *tb_vfinfo_list; struct ifinfomsg ifinfo; - struct nlmsghdr *hdr; void *data; int rem; char *ifname; @@ -520,7 +513,6 @@ virNetDevMacVLanVPortProfileCallback(unsigned char *msg, pid_t virip_pid = 0; char macaddr[VIR_MAC_STRING_BUFLEN]; - hdr = (struct nlmsghdr *) msg; data = nlmsg_data(hdr); /* Quickly decide if we want this or not */ diff --git a/src/util/virnetdevvportprofile.c b/src/util/virnetdevvportprofile.c index bb97e3a..13ccbab 100644 --- a/src/util/virnetdevvportprofile.c +++ b/src/util/virnetdevvportprofile.c @@ -588,13 +588,12 @@ virNetDevVPortProfileOpSetLink(const char *ifname, int ifindex, uint8_t op) { int rc = -1; - struct nlmsghdr *resp; + struct nlmsghdr *resp = NULL; struct nlmsgerr *err; struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC, .ifi_index = ifindex, }; - unsigned char *recvbuf = NULL; unsigned int recvbuflen = 0; int src_pid = 0; uint32_t dst_pid = 0; @@ -711,15 +710,13 @@ virNetDevVPortProfileOpSetLink(const char *ifname, int ifindex, goto cleanup; } - if (virNetlinkCommand(nl_msg, &recvbuf, &recvbuflen, + if (virNetlinkCommand(nl_msg, &resp, &recvbuflen, src_pid, dst_pid, NETLINK_ROUTE, 0) < 0) goto cleanup; - if (recvbuflen < NLMSG_LENGTH(0) || recvbuf == NULL) + if (recvbuflen < NLMSG_LENGTH(0) || resp == NULL) goto malformed_resp; - resp = (struct nlmsghdr *)recvbuf; - switch (resp->nlmsg_type) { case NLMSG_ERROR: err = (struct nlmsgerr *)NLMSG_DATA(resp); @@ -744,7 +741,7 @@ virNetDevVPortProfileOpSetLink(const char *ifname, int ifindex, rc = 0; cleanup: nlmsg_free(nl_msg); - VIR_FREE(recvbuf); + VIR_FREE(resp); return rc; malformed_resp: @@ -784,7 +781,6 @@ virNetDevVPortProfileGetNthParent(const char *ifname, int ifindex, unsigned int { int rc; struct nlattr *tb[IFLA_MAX + 1] = { NULL, }; - unsigned char *recvbuf = NULL; bool end = false; unsigned int i = 0; @@ -794,7 +790,7 @@ virNetDevVPortProfileGetNthParent(const char *ifname, int ifindex, unsigned int return -1; while (!end && i <= nthParent) { - rc = virNetDevLinkDump(ifname, ifindex, tb, &recvbuf, 0, 0); + rc = virNetDevLinkDump(ifname, ifindex, tb, 0, 0); if (rc < 0) break; @@ -803,7 +799,6 @@ virNetDevVPortProfileGetNthParent(const char *ifname, int ifindex, unsigned int IFNAMSIZ)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("buffer for root interface name is too small")); - VIR_FREE(recvbuf); return -1; } *parent_ifindex = ifindex; @@ -815,8 +810,6 @@ virNetDevVPortProfileGetNthParent(const char *ifname, int ifindex, unsigned int } else end = true; - VIR_FREE(recvbuf); - i++; } @@ -843,7 +836,6 @@ virNetDevVPortProfileOpCommon(const char *ifname, int ifindex, int rc; int src_pid = 0; uint32_t dst_pid = 0; - unsigned char *recvbuf = NULL; struct nlattr *tb[IFLA_MAX + 1] = { NULL , }; int repeats = STATUS_POLL_TIMEOUT_USEC / STATUS_POLL_INTERVL_USEC; uint16_t status = 0; @@ -876,7 +868,7 @@ virNetDevVPortProfileOpCommon(const char *ifname, int ifindex, } while (--repeats >= 0) { - rc = virNetDevLinkDump(NULL, ifindex, tb, &recvbuf, src_pid, dst_pid); + rc = virNetDevLinkDump(NULL, ifindex, tb, src_pid, dst_pid); if (rc < 0) goto cleanup; @@ -899,8 +891,6 @@ virNetDevVPortProfileOpCommon(const char *ifname, int ifindex, } usleep(STATUS_POLL_INTERVL_USEC); - - VIR_FREE(recvbuf); } if (status == PORT_PROFILE_RESPONSE_INPROGRESS) { @@ -910,7 +900,6 @@ virNetDevVPortProfileOpCommon(const char *ifname, int ifindex, } cleanup: - VIR_FREE(recvbuf); return rc; } diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c index 0b36fdc..af1985c 100644 --- a/src/util/virnetlink.c +++ b/src/util/virnetlink.c @@ -174,7 +174,7 @@ virNetlinkShutdown(void) * buffer will be returned. */ int virNetlinkCommand(struct nl_msg *nl_msg, - unsigned char **respbuf, unsigned int *respbuflen, + struct nlmsghdr **resp, unsigned int *respbuflen, uint32_t src_pid, uint32_t dst_pid, unsigned int protocol, unsigned int groups) { @@ -257,7 +257,8 @@ int virNetlinkCommand(struct nl_msg *nl_msg, goto error; } - *respbuflen = nl_recv(nlhandle, &nladdr, respbuf, NULL); + *respbuflen = nl_recv(nlhandle, &nladdr, + (unsigned char **)resp, NULL); if (*respbuflen <= 0) { virReportSystemError(errno, "%s", _("nl_recv failed")); @@ -265,8 +266,8 @@ int virNetlinkCommand(struct nl_msg *nl_msg, } error: if (rc == -1) { - VIR_FREE(*respbuf); - *respbuf = NULL; + VIR_FREE(*resp); + *resp = NULL; *respbuflen = 0; } @@ -324,13 +325,14 @@ virNetlinkEventCallback(int watch, void *opaque) { virNetlinkEventSrvPrivatePtr srv = opaque; - unsigned char *msg; + struct nlmsghdr *msg; struct sockaddr_nl peer; struct ucred *creds = NULL; int i, length; bool handled = false; - length = nl_recv(srv->netlinknh, &peer, &msg, &creds); + length = nl_recv(srv->netlinknh, &peer, + (unsigned char **)&msg, &creds); if (length == 0) return; diff --git a/src/util/virnetlink.h b/src/util/virnetlink.h index 064f3d1..9a69a0b 100644 --- a/src/util/virnetlink.h +++ b/src/util/virnetlink.h @@ -41,6 +41,7 @@ struct nl_msg; struct sockaddr_nl; struct nlattr; +struct nlmsghdr; # endif /* __linux__ */ @@ -48,11 +49,15 @@ int virNetlinkStartup(void); void virNetlinkShutdown(void); int virNetlinkCommand(struct nl_msg *nl_msg, - unsigned char **respbuf, unsigned int *respbuflen, + struct nlmsghdr **resp, unsigned int *respbuflen, uint32_t src_pid, uint32_t dst_pid, unsigned int protocol, unsigned int groups); -typedef void (*virNetlinkEventHandleCallback)(unsigned char *msg, int length, struct sockaddr_nl *peer, bool *handled, void *opaque); +typedef void (*virNetlinkEventHandleCallback)(struct nlmsghdr *, + unsigned int length, + struct sockaddr_nl *peer, + bool *handled, + void *opaque); typedef void (*virNetlinkEventRemoveCallback)(int watch, const virMacAddrPtr macaddr, void *opaque); -- 1.8.1.4 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list