10:06 a.m.
From: "Daniel P. Berrange" <berrange(a)redhat.com>
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(a)redhat.com>
---
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