Date: Tue, 10 Sep 2019 08:03:29 -0600 From: Ian Lepore <ian@freebsd.org> To: Conrad Meyer <cem@FreeBSD.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r352113 - head/sys/net Message-ID: <06266e49ff64bba81b435c58985db7d88c74be03.camel@freebsd.org> In-Reply-To: <201909100156.x8A1ulJx087903@repo.freebsd.org> References: <201909100156.x8A1ulJx087903@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 2019-09-10 at 01:56 +0000, Conrad Meyer wrote: > Author: cem > Date: Tue Sep 10 01:56:47 2019 > New Revision: 352113 > URL: https://svnweb.freebsd.org/changeset/base/352113 > > Log: > Appease Clang false-positive Werrors in r352112 > > Reported by: bcran > > Modified: > head/sys/net/rtsock.c > > Modified: head/sys/net/rtsock.c > ===================================================================== > ========= > --- head/sys/net/rtsock.c Mon Sep 9 22:54:27 2019 (r352112) > +++ head/sys/net/rtsock.c Tue Sep 10 01:56:47 2019 (r352113) > @@ -2105,7 +2105,7 @@ rt_dumpentry_ddb(struct radix_node *rn, void > *arg __un > > if (flags != rt->rt_flags) > db_printf(","); > - db_printf(rt_flag_name(idx)); > + db_printf("%s", rt_flag_name(idx)); > > flags &= ~(1ul << idx); > } > @@ -2374,8 +2374,12 @@ _DB_FUNC(_show, route, db_show_route_cmd, > db_show_tabl > u.dest_sin6.sin6_addr.s6_addr16[i] = > htons(hextets[i]); > dstp = (void *)&u.dest_sin6; > dst_addrp = &u.dest_sin6.sin6_addr; > - } else > + } else { > MPASS(false); > + /* UNREACHABLE */ > + /* Appease Clang false positive: */ > + dstp = NULL; > + } > > bp = inet_ntop(af, dst_addrp, buf, sizeof(buf)); > if (bp != NULL) > I don't think this was a false positive. MPASS resolves to KASSERT which resolves to nothing when built without INVARIANTS defined. So that comment is misleading, the code isn't unreachable, and after falling through, dstp is going to be dereferenced a few lines later. Instead of just squelching the coverity error, I think it should lead to the question: Does it make any sense to assert in a ddb command handler? Would it make more sense to make that else block do something like db_printf("Unexpected address family %d\n", af); goto exit; ? -- Ian
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?06266e49ff64bba81b435c58985db7d88c74be03.camel>