[ovs-dev] [strings 3/3] util: New function ovs_strzcpy().

Ben Pfaff blp at nicira.com
Wed Feb 23 00:32:25 UTC 2011


I'll do that.

On Tue, Feb 22, 2011 at 04:25:14PM -0800, Justin Pettit wrote:
> Thanks for doing this.  The series looks good to me.  Did you want to
> include the Coverity bug numbers that this addresses
> (10697,10696,10695,10694,10693,10692,10691,10690)?
> 
> --Justin
> 
> 
> On Feb 22, 2011, at 11:01 AM, Ben Pfaff wrote:
> 
> > Static analyzers hate strncpy().  This new function shares its property of
> > initializing an entire buffer, without its nasty habit of failing to
> > null-terminate long strings.
> > ---
> > lib/automake.mk    |    1 +
> > lib/netdev-linux.c |   16 ++++++++--------
> > lib/socket-util.c  |    3 +--
> > lib/util.c         |   24 +++++++++++++++++++++++-
> > lib/util.h         |    3 ++-
> > 5 files changed, 35 insertions(+), 12 deletions(-)
> > 
> > diff --git a/lib/automake.mk b/lib/automake.mk
> > index 2758d36..5da1971 100644
> > --- a/lib/automake.mk
> > +++ b/lib/automake.mk
> > @@ -137,6 +137,7 @@ lib_libopenvswitch_a_SOURCES = \
> > 	lib/stream.h \
> > 	lib/stress.c \
> > 	lib/stress.h \
> > +	lib/string.c \
> > 	lib/string.h \
> > 	lib/svec.c \
> > 	lib/svec.h \
> > diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> > index 1428ce6..0aceb45 100644
> > --- a/lib/netdev-linux.c
> > +++ b/lib/netdev-linux.c
> > @@ -563,7 +563,7 @@ netdev_linux_create_tap(const struct netdev_class *class OVS_UNUSED,
> > 
> >     /* Create tap device. */
> >     ifr.ifr_flags = IFF_TAP | IFF_NO_PI;
> > -    strncpy(ifr.ifr_name, name, sizeof ifr.ifr_name);
> > +    ovs_strzcpy(ifr.ifr_name, name, sizeof ifr.ifr_name);
> >     if (ioctl(state->fd, TUNSETIFF, &ifr) == -1) {
> >         VLOG_WARN("%s: creating tap device failed: %s", name,
> >                   strerror(errno));
> > @@ -1924,7 +1924,7 @@ do_set_addr(struct netdev *netdev,
> >             int ioctl_nr, const char *ioctl_name, struct in_addr addr)
> > {
> >     struct ifreq ifr;
> > -    strncpy(ifr.ifr_name, netdev_get_name(netdev), sizeof ifr.ifr_name);
> > +    ovs_strzcpy(ifr.ifr_name, netdev_get_name(netdev), sizeof ifr.ifr_name);
> >     make_in4_sockaddr(&ifr.ifr_addr, addr);
> > 
> >     return netdev_linux_do_ioctl(netdev_get_name(netdev), &ifr, ioctl_nr,
> > @@ -2051,7 +2051,7 @@ netdev_linux_arp_lookup(const struct netdev *netdev,
> >     memcpy(&r.arp_pa, &sin, sizeof sin);
> >     r.arp_ha.sa_family = ARPHRD_ETHER;
> >     r.arp_flags = 0;
> > -    strncpy(r.arp_dev, netdev_get_name(netdev), sizeof r.arp_dev);
> > +    ovs_strzcpy(r.arp_dev, netdev_get_name(netdev), sizeof r.arp_dev);
> >     COVERAGE_INC(netdev_arp_lookup);
> >     retval = ioctl(af_inet_sock, SIOCGARP, &r) < 0 ? errno : 0;
> >     if (!retval) {
> > @@ -4054,7 +4054,7 @@ do_get_ifindex(const char *netdev_name)
> > {
> >     struct ifreq ifr;
> > 
> > -    strncpy(ifr.ifr_name, netdev_name, sizeof ifr.ifr_name);
> > +    ovs_strzcpy(ifr.ifr_name, netdev_name, sizeof ifr.ifr_name);
> >     COVERAGE_INC(netdev_get_ifindex);
> >     if (ioctl(af_inet_sock, SIOCGIFINDEX, &ifr) < 0) {
> >         VLOG_WARN_RL(&rl, "ioctl(SIOCGIFINDEX) on %s device failed: %s",
> > @@ -4089,7 +4089,7 @@ get_etheraddr(const char *netdev_name, uint8_t ea[ETH_ADDR_LEN])
> >     int hwaddr_family;
> > 
> >     memset(&ifr, 0, sizeof ifr);
> > -    strncpy(ifr.ifr_name, netdev_name, sizeof ifr.ifr_name);
> > +    ovs_strzcpy(ifr.ifr_name, netdev_name, sizeof ifr.ifr_name);
> >     COVERAGE_INC(netdev_get_hwaddr);
> >     if (ioctl(af_inet_sock, SIOCGIFHWADDR, &ifr) < 0) {
> >         VLOG_ERR("ioctl(SIOCGIFHWADDR) on %s device failed: %s",
> > @@ -4112,7 +4112,7 @@ set_etheraddr(const char *netdev_name, int hwaddr_family,
> >     struct ifreq ifr;
> > 
> >     memset(&ifr, 0, sizeof ifr);
> > -    strncpy(ifr.ifr_name, netdev_name, sizeof ifr.ifr_name);
> > +    ovs_strzcpy(ifr.ifr_name, netdev_name, sizeof ifr.ifr_name);
> >     ifr.ifr_hwaddr.sa_family = hwaddr_family;
> >     memcpy(ifr.ifr_hwaddr.sa_data, mac, ETH_ADDR_LEN);
> >     COVERAGE_INC(netdev_set_hwaddr);
> > @@ -4131,7 +4131,7 @@ netdev_linux_do_ethtool(const char *name, struct ethtool_cmd *ecmd,
> >     struct ifreq ifr;
> > 
> >     memset(&ifr, 0, sizeof ifr);
> > -    strncpy(ifr.ifr_name, name, sizeof ifr.ifr_name);
> > +    ovs_strzcpy(ifr.ifr_name, name, sizeof ifr.ifr_name);
> >     ifr.ifr_data = (caddr_t) ecmd;
> > 
> >     ecmd->cmd = cmd;
> > @@ -4154,7 +4154,7 @@ static int
> > netdev_linux_do_ioctl(const char *name, struct ifreq *ifr, int cmd,
> >                       const char *cmd_name)
> > {
> > -    strncpy(ifr->ifr_name, name, sizeof ifr->ifr_name);
> > +    ovs_strzcpy(ifr->ifr_name, name, sizeof ifr->ifr_name);
> >     if (ioctl(af_inet_sock, cmd, ifr) == -1) {
> >         VLOG_DBG_RL(&rl, "%s: ioctl(%s) failed: %s", name, cmd_name,
> >                      strerror(errno));
> > diff --git a/lib/socket-util.c b/lib/socket-util.c
> > index 275bf30..e0f34e7 100644
> > --- a/lib/socket-util.c
> > +++ b/lib/socket-util.c
> > @@ -233,8 +233,7 @@ static void
> > make_sockaddr_un__(const char *name, struct sockaddr_un *un, socklen_t *un_len)
> > {
> >     un->sun_family = AF_UNIX;
> > -    strncpy(un->sun_path, name, sizeof un->sun_path);
> > -    un->sun_path[sizeof un->sun_path - 1] = '\0';
> > +    ovs_strzcpy(un->sun_path, name, sizeof un->sun_path);
> >     *un_len = (offsetof(struct sockaddr_un, sun_path)
> >                 + strlen (un->sun_path) + 1);
> > }
> > diff --git a/lib/util.c b/lib/util.c
> > index 1aa8271..f784f03 100644
> > --- a/lib/util.c
> > +++ b/lib/util.c
> > @@ -1,5 +1,5 @@
> > /*
> > - * Copyright (c) 2008, 2009, 2010 Nicira Networks.
> > + * Copyright (c) 2008, 2009, 2010, 2011 Nicira Networks.
> >  *
> >  * Licensed under the Apache License, Version 2.0 (the "License");
> >  * you may not use this file except in compliance with the License.
> > @@ -149,6 +149,28 @@ ovs_strlcpy(char *dst, const char *src, size_t size)
> >     }
> > }
> > 
> > +/* Copies 'src' to 'dst'.  Reads no more than 'size - 1' bytes from 'src'.
> > + * Always null-terminates 'dst' (if 'size' is nonzero), and writes a zero byte
> > + * to every otherwise unused byte in 'dst'.
> > + *
> > + * Except for performance, the following call:
> > + *     ovs_strzcpy(dst, src, size);
> > + * is equivalent to these two calls:
> > + *     memset(dst, '\0', size);
> > + *     ovs_strlcpy(dst, src, size);
> > + *
> > + * (Thus, ovs_strzcpy() is similar to strncpy() without some of the pitfalls.)
> > + */
> > +void
> > +ovs_strzcpy(char *dst, const char *src, size_t size)
> > +{
> > +    if (size > 0) {
> > +        size_t len = strnlen(src, size - 1);
> > +        memcpy(dst, src, len);
> > +        memset(dst + len, '\0', size - len);
> > +    }
> > +}
> > +
> > void
> > ovs_fatal(int err_no, const char *format, ...)
> > {
> > diff --git a/lib/util.h b/lib/util.h
> > index f3bf80c..e741067 100644
> > --- a/lib/util.h
> > +++ b/lib/util.h
> > @@ -1,5 +1,5 @@
> > /*
> > - * Copyright (c) 2008, 2009, 2010 Nicira Networks.
> > + * Copyright (c) 2008, 2009, 2010, 2011 Nicira Networks.
> >  *
> >  * Licensed under the Apache License, Version 2.0 (the "License");
> >  * you may not use this file except in compliance with the License.
> > @@ -133,6 +133,7 @@ char *xvasprintf(const char *format, va_list) PRINTF_FORMAT(1, 0) MALLOC_LIKE;
> > void *x2nrealloc(void *p, size_t *n, size_t s);
> > 
> > void ovs_strlcpy(char *dst, const char *src, size_t size);
> > +void ovs_strzcpy(char *dst, const char *src, size_t size);
> > 
> > void ovs_fatal(int err_no, const char *format, ...)
> >     PRINTF_FORMAT(2, 3) NO_RETURN;
> > -- 
> > 1.7.1
> > 
> 




More information about the dev mailing list