From owner-svn-src-head@freebsd.org Wed Oct 2 15:41:34 2019 Return-Path: Delivered-To: svn-src-head@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 0404A131F68; Wed, 2 Oct 2019 15:41:34 +0000 (UTC) (envelope-from freebsd@gndrsh.dnsmgr.net) Received: from gndrsh.dnsmgr.net (br1.CN84in.dnsmgr.net [69.59.192.140]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 46k0jn5tqZz4ZgP; Wed, 2 Oct 2019 15:41:33 +0000 (UTC) (envelope-from freebsd@gndrsh.dnsmgr.net) Received: from gndrsh.dnsmgr.net (localhost [127.0.0.1]) by gndrsh.dnsmgr.net (8.13.3/8.13.3) with ESMTP id x92FfUnQ049030; Wed, 2 Oct 2019 08:41:30 -0700 (PDT) (envelope-from freebsd@gndrsh.dnsmgr.net) Received: (from freebsd@localhost) by gndrsh.dnsmgr.net (8.13.3/8.13.3/Submit) id x92FfUwY049029; Wed, 2 Oct 2019 08:41:30 -0700 (PDT) (envelope-from freebsd) From: "Rodney W. Grimes" Message-Id: <201910021541.x92FfUwY049029@gndrsh.dnsmgr.net> Subject: Re: svn commit: r352953 - in head/usr.bin: killall split In-Reply-To: To: cem@freebsd.org Date: Wed, 2 Oct 2019 08:41:30 -0700 (PDT) CC: Alexander Kabaev , src-committers , svn-src-all , svn-src-head Reply-To: rgrimes@freebsd.org X-Mailer: ELM [version 2.4ME+ PL121h (25)] MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset=US-ASCII X-Rspamd-Queue-Id: 46k0jn5tqZz4ZgP X-Spamd-Bar: ----- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-5.99 / 15.00]; NEURAL_HAM_MEDIUM(-0.99)[-0.994,0]; REPLY(-4.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000,0] X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 02 Oct 2019 15:41:34 -0000 > Hi Alexander, > > Coverity has millions of spurious warnings of this class and they're > basically all false positives. I think we should instead try to > figure out how to disable this class of warning on our codebase, since > it is largely incorrect. > > I would encourage reverting this change, because it uglies up the code > for no functional benefit. The code was correct before the change; > only Coverity was wrong. Again, I would like to suggest that a "working group", or team be formed that would be responsible for reviewing all Coverty/static analysis type fixes before they are committed to the tree? Simply a group of people that are willing to look at a code review in phabricator before a change is committed, to prevent exactly this type of issue. The team should consist of experts in static analysis or language contruction so that we catch these types of false positive fixes before they are committed to the tree. I also fully support Conrad in his assertion that we should start to find ways to silence Coverity on these false positives. Regards, Rod > > Best, > Conrad > > On Tue, Oct 1, 2019 at 11:15 PM Alexander Kabaev wrote: > > > > Author: kan > > Date: Wed Oct 2 06:15:30 2019 > > New Revision: 352953 > > URL: https://svnweb.freebsd.org/changeset/base/352953 > > > > Log: > > Convert pnmatch to single element array in regexec calls > > > > The regexec function is declared as taking an array of regmatch_t > > elements, and passing in the pointer to singleton element, while > > correct, triggers a Coverity warning. Convert the singleton into > > an array of one to silence the warning. > > > > Reported by: Coverity > > Coverity CID: 1009732, 1009733 > > MFC after: 2 weeks > > > > Modified: > > head/usr.bin/killall/killall.c > > head/usr.bin/split/split.c > > > > Modified: head/usr.bin/killall/killall.c > > ============================================================================== > > --- head/usr.bin/killall/killall.c Wed Oct 2 02:37:34 2019 (r352952) > > +++ head/usr.bin/killall/killall.c Wed Oct 2 06:15:30 2019 (r352953) > > @@ -98,7 +98,7 @@ main(int ac, char **av) > > struct stat sb; > > struct passwd *pw; > > regex_t rgx; > > - regmatch_t pmatch; > > + regmatch_t pmatch[1]; > > int i, j, ch; > > char buf[256]; > > char first; > > @@ -361,9 +361,9 @@ main(int ac, char **av) > > } > > } > > if (mflag) { > > - pmatch.rm_so = 0; > > - pmatch.rm_eo = strlen(thiscmd); > > - if (regexec(&rgx, thiscmd, 0, &pmatch, > > + pmatch[0].rm_so = 0; > > + pmatch[0].rm_eo = strlen(thiscmd); > > + if (regexec(&rgx, thiscmd, 0, pmatch, > > REG_STARTEND) != 0) > > matched = 0; > > regfree(&rgx); > > @@ -387,9 +387,9 @@ main(int ac, char **av) > > } > > } > > if (mflag) { > > - pmatch.rm_so = 0; > > - pmatch.rm_eo = strlen(thiscmd); > > - if (regexec(&rgx, thiscmd, 0, &pmatch, > > + pmatch[0].rm_so = 0; > > + pmatch[0].rm_eo = strlen(thiscmd); > > + if (regexec(&rgx, thiscmd, 0, pmatch, > > REG_STARTEND) == 0) > > matched = 1; > > regfree(&rgx); > > > > Modified: head/usr.bin/split/split.c > > ============================================================================== > > --- head/usr.bin/split/split.c Wed Oct 2 02:37:34 2019 (r352952) > > +++ head/usr.bin/split/split.c Wed Oct 2 06:15:30 2019 (r352953) > > @@ -281,11 +281,11 @@ split2(void) > > > > /* Check if we need to start a new file */ > > if (pflag) { > > - regmatch_t pmatch; > > + regmatch_t pmatch[1]; > > > > - pmatch.rm_so = 0; > > - pmatch.rm_eo = len - 1; > > - if (regexec(&rgx, bfr, 0, &pmatch, REG_STARTEND) == 0) > > + pmatch[0].rm_so = 0; > > + pmatch[0].rm_eo = len - 1; > > + if (regexec(&rgx, bfr, 0, pmatch, REG_STARTEND) == 0) > > newfile(); > > } else if (lcnt++ == numlines) { > > newfile(); > -- Rod Grimes rgrimes@freebsd.org