| Message ID | 20191014181047.9565-1-blp@ovn.org |
|---|---|
| State | Accepted |
| Commit | 622ea8fd759587a08e7d36075675912cf379aab5 |
| Headers | show |
| Series | [ovs-dev] dpif-netlink: Fix some variable naming. | expand |
LGTM, thanks. Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com> On Mon, Oct 14, 2019 at 11:28 AM Ben Pfaff <blp@ovn.org> wrote: > > Usually a plural name refers to an array, but 'socks' and 'socksp' were > only single objects, so this changes their names to 'sock' and 'sockp'. > > Usually a 'p' suffix means that a variable is an output argument, but > that was only true in one place here, so this changes the names of the > other variables to plain 'sock'. > > Signed-off-by: Ben Pfaff <blp@ovn.org> > --- > lib/dpif-netlink.c | 48 +++++++++++++++++++++++----------------------- > 1 file changed, 24 insertions(+), 24 deletions(-) > > diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c > index ebe22106e0fc..d1f9b81db84f 100644 > --- a/lib/dpif-netlink.c > +++ b/lib/dpif-netlink.c > @@ -249,11 +249,11 @@ static int dpif_netlink_port_query__(const struct dpif_netlink *dpif, > struct dpif_port *dpif_port); > > static int > -create_nl_sock(struct dpif_netlink *dpif OVS_UNUSED, struct nl_sock **socksp) > +create_nl_sock(struct dpif_netlink *dpif OVS_UNUSED, struct nl_sock **sockp) > OVS_REQ_WRLOCK(dpif->upcall_lock) > { > #ifndef _WIN32 > - return nl_sock_create(NETLINK_GENERIC, socksp); > + return nl_sock_create(NETLINK_GENERIC, sockp); > #else > /* Pick netlink sockets to use in a round-robin fashion from each > * handler's pool of sockets. */ > @@ -263,13 +263,13 @@ create_nl_sock(struct dpif_netlink *dpif OVS_UNUSED, struct nl_sock **socksp) > > /* A pool of sockets is allocated when the handler is initialized. */ > if (sock_pool == NULL) { > - *socksp = NULL; > + *sockp = NULL; > return EINVAL; > } > > ovs_assert(index < VPORT_SOCK_POOL_SIZE); > - *socksp = sock_pool[index].nl_sock; > - ovs_assert(*socksp); > + *sockp = sock_pool[index].nl_sock; > + ovs_assert(*sockp); > index = (index == VPORT_SOCK_POOL_SIZE - 1) ? 0 : index + 1; > handler->last_used_pool_idx = index; > return 0; > @@ -277,10 +277,10 @@ create_nl_sock(struct dpif_netlink *dpif OVS_UNUSED, struct nl_sock **socksp) > } > > static void > -close_nl_sock(struct nl_sock *socksp) > +close_nl_sock(struct nl_sock *sock) > { > #ifndef _WIN32 > - nl_sock_destroy(socksp); > + nl_sock_destroy(sock); > #endif > } > > @@ -450,7 +450,7 @@ vport_get_pid(struct dpif_netlink *dpif, uint32_t port_idx, > > static int > vport_add_channel(struct dpif_netlink *dpif, odp_port_t port_no, > - struct nl_sock *socksp) > + struct nl_sock *sock) > { > struct epoll_event event; > uint32_t port_idx = odp_to_u32(port_no); > @@ -458,7 +458,7 @@ vport_add_channel(struct dpif_netlink *dpif, odp_port_t port_no, > int error; > > if (dpif->handlers == NULL) { > - close_nl_sock(socksp); > + close_nl_sock(sock); > return 0; > } > > @@ -499,14 +499,14 @@ vport_add_channel(struct dpif_netlink *dpif, odp_port_t port_no, > struct dpif_handler *handler = &dpif->handlers[i]; > > #ifndef _WIN32 > - if (epoll_ctl(handler->epoll_fd, EPOLL_CTL_ADD, nl_sock_fd(socksp), > + if (epoll_ctl(handler->epoll_fd, EPOLL_CTL_ADD, nl_sock_fd(sock), > &event) < 0) { > error = errno; > goto error; > } > #endif > } > - dpif->channels[port_idx].sock = socksp; > + dpif->channels[port_idx].sock = sock; > dpif->channels[port_idx].last_poll = LLONG_MIN; > > return 0; > @@ -515,7 +515,7 @@ error: > #ifndef _WIN32 > while (i--) { > epoll_ctl(dpif->handlers[i].epoll_fd, EPOLL_CTL_DEL, > - nl_sock_fd(socksp), NULL); > + nl_sock_fd(sock), NULL); > } > #endif > dpif->channels[port_idx].sock = NULL; > @@ -750,12 +750,12 @@ dpif_netlink_port_add__(struct dpif_netlink *dpif, const char *name, > { > struct dpif_netlink_vport request, reply; > struct ofpbuf *buf; > - struct nl_sock *socksp = NULL; > + struct nl_sock *sock = NULL; > uint32_t upcall_pids = 0; > int error = 0; > > if (dpif->handlers) { > - error = create_nl_sock(dpif, &socksp); > + error = create_nl_sock(dpif, &sock); > if (error) { > return error; > } > @@ -768,8 +768,8 @@ dpif_netlink_port_add__(struct dpif_netlink *dpif, const char *name, > request.name = name; > > request.port_no = *port_nop; > - if (socksp) { > - upcall_pids = nl_sock_pid(socksp); > + if (sock) { > + upcall_pids = nl_sock_pid(sock); > } > request.n_upcall_pids = 1; > request.upcall_pids = &upcall_pids; > @@ -788,11 +788,11 @@ dpif_netlink_port_add__(struct dpif_netlink *dpif, const char *name, > dpif_name(&dpif->dpif), *port_nop); > } > > - close_nl_sock(socksp); > + close_nl_sock(sock); > goto exit; > } > > - error = vport_add_channel(dpif, *port_nop, socksp); > + error = vport_add_channel(dpif, *port_nop, sock); > if (error) { > VLOG_INFO("%s: could not add channel for port %s", > dpif_name(&dpif->dpif), name); > @@ -803,7 +803,7 @@ dpif_netlink_port_add__(struct dpif_netlink *dpif, const char *name, > request.dp_ifindex = dpif->dp_ifindex; > request.port_no = *port_nop; > dpif_netlink_vport_transact(&request, NULL, NULL); > - close_nl_sock(socksp); > + close_nl_sock(sock); > goto exit; > } > > @@ -2316,22 +2316,22 @@ dpif_netlink_refresh_channels(struct dpif_netlink *dpif, uint32_t n_handlers) > > if (port_no >= dpif->uc_array_size > || !vport_get_pid(dpif, port_no, &upcall_pid)) { > - struct nl_sock *socksp; > - error = create_nl_sock(dpif, &socksp); > + struct nl_sock *sock; > + error = create_nl_sock(dpif, &sock); > > if (error) { > goto error; > } > > - error = vport_add_channel(dpif, vport.port_no, socksp); > + error = vport_add_channel(dpif, vport.port_no, sock); > if (error) { > VLOG_INFO("%s: could not add channels for port %s", > dpif_name(&dpif->dpif), vport.name); > - nl_sock_destroy(socksp); > + nl_sock_destroy(sock); > retval = error; > goto error; > } > - upcall_pid = nl_sock_pid(socksp); > + upcall_pid = nl_sock_pid(sock); > } > > /* Configure the vport to deliver misses to 'sock'. */ > -- > 2.21.0 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Thank you for the review. I applied this to master. On Mon, Oct 14, 2019 at 12:16:59PM -0700, Yifeng Sun wrote: > LGTM, thanks. > > Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com> > > On Mon, Oct 14, 2019 at 11:28 AM Ben Pfaff <blp@ovn.org> wrote: > > > > Usually a plural name refers to an array, but 'socks' and 'socksp' were > > only single objects, so this changes their names to 'sock' and 'sockp'. > > > > Usually a 'p' suffix means that a variable is an output argument, but > > that was only true in one place here, so this changes the names of the > > other variables to plain 'sock'. > > > > Signed-off-by: Ben Pfaff <blp@ovn.org> > > --- > > lib/dpif-netlink.c | 48 +++++++++++++++++++++++----------------------- > > 1 file changed, 24 insertions(+), 24 deletions(-) > > > > diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c > > index ebe22106e0fc..d1f9b81db84f 100644 > > --- a/lib/dpif-netlink.c > > +++ b/lib/dpif-netlink.c > > @@ -249,11 +249,11 @@ static int dpif_netlink_port_query__(const struct dpif_netlink *dpif, > > struct dpif_port *dpif_port); > > > > static int > > -create_nl_sock(struct dpif_netlink *dpif OVS_UNUSED, struct nl_sock **socksp) > > +create_nl_sock(struct dpif_netlink *dpif OVS_UNUSED, struct nl_sock **sockp) > > OVS_REQ_WRLOCK(dpif->upcall_lock) > > { > > #ifndef _WIN32 > > - return nl_sock_create(NETLINK_GENERIC, socksp); > > + return nl_sock_create(NETLINK_GENERIC, sockp); > > #else > > /* Pick netlink sockets to use in a round-robin fashion from each > > * handler's pool of sockets. */ > > @@ -263,13 +263,13 @@ create_nl_sock(struct dpif_netlink *dpif OVS_UNUSED, struct nl_sock **socksp) > > > > /* A pool of sockets is allocated when the handler is initialized. */ > > if (sock_pool == NULL) { > > - *socksp = NULL; > > + *sockp = NULL; > > return EINVAL; > > } > > > > ovs_assert(index < VPORT_SOCK_POOL_SIZE); > > - *socksp = sock_pool[index].nl_sock; > > - ovs_assert(*socksp); > > + *sockp = sock_pool[index].nl_sock; > > + ovs_assert(*sockp); > > index = (index == VPORT_SOCK_POOL_SIZE - 1) ? 0 : index + 1; > > handler->last_used_pool_idx = index; > > return 0; > > @@ -277,10 +277,10 @@ create_nl_sock(struct dpif_netlink *dpif OVS_UNUSED, struct nl_sock **socksp) > > } > > > > static void > > -close_nl_sock(struct nl_sock *socksp) > > +close_nl_sock(struct nl_sock *sock) > > { > > #ifndef _WIN32 > > - nl_sock_destroy(socksp); > > + nl_sock_destroy(sock); > > #endif > > } > > > > @@ -450,7 +450,7 @@ vport_get_pid(struct dpif_netlink *dpif, uint32_t port_idx, > > > > static int > > vport_add_channel(struct dpif_netlink *dpif, odp_port_t port_no, > > - struct nl_sock *socksp) > > + struct nl_sock *sock) > > { > > struct epoll_event event; > > uint32_t port_idx = odp_to_u32(port_no); > > @@ -458,7 +458,7 @@ vport_add_channel(struct dpif_netlink *dpif, odp_port_t port_no, > > int error; > > > > if (dpif->handlers == NULL) { > > - close_nl_sock(socksp); > > + close_nl_sock(sock); > > return 0; > > } > > > > @@ -499,14 +499,14 @@ vport_add_channel(struct dpif_netlink *dpif, odp_port_t port_no, > > struct dpif_handler *handler = &dpif->handlers[i]; > > > > #ifndef _WIN32 > > - if (epoll_ctl(handler->epoll_fd, EPOLL_CTL_ADD, nl_sock_fd(socksp), > > + if (epoll_ctl(handler->epoll_fd, EPOLL_CTL_ADD, nl_sock_fd(sock), > > &event) < 0) { > > error = errno; > > goto error; > > } > > #endif > > } > > - dpif->channels[port_idx].sock = socksp; > > + dpif->channels[port_idx].sock = sock; > > dpif->channels[port_idx].last_poll = LLONG_MIN; > > > > return 0; > > @@ -515,7 +515,7 @@ error: > > #ifndef _WIN32 > > while (i--) { > > epoll_ctl(dpif->handlers[i].epoll_fd, EPOLL_CTL_DEL, > > - nl_sock_fd(socksp), NULL); > > + nl_sock_fd(sock), NULL); > > } > > #endif > > dpif->channels[port_idx].sock = NULL; > > @@ -750,12 +750,12 @@ dpif_netlink_port_add__(struct dpif_netlink *dpif, const char *name, > > { > > struct dpif_netlink_vport request, reply; > > struct ofpbuf *buf; > > - struct nl_sock *socksp = NULL; > > + struct nl_sock *sock = NULL; > > uint32_t upcall_pids = 0; > > int error = 0; > > > > if (dpif->handlers) { > > - error = create_nl_sock(dpif, &socksp); > > + error = create_nl_sock(dpif, &sock); > > if (error) { > > return error; > > } > > @@ -768,8 +768,8 @@ dpif_netlink_port_add__(struct dpif_netlink *dpif, const char *name, > > request.name = name; > > > > request.port_no = *port_nop; > > - if (socksp) { > > - upcall_pids = nl_sock_pid(socksp); > > + if (sock) { > > + upcall_pids = nl_sock_pid(sock); > > } > > request.n_upcall_pids = 1; > > request.upcall_pids = &upcall_pids; > > @@ -788,11 +788,11 @@ dpif_netlink_port_add__(struct dpif_netlink *dpif, const char *name, > > dpif_name(&dpif->dpif), *port_nop); > > } > > > > - close_nl_sock(socksp); > > + close_nl_sock(sock); > > goto exit; > > } > > > > - error = vport_add_channel(dpif, *port_nop, socksp); > > + error = vport_add_channel(dpif, *port_nop, sock); > > if (error) { > > VLOG_INFO("%s: could not add channel for port %s", > > dpif_name(&dpif->dpif), name); > > @@ -803,7 +803,7 @@ dpif_netlink_port_add__(struct dpif_netlink *dpif, const char *name, > > request.dp_ifindex = dpif->dp_ifindex; > > request.port_no = *port_nop; > > dpif_netlink_vport_transact(&request, NULL, NULL); > > - close_nl_sock(socksp); > > + close_nl_sock(sock); > > goto exit; > > } > > > > @@ -2316,22 +2316,22 @@ dpif_netlink_refresh_channels(struct dpif_netlink *dpif, uint32_t n_handlers) > > > > if (port_no >= dpif->uc_array_size > > || !vport_get_pid(dpif, port_no, &upcall_pid)) { > > - struct nl_sock *socksp; > > - error = create_nl_sock(dpif, &socksp); > > + struct nl_sock *sock; > > + error = create_nl_sock(dpif, &sock); > > > > if (error) { > > goto error; > > } > > > > - error = vport_add_channel(dpif, vport.port_no, socksp); > > + error = vport_add_channel(dpif, vport.port_no, sock); > > if (error) { > > VLOG_INFO("%s: could not add channels for port %s", > > dpif_name(&dpif->dpif), vport.name); > > - nl_sock_destroy(socksp); > > + nl_sock_destroy(sock); > > retval = error; > > goto error; > > } > > - upcall_pid = nl_sock_pid(socksp); > > + upcall_pid = nl_sock_pid(sock); > > } > > > > /* Configure the vport to deliver misses to 'sock'. */ > > -- > > 2.21.0 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c index ebe22106e0fc..d1f9b81db84f 100644 --- a/lib/dpif-netlink.c +++ b/lib/dpif-netlink.c @@ -249,11 +249,11 @@ static int dpif_netlink_port_query__(const struct dpif_netlink *dpif, struct dpif_port *dpif_port); static int -create_nl_sock(struct dpif_netlink *dpif OVS_UNUSED, struct nl_sock **socksp) +create_nl_sock(struct dpif_netlink *dpif OVS_UNUSED, struct nl_sock **sockp) OVS_REQ_WRLOCK(dpif->upcall_lock) { #ifndef _WIN32 - return nl_sock_create(NETLINK_GENERIC, socksp); + return nl_sock_create(NETLINK_GENERIC, sockp); #else /* Pick netlink sockets to use in a round-robin fashion from each * handler's pool of sockets. */ @@ -263,13 +263,13 @@ create_nl_sock(struct dpif_netlink *dpif OVS_UNUSED, struct nl_sock **socksp) /* A pool of sockets is allocated when the handler is initialized. */ if (sock_pool == NULL) { - *socksp = NULL; + *sockp = NULL; return EINVAL; } ovs_assert(index < VPORT_SOCK_POOL_SIZE); - *socksp = sock_pool[index].nl_sock; - ovs_assert(*socksp); + *sockp = sock_pool[index].nl_sock; + ovs_assert(*sockp); index = (index == VPORT_SOCK_POOL_SIZE - 1) ? 0 : index + 1; handler->last_used_pool_idx = index; return 0; @@ -277,10 +277,10 @@ create_nl_sock(struct dpif_netlink *dpif OVS_UNUSED, struct nl_sock **socksp) } static void -close_nl_sock(struct nl_sock *socksp) +close_nl_sock(struct nl_sock *sock) { #ifndef _WIN32 - nl_sock_destroy(socksp); + nl_sock_destroy(sock); #endif } @@ -450,7 +450,7 @@ vport_get_pid(struct dpif_netlink *dpif, uint32_t port_idx, static int vport_add_channel(struct dpif_netlink *dpif, odp_port_t port_no, - struct nl_sock *socksp) + struct nl_sock *sock) { struct epoll_event event; uint32_t port_idx = odp_to_u32(port_no); @@ -458,7 +458,7 @@ vport_add_channel(struct dpif_netlink *dpif, odp_port_t port_no, int error; if (dpif->handlers == NULL) { - close_nl_sock(socksp); + close_nl_sock(sock); return 0; } @@ -499,14 +499,14 @@ vport_add_channel(struct dpif_netlink *dpif, odp_port_t port_no, struct dpif_handler *handler = &dpif->handlers[i]; #ifndef _WIN32 - if (epoll_ctl(handler->epoll_fd, EPOLL_CTL_ADD, nl_sock_fd(socksp), + if (epoll_ctl(handler->epoll_fd, EPOLL_CTL_ADD, nl_sock_fd(sock), &event) < 0) { error = errno; goto error; } #endif } - dpif->channels[port_idx].sock = socksp; + dpif->channels[port_idx].sock = sock; dpif->channels[port_idx].last_poll = LLONG_MIN; return 0; @@ -515,7 +515,7 @@ error: #ifndef _WIN32 while (i--) { epoll_ctl(dpif->handlers[i].epoll_fd, EPOLL_CTL_DEL, - nl_sock_fd(socksp), NULL); + nl_sock_fd(sock), NULL); } #endif dpif->channels[port_idx].sock = NULL; @@ -750,12 +750,12 @@ dpif_netlink_port_add__(struct dpif_netlink *dpif, const char *name, { struct dpif_netlink_vport request, reply; struct ofpbuf *buf; - struct nl_sock *socksp = NULL; + struct nl_sock *sock = NULL; uint32_t upcall_pids = 0; int error = 0; if (dpif->handlers) { - error = create_nl_sock(dpif, &socksp); + error = create_nl_sock(dpif, &sock); if (error) { return error; } @@ -768,8 +768,8 @@ dpif_netlink_port_add__(struct dpif_netlink *dpif, const char *name, request.name = name; request.port_no = *port_nop; - if (socksp) { - upcall_pids = nl_sock_pid(socksp); + if (sock) { + upcall_pids = nl_sock_pid(sock); } request.n_upcall_pids = 1; request.upcall_pids = &upcall_pids; @@ -788,11 +788,11 @@ dpif_netlink_port_add__(struct dpif_netlink *dpif, const char *name, dpif_name(&dpif->dpif), *port_nop); } - close_nl_sock(socksp); + close_nl_sock(sock); goto exit; } - error = vport_add_channel(dpif, *port_nop, socksp); + error = vport_add_channel(dpif, *port_nop, sock); if (error) { VLOG_INFO("%s: could not add channel for port %s", dpif_name(&dpif->dpif), name); @@ -803,7 +803,7 @@ dpif_netlink_port_add__(struct dpif_netlink *dpif, const char *name, request.dp_ifindex = dpif->dp_ifindex; request.port_no = *port_nop; dpif_netlink_vport_transact(&request, NULL, NULL); - close_nl_sock(socksp); + close_nl_sock(sock); goto exit; } @@ -2316,22 +2316,22 @@ dpif_netlink_refresh_channels(struct dpif_netlink *dpif, uint32_t n_handlers) if (port_no >= dpif->uc_array_size || !vport_get_pid(dpif, port_no, &upcall_pid)) { - struct nl_sock *socksp; - error = create_nl_sock(dpif, &socksp); + struct nl_sock *sock; + error = create_nl_sock(dpif, &sock); if (error) { goto error; } - error = vport_add_channel(dpif, vport.port_no, socksp); + error = vport_add_channel(dpif, vport.port_no, sock); if (error) { VLOG_INFO("%s: could not add channels for port %s", dpif_name(&dpif->dpif), vport.name); - nl_sock_destroy(socksp); + nl_sock_destroy(sock); retval = error; goto error; } - upcall_pid = nl_sock_pid(socksp); + upcall_pid = nl_sock_pid(sock); } /* Configure the vport to deliver misses to 'sock'. */
Usually a plural name refers to an array, but 'socks' and 'socksp' were only single objects, so this changes their names to 'sock' and 'sockp'. Usually a 'p' suffix means that a variable is an output argument, but that was only true in one place here, so this changes the names of the other variables to plain 'sock'. Signed-off-by: Ben Pfaff <blp@ovn.org> --- lib/dpif-netlink.c | 48 +++++++++++++++++++++++----------------------- 1 file changed, 24 insertions(+), 24 deletions(-)