On Thu, Jun 16, 2022 at 02:42:31PM +0200, Claudio Jeker wrote:
> tb@ noticed that name2id conversions never check for error.
> Now I think this is fine but in that case the api should not do a half
> assed job of setting errnos.
>
> In the end if 0 is returned from name2id but the input was not the empty
> string then an error occurred. None of the callers does this check and
> instead accept that the label is silently dropped.
>
> Most labels come from the config only rtlabel will be picked up from the
> kernel FIB. So I don't think that dropping such lables on the floor is a
> bad thing. Also the kernel has very similar limitations.
I agree with your description and the diff.
ok
>
> --
> :wq Claudio
>
> Index: name2id.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/name2id.c,v
> retrieving revision 1.11
> diff -u -p -r1.11 name2id.c
> --- name2id.c 6 Feb 2022 09:51:19 -0000 1.11
> +++ name2id.c 14 Jun 2022 17:03:47 -0000
> @@ -94,16 +94,18 @@ pftable_ref(uint16_t id)
> return (_ref(&pftable_labels, id));
> }
>
> +/*
> + * Try to convert a name into id. If something fails 0 is returned which
> + * is the ID of the empty label.
> + */
> uint16_t
> _name2id(struct n2id_labels *head, const char *name)
> {
> struct n2id_label *label, *p = NULL;
> uint16_t new_id = 1;
>
> - if (!name[0]) {
> - errno = EINVAL;
> + if (!name[0])
> return (0);
> - }
>
> TAILQ_FOREACH(label, head, entry)
> if (strcmp(name, label->name) == 0) {
> @@ -122,10 +124,8 @@ _name2id(struct n2id_labels *head, const
> p->id == new_id; p = TAILQ_NEXT(p, entry))
> new_id = p->id + 1;
>
> - if (new_id > IDVAL_MAX) {
> - errno = ERANGE;
> + if (new_id > IDVAL_MAX)
> return (0);
> - }
>
> if ((label = calloc(1, sizeof(struct n2id_label))) == NULL)
> return (0);
>
