Index | Thread | Search

From:
Theo Buehler <tb@theobuehler.org>
Subject:
Re: bgpd: cleanup log messages in rde_prefix.c
To:
tech@openbsd.org
Date:
Thu, 21 May 2026 11:12:44 +0200

Download raw body.

Thread
On Thu, May 21, 2026 at 10:41:17AM +0200, Claudio Jeker wrote:
> This is one of the todo items from the last few days.
> 
> Cleanup the log messages in rde_prefix.c
> 
> I tried to not put the function name in log messages if the errors remain
> unique and self explanatory.

The "unknown EVPN prefix type %d" is used once in a log_warnx() and once
in a fatalx, which probably doesn't count as a duplication.

ok

> Since this is clearing one TODO item, lets add another one:
> 	if (RB_REMOVE(pt_tree, &pttable, pte) == NULL)
> Can RB_REMOVE() ever return NULL? Is this and all the other RB_REMOVE
> checks unneeded?
> 
> The manpage has this:
>      The RB_REMOVE() macro removes the element elm from the tree pointed by
>      head.  RB_REMOVE() returns elm.

cc -E tells me that's correct and at least in our implementation it can't
return NULL: the thing you pass in (elm) is assigned to old and returned
(old is never changed) and if that is NULL there is a NULL access, so it
should crash (unless optimized out).

I don't think bgpd can call a checked RB_REMOVE with a NULL element.
That should fatal or crash before reaching the RB_REMOVE in all cases.