Date: Wed, 2 Oct 2019 08:41:30 -0700 (PDT) From: "Rodney W. Grimes" <freebsd@gndrsh.dnsmgr.net> To: cem@freebsd.org Cc: Alexander Kabaev <kan@freebsd.org>, src-committers <src-committers@freebsd.org>, svn-src-all <svn-src-all@freebsd.org>, svn-src-head <svn-src-head@freebsd.org> Subject: Re: svn commit: r352953 - in head/usr.bin: killall split Message-ID: <201910021541.x92FfUwY049029@gndrsh.dnsmgr.net> In-Reply-To: <CAG6CVpX4UF_WrLjkLBRJmKsza1Rfp5od%2BDJhHssVcNCzx=Tcow@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
> 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 <kan@freebsd.org> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201910021541.x92FfUwY049029>