[ovs-dev] [PATCH 1/2] userspace: Replace all uses of strncpy() by ovs_strlcpy().

Ben Pfaff blp at nicira.com
Fri Feb 20 20:19:50 UTC 2015


On Fri, Feb 20, 2015 at 03:18:38PM -0500, Russell Bryant wrote:
> On 02/20/2015 02:14 PM, Ben Pfaff wrote:
> > On Fri, Feb 20, 2015 at 01:55:49PM -0500, Russell Bryant wrote:
> >> On 02/20/2015 01:42 PM, Ben Pfaff wrote:
> >>> strncpy() has a lot of pitfalls.  A while back we replaced all its uses by
> >>> calls to ovs_strlcpy() or ovs_strzcpy(), but some more have crept in.  This
> >>> commit fixes them.
> >>>
> >>
> >> Great, I like this a lot more than what I did.
> >>
> >>
> >>> @@ -110,7 +110,7 @@ ovs_router_insert__(uint8_t priority, ovs_be32 ip_dst, uint8_t plen,
> >>>      rt_init_match(&match, ip_dst, plen);
> >>>  
> >>>      p = xzalloc(sizeof *p);
> >>> -    strncpy(p->output_bridge, output_bridge, IFNAMSIZ);
> >>> +    ovs_strlcpy(p->output_bridge, output_bridge, IFNAMSIZ);
> >>>      p->gw = gw;
> >>>      p->nw_addr = match.flow.nw_dst;
> >>>      p->plen = plen;
> >>
> >> Just a nit ... I generally prefer "sizeof p->output_bridge", even though
> >> IFNAMSIZ is the same thing.  It seems like a less error prone pattern to
> >> default to since it's still correct if the destination size were to be
> >> changed.  I suppose you could argue it could result in accidental cases
> >> of getting the size of a pointer instead of the type, but at least
> >> coverity catches that.  Anyway, I changed several in passing in my
> >> changes.  I'm still +1 on your patch without it, though.
> > 
> > I thought about this issue too but I was concentrating on not changing
> > more than necessary.  I'll post a v2.
> 
> Or I can follow-up later with those changes if you're OK with them.
> Whichever.  Thanks!

It only took a minute so it's already out:
        http://openvswitch.org/pipermail/dev/2015-February/051475.html



More information about the dev mailing list