On Tue, Nov 04, 2025 at 03:50:40PM +0100, Theo Buehler wrote:
> On Tue, Nov 04, 2025 at 03:40:22PM +0100, Claudio Jeker wrote:
> > Consistently check if aid2afi() failed
> > 
> > Check return value against == -1 also use the same error message in
> > most places.
> > 
> > Add check in mrt_dump_entry_v2(), the call in mrt_dump_entry() is skipped
> > since the aid is limited to AID_INET and AID_INET6 and so that function is
> > not supposed to fail.
> 
> Is it really limited or is there a logic error at the top?

I think this is a logic error. The following condition needs to be held:
p->pt->aid == peer->remote_addr.aid and
p->pt->aid is either AID_INET or AID_INET6 only then the code should be
called.
 
If I applied De Morgan correctly your diff is correct.

> I think coverity will still whine about the missing return check, but a
> cast tells it that we deliberately ignore the return value.

Right now coberity does not complain. I'm not sure if we should sprinkle
(void) on functions calls. We currently only have one of those.

> ok

First bit is OK claudio@
 
> Index: mrt.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/mrt.c,v
> diff -u -p -r1.127 mrt.c
> --- mrt.c	21 Aug 2025 15:15:25 -0000	1.127
> +++ mrt.c	4 Nov 2025 14:46:12 -0000
> @@ -527,8 +527,8 @@ mrt_dump_entry(struct mrt *mrt, struct p
>  	uint16_t	 subtype;
>  	uint8_t		 dummy;
>  
> -	if (p->pt->aid != peer->remote_addr.aid &&
> -	    p->pt->aid != AID_INET && p->pt->aid != AID_INET6)
> +	if (p->pt->aid != peer->remote_addr.aid ||
> +	    (p->pt->aid != AID_INET && p->pt->aid != AID_INET6))
>  		/* only able to dump pure IPv4/IPv6 */
>  		return (0);
>  
> @@ -549,7 +549,7 @@ mrt_dump_entry(struct mrt *mrt, struct p
>  		goto fail;
>  
>  	len = ibuf_size(buf);
> -	aid2afi(p->pt->aid, &subtype, &dummy);
> +	(void)aid2afi(p->pt->aid, &subtype, &dummy);
>  	if (mrt_dump_hdr_rde(&hbuf, MSG_TABLE_DUMP, subtype, len) == -1)
>  		goto fail;
>  
> 

-- 
:wq Claudio