On Wed, Nov 30, 2022 at 07:05:25PM +0100, Claudio Jeker wrote:
> Change ometric_set_int_with_label() to ometric_set_int_with_labels().
> In rpki-client I need to add more than one extra label and extending this
> interface seems to be the least ugly solution.
> I use C99 compound literals to make this less ugly and wrapped that magic
> into the macro EV(). IMO this is a compromise to keep the code short.
What does EV stand for?
> Am I abusing to much C99 magic here?
I don't think so. snmp and snmpd use similar constructs. I don't currently
have access to a gcc arch, so I can't check if old gcc is happy. If you
have such a machine running, it might be worth checking it is before commit.
ok
two small nits below
> --
> :wq Claudio
>
> Index: ometric.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpctl/ometric.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 ometric.c
> --- ometric.c 30 Nov 2022 10:15:01 -0000 1.3
> +++ ometric.c 30 Nov 2022 17:55:09 -0000
> @@ -209,15 +209,10 @@ olabels_free(struct olabels *ol)
> * value needs to be freed with olabels_free().
> */
> static struct olabels *
> -olabels_add_extra(struct olabels *ol, const char *key, const char *value)
> +olabels_add_extras(struct olabels *ol, const char **keys, const char
> **values)
> {
> - const char *keys[2] = { key, NULL };
> - const char *values[2] = { value, NULL };
> struct olabels *new;
>
> - if (value == NULL || *value == '\0')
> - return ol;
> -
> new = olabels_new(keys, values);
> new->next = olabels_ref(ol);
>
> @@ -382,10 +377,8 @@ ometric_set_info(struct ometric *om, con
> if (om->type != OMT_INFO)
> errx(1, "%s incorrect ometric type", __func__);
>
> - if (keys != NULL) {
> - extra = olabels_new(keys, values);
> - extra->next = olabels_ref(ol);
> - }
> + if (keys != NULL)
> + extra = olabels_add_extras(ol, keys, values);
>
> ometric_set_int_value(om, 1, extra != NULL ? extra : ol);
> olabels_free(extra);
> @@ -397,7 +390,6 @@ ometric_set_info(struct ometric *om, con
> void
> ometric_set_state(struct ometric *om, const char *state, struct olabels *ol)
> {
> - struct olabels *extra;
> size_t i;
> int val;
>
> @@ -410,9 +402,8 @@ ometric_set_state(struct ometric *om, co
> else
> val = 0;
>
> - extra = olabels_add_extra(ol, om->name, om->stateset[i]);
> - ometric_set_int_value(om, val, extra);
> - olabels_free(extra);
> + ometric_set_int_with_labels(om, val, EV(om->name),
> + EV(om->stateset[i]), ol);
> }
> }
>
> @@ -421,12 +412,12 @@ ometric_set_state(struct ometric *om, co
> * the value is copied into the extra label.
> */
> void
> -ometric_set_int_with_label(struct ometric *om, uint64_t val, const char *key,
> - const char *value, struct olabels *ol)
> +ometric_set_int_with_labels(struct ometric *om, uint64_t val,
> + const char **keys, const char **values, struct olabels *ol)
> {
> struct olabels *extra;
>
> - extra = olabels_add_extra(ol, key, value);
> + extra = olabels_add_extras(ol, keys, values);
> ometric_set_int(om, val, extra);
> olabels_free(extra);
> }
> Index: ometric.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpctl/ometric.h,v
> retrieving revision 1.2
> diff -u -p -r1.2 ometric.h
> --- ometric.h 30 Nov 2022 10:15:01 -0000 1.2
> +++ ometric.h 30 Nov 2022 17:47:47 -0000
> @@ -44,5 +44,6 @@ void ometric_set_float(struct ometric *,
> void ometric_set_info(struct ometric *, const char **, const char **,
> struct olabels *);
> void ometric_set_state(struct ometric *, const char *, struct olabels *);
> -void ometric_set_int_with_label(struct ometric *, uint64_t, const char *,
> - const char *, struct olabels *);
> +void ometric_set_int_with_labels(struct ometric *, uint64_t, const char **,
> + const char **, struct olabels *);
> +#define EV(...) (const char *[]){ __VA_ARGS__, NULL }
> Index: output_ometric.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpctl/output_ometric.c,v
> retrieving revision 1.4
> diff -u -p -r1.4 output_ometric.c
> --- output_ometric.c 30 Nov 2022 10:15:01 -0000 1.4
> +++ output_ometric.c 30 Nov 2022 17:51:28 -0000
> @@ -203,27 +203,31 @@ ometric_neighbor_stats(struct peer *p, s
> ometric_set_int(peer_prefixes_transmit, p->stats.prefix_out_cnt, ol);
> ometric_set_int(peer_prefixes_receive, p->stats.prefix_cnt, ol);
>
> - ometric_set_int_with_label(peer_message_transmit,
> - p->stats.msg_sent_open, "message", "open", ol);
> - ometric_set_int_with_label(peer_message_transmit,
> - p->stats.msg_sent_notification, "message", "notification", ol);
> - ometric_set_int_with_label(peer_message_transmit,
> - p->stats.msg_sent_update, "message", "update", ol);
> - ometric_set_int_with_label(peer_message_transmit,
> - p->stats.msg_sent_keepalive, "message", "keepalive", ol);
> - ometric_set_int_with_label(peer_message_transmit,
> - p->stats.msg_sent_rrefresh, "message", "route_refresh", ol);
> -
> - ometric_set_int_with_label(peer_message_recieve,
> - p->stats.msg_rcvd_open, "message", "open", ol);
> - ometric_set_int_with_label(peer_message_recieve,
> - p->stats.msg_rcvd_notification, "message", "notification", ol);
> - ometric_set_int_with_label(peer_message_recieve,
> - p->stats.msg_rcvd_update, "message", "update", ol);
> - ometric_set_int_with_label(peer_message_recieve,
> - p->stats.msg_rcvd_keepalive, "message", "keepalive", ol);
> - ometric_set_int_with_label(peer_message_recieve,
> - p->stats.msg_rcvd_rrefresh, "message", "route_refresh", ol);
> + ometric_set_int_with_labels(peer_message_transmit,
> + p->stats.msg_sent_open, EV("messages"), EV("open"), ol);
> + ometric_set_int_with_labels(peer_message_transmit,
> + p->stats.msg_sent_notification, EV("messages"), EV("notification"),
> + ol);
> + ometric_set_int_with_labels(peer_message_transmit,
> + p->stats.msg_sent_update, EV("messages"), EV("update"), ol);
> + ometric_set_int_with_labels(peer_message_transmit,
> + p->stats.msg_sent_keepalive, EV("messages"), EV("keepalive"), ol);
> + ometric_set_int_with_labels(peer_message_transmit,
> + p->stats.msg_sent_rrefresh, EV("messages"), EV("route_refresh"),
> + ol);
> +
> + ometric_set_int_with_labels(peer_message_recieve,
Could you fix the typo? receive, not recieve. Perhaps a separate commit.
> + p->stats.msg_rcvd_open, EV("messages"), EV("open"), ol);
> + ometric_set_int_with_labels(peer_message_recieve,
> + p->stats.msg_rcvd_notification, EV("messages"), EV("notification"),
> + ol);
> + ometric_set_int_with_labels(peer_message_recieve,
> + p->stats.msg_rcvd_update, EV("messages"), EV("update"), ol);
> + ometric_set_int_with_labels(peer_message_recieve,
> + p->stats.msg_rcvd_keepalive, EV("messages"), EV("keepalive"), ol);
> + ometric_set_int_with_labels(peer_message_recieve,
> + p->stats.msg_rcvd_rrefresh, EV("messages"), EV("route_refresh"),
> + ol);
>
> ometric_set_int(peer_update_transmit, p->stats.prefix_sent_update, ol);
> ometric_set_int(peer_update_pending, p->stats.pending_update, ol);
> @@ -249,13 +253,14 @@ ometric_rib_mem_element(const char *v, u
> uint64_t refs)
> {
> if (count != UINT64_MAX)
> - ometric_set_int_with_label(rde_mem_count, count, "type", v,
> - NULL);
> + ometric_set_int_with_labels(rde_mem_count, count,
> + EV("type"), EV(v), NULL);
> if (size != UINT64_MAX)
> - ometric_set_int_with_label(rde_mem_size, size, "type", v, NULL);
> + ometric_set_int_with_labels(rde_mem_size, size,
> + EV("type"), EV(v), NULL);
> if (refs != UINT64_MAX)
> - ometric_set_int_with_label(rde_mem_ref_count, refs, "type", v,
> - NULL);
> + ometric_set_int_with_labels(rde_mem_ref_count, refs,
> + EV("type"), EV(v), NULL);
> }
>
> static void
> @@ -297,14 +302,15 @@ ometric_rib_mem(struct rde_memstats *sta
> stats->attr_data, UINT64_MAX);
>
> ometric_set_int(rde_table_count, stats->aset_cnt, NULL);
> - ometric_set_int_with_label(rde_set_size, stats->aset_size,
> - "type", "as_set", NULL);
> - ometric_set_int_with_label(rde_set_count, stats->aset_nmemb,
> - "type", "as_set", NULL);
> - ometric_set_int_with_label(rde_set_size, stats->pset_size,
> - "type", "prefix_set", NULL);
> - ometric_set_int_with_label(rde_set_count, stats->pset_cnt,
> - "type", "prefix_set", NULL);
> +
> + ometric_set_int_with_labels(rde_set_size, stats->aset_size,
> + EV("type"), EV("as_set"), NULL);
please fix indent: four spaces indent before EV("type"), not three
(also in the lines below).
> + ometric_set_int_with_labels(rde_set_count, stats->aset_nmemb,
> + EV("type"), EV("as_set"), NULL);
> + ometric_set_int_with_labels(rde_set_size, stats->pset_size,
> + EV("type"), EV("prefix_set"), NULL);
> + ometric_set_int_with_labels(rde_set_count, stats->pset_cnt,
> + EV("type"), EV("prefix_set"), NULL);
> ometric_rib_mem_element("set_total", UINT64_MAX,
> stats->aset_size + stats->pset_size, UINT64_MAX);
> }
>
