Date: Sat, 26 Jan 2019 21:59:44 +0000 From: Colin Percival <cperciva@tarsnap.com> To: rgrimes@freebsd.org, Stefan Esser <se@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r343480 - head/lib/libfigpar Message-ID: <010001688c2cfbc4-d34e12a0-c113-4f96-8853-1543ea0b04cb-000000@email.amazonses.com> In-Reply-To: <201901262136.x0QLaAJv095518@pdx.rh.CN85.dnsmgr.net> References: <201901262136.x0QLaAJv095518@pdx.rh.CN85.dnsmgr.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On 1/26/19 1:36 PM, Rodney W. Grimes wrote: >> Author: se >> Date: Sat Jan 26 21:30:26 2019 >> New Revision: 343480 >> URL: https://svnweb.freebsd.org/changeset/base/343480 >> >> Log: >> Silence Clang Scan warning about potentially unsafe use of strcpy. >> >> While this is a false positive, the use of strdup() simplifies the code. > > Though that might be true, it also has to recalculate the > length of the string which was already known by slen. > > I am not sure how often this code is called, > but that is wasted cycles in a library. The length of the string was already being recalculated, by strcpy. It seems to me that this could be written as temp = malloc(slen + 1); if (temp == NULL) /* could not allocate memory */ return (-1); memcpy(temp, source, slen + 1); which avoids both recalculating the string length and using strcpy? >> Modified: head/lib/libfigpar/string_m.c >> ============================================================================== >> --- head/lib/libfigpar/string_m.c Sat Jan 26 20:43:28 2019 (r343479) >> +++ head/lib/libfigpar/string_m.c Sat Jan 26 21:30:26 2019 (r343480) >> @@ -119,10 +119,9 @@ replaceall(char *source, const char *find, const char >> >> /* If replace is longer than find, we'll need to create a temp copy */ >> if (rlen > flen) { >> - temp = malloc(slen + 1); >> - if (errno != 0) /* could not allocate memory */ >> + temp = strdup(source); >> + if (temp == NULL) /* could not allocate memory */ >> return (-1); >> - strcpy(temp, source); >> } else >> temp = source; -- Colin Percival Security Officer Emeritus, FreeBSD | The power to serve Founder, Tarsnap | www.tarsnap.com | Online backups for the truly paranoid
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?010001688c2cfbc4-d34e12a0-c113-4f96-8853-1543ea0b04cb-000000>