Avoid casts between unsigned char * and struct nlmsghdr

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: NDaniel P. Berrange <berrange@redhat.com>

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)
 {

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;
 

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 */

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;
 }

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;

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);