On Wed, Nov 30, 2022 at 10:36:08AM +0100, Claudio Jeker wrote:
> I want to use the bgpctl ometric.c code in rpki-client to implement a
> metrics output. Currently ometric_output_all() just dumps to stdout but
> that does not work for rpki-client. Instead pass a FILE pointer to
> ometric_output_all() and also return -1 if an error occured. With this
> ometric usage becomes more versatile.
The *printf functions are documented to return a value less than 0 on
error. Your diff assumes that value is -1 which I believe to be correct
on OpenBSD. Is it safe to assume that this is the case for -portable?
Should we ensure -1 is returned in the ometric_foo() functions, should
the ometric_foo() functions be error checked with if (ometric_foo() < 0)
or should we leave the diff as-is because I'm overthinking this?
Other than that, the diff reads fine to me.
> --
> :wq Claudio
>
> Index: ometric.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpctl/ometric.c,v
> retrieving revision 1.2
> diff -u -p -r1.2 ometric.c
> --- ometric.c 1 Nov 2022 13:35:09 -0000 1.2
> +++ ometric.c 30 Nov 2022 08:56:26 -0000
> @@ -246,66 +246,77 @@ ometric_type(enum ometric_type type)
> }
> }
>
> -static void
> -ometric_output_labels(const struct olabels *ol)
> +static int
> +ometric_output_labels(FILE *out, const struct olabels *ol)
> {
> struct olabel *l;
> const char *comma = "";
>
> - if (ol == NULL) {
> - printf(" ");
> - return;
> - }
> + if (ol == NULL)
> + return fprintf(out, " ");
>
> - printf("{");
> + if (fprintf(out, "{") == -1)
> + return -1;
>
> while (ol != NULL) {
> STAILQ_FOREACH(l, &ol->labels, entry) {
> - printf("%s%s=\"%s\"", comma, l->key, l->value);
> + if (fprintf(out, "%s%s=\"%s\"", comma, l->key,
> + l->value) == -1)
> + return -1;
> comma = ",";
> }
> ol = ol->next;
> }
>
> - printf("} ");
> + return fprintf(out, "} ");
> }
>
> -static void
> -ometric_output_value(const struct ovalue *ov)
> +static int
> +ometric_output_value(FILE *out, const struct ovalue *ov)
> {
> switch (ov->valtype) {
> case OVT_INTEGER:
> - printf("%llu", ov->value.i);
> - return;
> + return fprintf(out, "%llu", ov->value.i);
> case OVT_DOUBLE:
> - printf("%g", ov->value.f);
> - return;
> + return fprintf(out, "%g", ov->value.f);
> }
> + return -1;
> }
>
> /*
> * Output all metric values with TYPE and optional HELP strings.
> */
> -void
> -ometric_output_all(void)
> +int
> +ometric_output_all(FILE *out)
> {
> struct ometric *om;
> struct ovalue *ov;
>
> STAILQ_FOREACH(om, &ometrics, entry) {
> if (om->help)
> - printf("# HELP %s %s\n", om->name, om->help);
> - printf("# TYPE %s %s\n", om->name, ometric_type(om->type));
> + if (fprintf(out, "# HELP %s %s\n", om->name,
> + om->help) == -1)
> + return -1;
> +
> + if (fprintf(out, "# TYPE %s %s\n", om->name,
> + ometric_type(om->type)) == -1)
> + return -1;
>
> STAILQ_FOREACH(ov, &om->vals, entry) {
> - printf("%s", om->name);
> - ometric_output_labels(ov->labels);
> - ometric_output_value(ov);
> - printf("\n");
> + if (fprintf(out, "%s", om->name) == -1)
> + return -1;
> + if (ometric_output_labels(out, ov->labels) == -1)
> + return -1;
> + if (ometric_output_value(out, ov) == -1)
> + return -1;
> + if (fprintf(out, "\n") == -1)
> + return -1;
> }
> }
>
> - printf("# EOF\n");
> + if (fprintf(out, "# EOF\n") == -1)
> + return -1;
> + return 0;
> }
>
> /*
> Index: ometric.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpctl/ometric.h,v
> retrieving revision 1.1
> diff -u -p -r1.1 ometric.h
> --- ometric.h 17 Oct 2022 12:01:19 -0000 1.1
> +++ ometric.h 30 Nov 2022 08:49:36 -0000
> @@ -36,9 +36,8 @@ void ometric_free_all(void);
> struct olabels *olabels_new(const char * const *, const char **);
> void olabels_free(struct olabels *);
>
> -void ometric_output_all(void);
> +int ometric_output_all(FILE *);
>
> -/* XXX how to pass attributes */
> /* functions to set gauge and counter metrics */
> void ometric_set_int(struct ometric *, uint64_t, struct olabels *);
> void ometric_set_float(struct ometric *, double, struct olabels *);
> Index: output_ometric.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpctl/output_ometric.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 output_ometric.c
> --- output_ometric.c 7 Nov 2022 11:33:24 -0000 1.3
> +++ output_ometric.c 30 Nov 2022 08:48:32 -0000
> @@ -75,9 +75,7 @@ ometric_head(struct parse_result *arg)
> values[3] = NULL;
>
> ol = olabels_new(keys, values);
> -
> ometric_set_info(bgpd_info, NULL, NULL, ol);
> -
> olabels_free(ol);
>
> /*
> @@ -324,7 +322,7 @@ ometric_tail(void)
> (double)elapsed_time.tv_usec / 1000000;
>
> ometric_set_float(bgpd_scrape_time, scrape, NULL);
> - ometric_output_all();
> + ometric_output_all(stdout);
>
> ometric_free_all();
> }
>
