On Thu, Jul 13, 2023 at 10:04:33AM +0200, Claudio Jeker wrote:
> This is a follow-up to use more of the new ibuf API to write the mrt message.
>
> This removes all of the DUMP_XYZ macros and replaces them with
> ibuf_add_nX() calls. Also unify the error handling by using
> goto fail; in all cases and use a more generic log_warn() there once.
The conversions all look correct, so that's ok.
There are a few silent failures and a few double logs. The former should
be avoided and I think this should be done in this diff. I'm not sure
how much effort should be invested in fully avoiding the latter. It's a
bit messy:
The only caller mrt_dump_entry_v2_rib() already logs an ibuf failure on
error, so there's no need to add a log to mrt_dump_entry_v2_rib()'s fail
label.
In mrt_dump_hdr_se() there is no log after the fail: label. I think it
needs one, otherwise mrt_dump_{bgp_msg,state}() could fail silently.
Most callers of mrt_dump_hdr_rde() will log an ibuf error on failure, so
you probably want to remove the log in the early return after ibuf_dynamic.
Also, add a log (or goto fail) on mrt_dump_hdr_rde() failure in
mrt_dump_entry(). In particular, there is often a double log for the
'unsupported type' case in mrt_dump_hdr_rde()...
