On Sat, Sep 01, 2018 at 09:20:59PM +0800, Michael Mikonos wrote: > Hello, > > Replace a malloc+strlcpy with strndup in cmdline_symset(). > Parameter s is a "keyname=value" string and sym is the > "keyname" part. > > If s is "=value", sym will be an empty string. > The patch doesn't change this behaviour although > it might be undesirable to call symset() with > an empty string. Possibly it could also return -1 > if len is zero. Thoughts? >
Not opposed to the diff but at this late hour I find it easier to read the malloc+strlcpy and be sure there's not an off-by-one than with the strndup version, I'll read again tomorrow. Just wanted to remind you that this function is shared between daemons so this can't be an smtpd-only change :-) > Index: parse.y > =================================================================== > RCS file: /cvs/src/usr.sbin/smtpd/parse.y,v > retrieving revision 1.218 > diff -u -p -u -r1.218 parse.y > --- parse.y 25 Aug 2018 19:05:23 -0000 1.218 > +++ parse.y 1 Sep 2018 12:42:45 -0000 > @@ -2129,11 +2129,10 @@ cmdline_symset(char *s) > if ((val = strrchr(s, '=')) == NULL) > return (-1); > > - len = strlen(s) - strlen(val) + 1; > - if ((sym = malloc(len)) == NULL) > - errx(1, "cmdline_symset: malloc"); > - > - (void)strlcpy(sym, s, len); > + len = strlen(s) - strlen(val); > + sym = strndup(s, len); > + if (sym == NULL) > + errx(1, "%s: strndup", __func__); > > ret = symset(sym, val + 1, 1); > free(sym); > -- Gilles Chehade https://www.poolp.org @poolpOrg
