svn commit: r352113 - head/sys/net

[Ian Lepore]

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