Re: [patch iproute2 v3 1/4] lib/libnetlink: Add a function rtnl_talk_msg

[Chris Mi]

On 12/25/17 2:46 AM, Chris Mi wrote:
diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index 00e6ce0c..f5f675cf 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -581,36 +581,21 @@ static void rtnl_talk_error(struct nlmsghdr *h, struct 
nlmsgerr *err,
                strerror(-err->error));
  }
  
-static int __rtnl_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n,
-                      struct nlmsghdr **answer,
-                      bool show_rtnl_err, nl_ext_ack_fn_t errfn)
+static int __rtnl_check_ack(struct rtnl_handle *rtnl, struct nlmsghdr **answer,
Make this function __rtnl_talk_msg. Include the assignment of nlmsg_seq
and ack setting using the for loop below and sendmsg() call. All of that
code can be common for both the single and multiple iov case.
Thanks for your suggestion. Done.

+                      bool show_rtnl_err, nl_ext_ack_fn_t errfn,
+                      unsigned int seq)
  {
        int status;
-       unsigned int seq;
-       struct nlmsghdr *h;
+       char *buf;
Please order variables in the reverse xmas tree style used in the net code.
Actually, I divide the variables in two parts, none-struct variables and 
struct variables.
Not sure if that meets the reverse xmac tree style, but I think it is 
more readable.

        struct sockaddr_nl nladdr = { .nl_family = AF_NETLINK };
-       struct iovec iov = {
-               .iov_base = n,
-               .iov_len = n->nlmsg_len
-       };
+       struct nlmsghdr *h;
+       struct iovec iov;
        struct msghdr msg = {
                .msg_name = &nladdr,
                .msg_namelen = sizeof(nladdr),
                .msg_iov = &iov,
                .msg_iovlen = 1,
        };
-       char *buf;
-
-       n->nlmsg_seq = seq = ++rtnl->seq;
-
-       if (answer == NULL)
-               n->nlmsg_flags |= NLM_F_ACK;
-
-       status = sendmsg(rtnl->fd, &msg, 0);
-       if (status < 0) {
-               perror("Cannot talk to rtnetlink");
-               return -1;
-       }
  
  	while (1) {
                status = rtnl_recvmsg(rtnl->fd, &msg, &buf);