Date: Sat, 26 Jan 2019 23:26:23 +0100 From: Stefan Esser <se@freebsd.org> To: Colin Percival <cperciva@tarsnap.com>, rgrimes@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: <a949233a-6689-369b-3a7f-2176fdc11d73@freebsd.org> In-Reply-To: <010001688c2cfc3e-e319d851-8b9e-4468-8bd1-f93331f35116-000000@email.amazonses.com> References: <201901262136.x0QLaAJv095518@pdx.rh.CN85.dnsmgr.net> <010001688c2cfc3e-e319d851-8b9e-4468-8bd1-f93331f35116-000000@email.amazonses.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Am 26.01.19 um 22:59 schrieb Colin Percival: > 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? In principle you are right, there is a small run-time cost by not using the known length for the allocation. The Clang Scan checks are leading to lots (thousands) of false positives and a I have looked at quite a number and abstained from silencing them in the hope that the scan will be improved. It should learn that at least the trivial case of an allocation of the value of strlen()+1 followed by a strcpy (and no further strcat or the like) is safe. But I have seen cases where the assumption is that "i<0" followed by "i>=0" at the same location and then a warning about potential problem. I have committed the version you suggest in rev. 343482 ... Regards, STefan
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?a949233a-6689-369b-3a7f-2176fdc11d73>