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); >