On Wed, Feb 12, 2025 at 01:32:25PM +0100, Claudio Jeker wrote:
> As requested by tb@ move the getpeerbyid() call from control_imsg_relay()
> back to the session imsg dispatcher. So all of them are in the same place.
> 
> I left the p == NULL check just in case but I'm torn about that.

I think it makes sense to keep it, seeing as all other types accept
NULL. Better one check too many than one too few.

ok tb

It doesn't really matter since no caller checks the returned value, but I
think control_imsg_relay() should either return (-1) on error or the two
"return imsg_*" should translate -1 to 0 like it's done for imsg_get_data().

> -- 
> :wq Claudio
> 
> Index: control.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/control.c,v
> diff -u -p -r1.132 control.c
> --- control.c	2 Dec 2024 15:03:17 -0000	1.132
> +++ control.c	12 Feb 2025 12:28:15 -0000
> @@ -552,11 +552,9 @@ control_imsg_relay(struct imsg *imsg, st
>  		struct rde_peer_stats stats;
>  		struct peer peer;
>  
> -		if (p == NULL) {
> -			log_warnx("%s: no such peer: id=%u", __func__,
> -			    imsg_get_id(imsg));
> +		if (p == NULL)
>  			return (0);
> -		}
> +
>  		if (imsg_get_data(imsg, &stats, sizeof(stats)) == -1) {
>  			log_warnx("%s: imsg_get_data", __func__);
>  			return (0);
> Index: session.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/session.c,v
> diff -u -p -r1.513 session.c
> --- session.c	11 Feb 2025 19:28:45 -0000	1.513
> +++ session.c	11 Feb 2025 19:33:15 -0000
> @@ -3317,7 +3317,11 @@ session_dispatch_imsg(struct imsgbuf *im
>  		case IMSG_CTL_SHOW_NEIGHBOR:
>  			if (idx != PFD_PIPE_ROUTE_CTL)
>  				fatalx("ctl rib request not from RDE");
> -			p = getpeerbyid(conf, peerid);
> +			if ((p = getpeerbyid(conf, peerid)) == NULL) {
> +				log_warnx("%s: no such peer: id=%u",
> +				    "IMSG_CTL_SHOW_NEIGHBOR", peerid);
> +				break;
> +			}
>  			control_imsg_relay(&imsg, p);
>  			break;
>  		case IMSG_CTL_SHOW_RIB:
>