| Message ID | 20131220082107.0467d57c@nehalam.linuxnetplumber.net |
|---|---|
| State | Accepted, archived |
| Delegated to: | stephen hemminger |
| Headers | show |
On Fri, Dec 20, 2013 at 08:21:07AM -0800, Stephen Hemminger wrote: > I took your idea and enhanced it to all of iproute2 by doing the following: > > From c4b6330a3a033bd9c9b0664c5f844493137ae599 Mon Sep 17 00:00:00 2001 > From: Stephen Hemminger <stephen@networkplumber.org> > Date: Fri, 20 Dec 2013 08:15:02 -0800 > Subject: [PATCH] check return value of rtnl_send and related functions > MIME-Version: 1.0 > Content-Type: text/plain; charset=UTF-8 > Content-Transfer-Encoding: 8bit > > Use warn_unused_result to enforce checking return value of rtnl_send, > and fix where the errors are. > [...] > diff --git a/ip/iplink.c b/ip/iplink.c > index 58b6c20..e0c14e6 100644 > --- a/ip/iplink.c > +++ b/ip/iplink.c > @@ -178,7 +178,10 @@ static int iplink_have_newlink(void) > req.n.nlmsg_type = RTM_NEWLINK; > req.i.ifi_family = AF_UNSPEC; > > - rtnl_send(&rth, &req.n, req.n.nlmsg_len); > + if (rtnl_send(&rth, &req.n, req.n.nlmsg_len) < 0) { > + perror("request send failed"); > + exit(1); > + } > rtnl_listen(&rth, accept_msg, NULL); > } > return have_rtnl_newlink; This one exits instead of falling back to IOCTL. iplink_have_newlink() is called even from iplink_usage(). This even prevents from printing usage. I think failure in this very place should be treated as I suggested. -- Petr
On Thu, 2 Jan 2014 08:42:57 +0100 Petr Pisar <ppisar@redhat.com> wrote: > On Fri, Dec 20, 2013 at 08:21:07AM -0800, Stephen Hemminger wrote: > > I took your idea and enhanced it to all of iproute2 by doing the following: > > > > From c4b6330a3a033bd9c9b0664c5f844493137ae599 Mon Sep 17 00:00:00 2001 > > From: Stephen Hemminger <stephen@networkplumber.org> > > Date: Fri, 20 Dec 2013 08:15:02 -0800 > > Subject: [PATCH] check return value of rtnl_send and related functions > > MIME-Version: 1.0 > > Content-Type: text/plain; charset=UTF-8 > > Content-Transfer-Encoding: 8bit > > > > Use warn_unused_result to enforce checking return value of rtnl_send, > > and fix where the errors are. > > > [...] > > diff --git a/ip/iplink.c b/ip/iplink.c > > index 58b6c20..e0c14e6 100644 > > --- a/ip/iplink.c > > +++ b/ip/iplink.c > > @@ -178,7 +178,10 @@ static int iplink_have_newlink(void) > > req.n.nlmsg_type = RTM_NEWLINK; > > req.i.ifi_family = AF_UNSPEC; > > > > - rtnl_send(&rth, &req.n, req.n.nlmsg_len); > > + if (rtnl_send(&rth, &req.n, req.n.nlmsg_len) < 0) { > > + perror("request send failed"); > > + exit(1); > > + } > > rtnl_listen(&rth, accept_msg, NULL); > > } > > return have_rtnl_newlink; > > This one exits instead of falling back to IOCTL. iplink_have_newlink() is > called even from iplink_usage(). This even prevents from printing usage. > > I think failure in this very place should be treated as I suggested. > > -- Petr The code gets used in multiple paths, some should fail and others have fallback. I will fix the cases where fallback is possible. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/libnetlink.h b/include/libnetlink.h index ec3d657..fe7d5d3 100644 --- a/include/libnetlink.h +++ b/include/libnetlink.h @@ -22,13 +22,22 @@ struct rtnl_handle extern int rcvbuf; -extern int rtnl_open(struct rtnl_handle *rth, unsigned subscriptions); -extern int rtnl_open_byproto(struct rtnl_handle *rth, unsigned subscriptions, int protocol); +extern int rtnl_open(struct rtnl_handle *rth, unsigned subscriptions) + __attribute__((warn_unused_result)); + +extern int rtnl_open_byproto(struct rtnl_handle *rth, unsigned subscriptions, + int protocol) + __attribute__((warn_unused_result)); + extern void rtnl_close(struct rtnl_handle *rth); -extern int rtnl_wilddump_request(struct rtnl_handle *rth, int fam, int type); +extern int rtnl_wilddump_request(struct rtnl_handle *rth, int fam, int type) + __attribute__((warn_unused_result)); extern int rtnl_wilddump_req_filter(struct rtnl_handle *rth, int fam, int type, - __u32 filt_mask); -extern int rtnl_dump_request(struct rtnl_handle *rth, int type, void *req, int len); + __u32 filt_mask) + __attribute__((warn_unused_result)); +extern int rtnl_dump_request(struct rtnl_handle *rth, int type, void *req, + int len) + __attribute__((warn_unused_result)); typedef int (*rtnl_filter_t)(const struct sockaddr_nl *, struct nlmsghdr *n, void *); @@ -44,9 +53,12 @@ extern int rtnl_dump_filter_l(struct rtnl_handle *rth, extern int rtnl_dump_filter(struct rtnl_handle *rth, rtnl_filter_t filter, void *arg); extern int rtnl_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n, pid_t peer, - unsigned groups, struct nlmsghdr *answer); -extern int rtnl_send(struct rtnl_handle *rth, const void *buf, int); -extern int rtnl_send_check(struct rtnl_handle *rth, const void *buf, int); + unsigned groups, struct nlmsghdr *answer) + __attribute__((warn_unused_result)); +extern int rtnl_send(struct rtnl_handle *rth, const void *buf, int) + __attribute__((warn_unused_result)); +extern int rtnl_send_check(struct rtnl_handle *rth, const void *buf, int) + __attribute__((warn_unused_result)); extern int addattr(struct nlmsghdr *n, int maxlen, int type); extern int addattr8(struct nlmsghdr *n, int maxlen, int type, __u8 data); diff --git a/ip/iplink.c b/ip/iplink.c index 58b6c20..e0c14e6 100644 --- a/ip/iplink.c +++ b/ip/iplink.c @@ -178,7 +178,10 @@ static int iplink_have_newlink(void) req.n.nlmsg_type = RTM_NEWLINK; req.i.ifi_family = AF_UNSPEC; - rtnl_send(&rth, &req.n, req.n.nlmsg_len); + if (rtnl_send(&rth, &req.n, req.n.nlmsg_len) < 0) { + perror("request send failed"); + exit(1); + } rtnl_listen(&rth, accept_msg, NULL); } return have_rtnl_newlink; diff --git a/ip/ipnetconf.c b/ip/ipnetconf.c index 9a77ecb..37aaf45 100644 --- a/ip/ipnetconf.c +++ b/ip/ipnetconf.c @@ -161,7 +161,10 @@ static int do_show(int argc, char **argv) addattr_l(&req.n, sizeof(req), NETCONFA_IFINDEX, &filter.ifindex, sizeof(filter.ifindex)); - rtnl_send(&rth, &req.n, req.n.nlmsg_len); + if (rtnl_send(&rth, &req.n, req.n.nlmsg_len) < 0) { + perror("Can not send request"); + exit(1); + } rtnl_listen(&rth, print_netconf, stdout); } else { dump: diff --git a/misc/arpd.c b/misc/arpd.c index ec9d570..d293b70 100644 --- a/misc/arpd.c +++ b/misc/arpd.c @@ -428,7 +428,11 @@ static int do_one_request(struct nlmsghdr *n) static void load_initial_table(void) { - rtnl_wilddump_request(&rth, AF_INET, RTM_GETNEIGH); + if (rtnl_wilddump_request(&rth, AF_INET, RTM_GETNEIGH) < 0) { + perrror("dump request failed"); + exit(1); + } + } static void get_kern_msg(void) diff --git a/misc/ss.c b/misc/ss.c index 6f38ae7..e59ca5c 100644 --- a/misc/ss.c +++ b/misc/ss.c @@ -996,7 +996,9 @@ static int xll_initted = 0; static void xll_init(void) { struct rtnl_handle rth; - rtnl_open(&rth, 0); + if (rtnl_open(&rth, 0) < 0) + exit(1); + ll_init_map(&rth); rtnl_close(&rth); xll_initted = 1;
I took your idea and enhanced it to all of iproute2 by doing the following: From c4b6330a3a033bd9c9b0664c5f844493137ae599 Mon Sep 17 00:00:00 2001 From: Stephen Hemminger <stephen@networkplumber.org> Date: Fri, 20 Dec 2013 08:15:02 -0800 Subject: [PATCH] check return value of rtnl_send and related functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use warn_unused_result to enforce checking return value of rtnl_send, and fix where the errors are. Suggested by initial patch from Petr Písař <ppisar@redhat.com> --- include/libnetlink.h | 28 ++++++++++++++++++++-------- ip/iplink.c | 5 ++++- ip/ipnetconf.c | 5 ++++- misc/arpd.c | 6 +++++- misc/ss.c | 4 +++- 5 files changed, 36 insertions(+), 12 deletions(-)