[v2,06/16] telemetry: add utility functions for creating json

Message ID 20200408164956.47864-7-ciara.power@intel.com (mailing list archive)
State Superseded, archived
Delegated to: David Marchand
Headers
Series update and simplify telemetry library. |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Power, Ciara April 8, 2020, 4:49 p.m. UTC
  From: Bruce Richardson <bruce.richardson@intel.com>

The functions added in this patch will make it easier for applications
to build up correct JSON responses to telemetry requests.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/librte_telemetry/Makefile             |   1 +
 lib/librte_telemetry/meson.build          |   2 +-
 lib/librte_telemetry/rte_telemetry.h      |   1 +
 lib/librte_telemetry/rte_telemetry_json.h | 205 ++++++++++++++++++++++
 4 files changed, 208 insertions(+), 1 deletion(-)
 create mode 100644 lib/librte_telemetry/rte_telemetry_json.h
  

Comments

Wiles, Keith April 8, 2020, 6:12 p.m. UTC | #1
> On Apr 8, 2020, at 11:49 AM, Power, Ciara <ciara.power@intel.com> wrote:
> 
> From: Bruce Richardson <bruce.richardson@intel.com>
> 
> The functions added in this patch will make it easier for applications
> to build up correct JSON responses to telemetry requests.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
> lib/librte_telemetry/Makefile             |   1 +
> lib/librte_telemetry/meson.build          |   2 +-
> lib/librte_telemetry/rte_telemetry.h      |   1 +
> lib/librte_telemetry/rte_telemetry_json.h | 205 ++++++++++++++++++++++
> 4 files changed, 208 insertions(+), 1 deletion(-)
> create mode 100644 lib/librte_telemetry/rte_telemetry_json.h
> 
> diff --git a/lib/librte_telemetry/Makefile b/lib/librte_telemetry/Makefile
> index 74a6e2d2f7..9012156c1b 100644
> --- a/lib/librte_telemetry/Makefile
> +++ b/lib/librte_telemetry/Makefile
> @@ -29,5 +29,6 @@ SRCS-$(CONFIG_RTE_LIBRTE_TELEMETRY) += telemetry.c
> 
> # export include files
> SYMLINK-$(CONFIG_RTE_LIBRTE_TELEMETRY)-include := rte_telemetry.h
> +SYMLINK-$(CONFIG_RTE_LIBRTE_TELEMETRY)-include += rte_telemetry_json.h
> 
> include $(RTE_SDK)/mk/rte.lib.mk
> diff --git a/lib/librte_telemetry/meson.build b/lib/librte_telemetry/meson.build
> index 710c119b7a..cc0cdeb5f6 100644
> --- a/lib/librte_telemetry/meson.build
> +++ b/lib/librte_telemetry/meson.build
> @@ -5,7 +5,7 @@ includes = [global_inc]
> 
> sources = files('rte_telemetry.c', 'rte_telemetry_parser.c', 'rte_telemetry_parser_test.c',
> 	'telemetry.c')
> -headers = files('rte_telemetry.h', 'rte_telemetry_internal.h', 'rte_telemetry_parser.h')
> +headers = files('rte_telemetry.h', 'rte_telemetry_internal.h', 'rte_telemetry_parser.h', 'rte_telemetry_json.h')
> cflags += '-DALLOW_EXPERIMENTAL_API'
> includes += include_directories('../librte_metrics')
> 
> diff --git a/lib/librte_telemetry/rte_telemetry.h b/lib/librte_telemetry/rte_telemetry.h
> index d0230d4544..8b3df04292 100644
> --- a/lib/librte_telemetry/rte_telemetry.h
> +++ b/lib/librte_telemetry/rte_telemetry.h
> @@ -4,6 +4,7 @@
> 
> #include <stdint.h>
> #include <rte_compat.h>
> +#include <rte_telemetry_json.h>
> 
> #ifndef _RTE_TELEMETRY_H_
> #define _RTE_TELEMETRY_H_
> diff --git a/lib/librte_telemetry/rte_telemetry_json.h b/lib/librte_telemetry/rte_telemetry_json.h
> new file mode 100644
> index 0000000000..02fcafc73a
> --- /dev/null
> +++ b/lib/librte_telemetry/rte_telemetry_json.h
> @@ -0,0 +1,205 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2020 Intel Corporation
> + */
> +
> +#ifndef _RTE_TELEMETRY_JSON_H_
> +#define _RTE_TELEMETRY_JSON_H_
> +
> +#include <inttypes.h>
> +#include <stdarg.h>
> +#include <stdio.h>
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: all functions in this file may change without prior notice
> + *
> + * @file
> + * RTE Telemetry Utility Functions for Creating JSON Responses
> + *
> + * This file contains small inline functions to make it easier for applications
> + * to build up valid JSON responses to telemetry requests.
> + *
> + ***/
> +
> +/**
> + * @internal
> + *
> + * Copies a value into a buffer if the buffer has enough available space.
> + * Nothing written to buffer if an overflow ocurs.
> + * This function is not for use for values larger than 1k.
> + *
> + * @param buf
> + * Buffer for data to be appended to.
> + * @param len
> + * Length of buffer.
> + * @param format
> + * Format string.
> + * @param ...
> + * Optional arguments that may be required by the format string.
> + *
> + * @return
> + *  Number of characters added to buffer
> + */
> +__attribute__((__format__(__printf__, 3, 4)))
> +static inline int
> +__json_snprintf(char *buf, const int len, const char *format, ...)
> +{
> +	char tmp[1024];
> +	va_list ap;
> +	int ret;
> +
> +	va_start(ap, format);
> +	ret = vsnprintf(tmp, sizeof(tmp), format, ap);
> +	va_end(ap);

Would strlcpy(buf, tmp, len); reduce the test below? vsnprintf() writes at most size bytes including the ‘\0’ to the buffer. Maybe able to reduce the code to this:

	return strlcpy(buf, tmp, len);		// strlcpy returns the total length of buf.

This means the ret is not really required, unless I missed something. 

> +	if (ret > 0 && ret < (int)sizeof(tmp) && ret < len) {
> +		strcpy(buf, tmp);
> +		return ret;
> +	}
> +	return 0; /* nothing written or modified */
> +}
> +
> +/**
> + * Copies an empty array into the provided buffer.
> + *
> + * @param buf
> + * Buffer to hold the empty array.
> + * @param len
> + * Length of buffer.
> + * @param used
> + * The number of used characters in the buffer.
> + *
> + * @return
> + *  Total number of characters in buffer.
> + */
> +static inline int
> +rte_tel_json_empty_array(char *buf, const int len, const int used)
> +{
> +	return used + __json_snprintf(buf + used, len - used, "[]");
> +}
> +
> +/**
> + * Copies an empty object into the provided buffer.
> + *
> + * @param buf
> + * Buffer to hold the empty object.
> + * @param len
> + * Length of buffer.
> + * @param used
> + * The number of used characters in the buffer.
> + *
> + * @return
> + *  Total number of characters in buffer
> + */
> +static inline int
> +rte_tel_json_empty_obj(char *buf, const int len, const int used)
> +{
> +	return used + __json_snprintf(buf + used, len - used, "{}");
> +}
> +
> +/**
> + * Copies a string into the provided buffer, in JSON format.
> + *
> + * @param buf
> + * Buffer to copy string into.
> + * @param len
> + * Length of buffer.
> + * @param used
> + * The number of used characters in the buffer.
> + * @param str
> + * String value to copy into buffer.
> + *
> + * @return
> + *  Total number of characters in buffer
> + */
> +static inline int
> +rte_tel_json_str(char *buf, const int len, const int used, const char *str)
> +{
> +	return used + __json_snprintf(buf + used, len - used, "\"%s\"", str);
> +}
> +
> +/**
> + * Appends a string into the JSON array in the provided buffer.
> + *
> + * @param buf
> + * Buffer to append array string to.
> + * @param len
> + * Length of buffer.
> + * @param used
> + * The number of used characters in the buffer.
> + * @param str
> + * String value to append to buffer.
> + *
> + * @return
> + *  Total number of characters in buffer
> + */
> +static inline int
> +rte_tel_json_add_array_string(char *buf, const int len, const int used,
> +		const char *str)
> +{
> +	int ret, end = used - 1; /* strip off final delimiter */
> +	if (used <= 2) /* assume empty, since minimum is '[]' */
> +		return __json_snprintf(buf, len, "[\"%s\"]", str);
> +
> +	ret = __json_snprintf(buf + end, len - end, ",\"%s\"]", str);
> +	return ret == 0 ? used : end + ret;
> +}
> +
> +/**
> + * Appends an integer into the JSON array in the provided buffer.
> + *
> + * @param buf
> + * Buffer to append array integer to.
> + * @param len
> + * Length of buffer.
> + * @param used
> + * The number of used characters in the buffer.
> + * @param val
> + * Integer value to append to buffer.
> + *
> + * @return
> + *  Total number of characters in buffer
> + */
> +static inline int
> +rte_tel_json_add_array_int(char *buf, const int len, const int used, int val)
> +{
> +	int ret, end = used - 1; /* strip off final delimiter */
> +	if (used <= 2) /* assume empty, since minimum is '[]' */
> +		return __json_snprintf(buf, len, "[%d]", val);
> +
> +	ret = __json_snprintf(buf + end, len - end, ",%d]", val);
> +	return ret == 0 ? used : end + ret;
> +}
> +
> +/**
> + * Add a new element with uint64_t value to the JSON object stored in the
> + * provided buffer.
> + *
> + * @param buf
> + * Buffer to append object element to.
> + * @param len
> + * Length of buffer.
> + * @param used
> + * The number of used characters in the buffer.
> + * @param name
> + * String for object element key.
> + * @param val
> + * Uint64_t for object element value.
> + *
> + * @return
> + *  Total number of characters in buffer
> + */
> +static inline int
> +rte_tel_json_add_obj_u64(char *buf, const int len, const int used,
> +		const char *name, uint64_t val)
> +{
> +	int ret, end = used - 1;
> +	if (used <= 2) /* assume empty, since minimum is '{}' */
> +		return __json_snprintf(buf, len, "{\"%s\":%"PRIu64"}", name,
> +				val);
> +
> +	ret = __json_snprintf(buf + end, len - end, ",\"%s\":%"PRIu64"}",
> +			name, val);
> +	return ret == 0 ? used : end + ret;
> +}
> +
> +#endif /*_RTE_TELEMETRY_JSON_H_*/
> -- 
> 2.17.1
>
  
Bruce Richardson April 9, 2020, 8:19 a.m. UTC | #2
On Wed, Apr 08, 2020 at 07:12:57PM +0100, Wiles, Keith wrote:
> 
> 
> > On Apr 8, 2020, at 11:49 AM, Power, Ciara <ciara.power@intel.com> wrote:
> >
> > From: Bruce Richardson <bruce.richardson@intel.com>
> >
> > The functions added in this patch will make it easier for applications
> > to build up correct JSON responses to telemetry requests.
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> > lib/librte_telemetry/Makefile             |   1 +
> > lib/librte_telemetry/meson.build          |   2 +-
> > lib/librte_telemetry/rte_telemetry.h      |   1 +
> > lib/librte_telemetry/rte_telemetry_json.h | 205 ++++++++++++++++++++++
> > 4 files changed, 208 insertions(+), 1 deletion(-)
> > create mode 100644 lib/librte_telemetry/rte_telemetry_json.h
<snip>
> > +/**
> > + * @internal
> > + *
> > + * Copies a value into a buffer if the buffer has enough available space.
> > + * Nothing written to buffer if an overflow ocurs.
> > + * This function is not for use for values larger than 1k.
> > + *
> > + * @param buf
> > + * Buffer for data to be appended to.
> > + * @param len
> > + * Length of buffer.
> > + * @param format
> > + * Format string.
> > + * @param ...
> > + * Optional arguments that may be required by the format string.
> > + *
> > + * @return
> > + *  Number of characters added to buffer
> > + */
> > +__attribute__((__format__(__printf__, 3, 4)))
> > +static inline int
> > +__json_snprintf(char *buf, const int len, const char *format, ...)
> > +{
> > +char tmp[1024];
> > +va_list ap;
> > +int ret;
> > +
> > +va_start(ap, format);
> > +ret = vsnprintf(tmp, sizeof(tmp), format, ap);
> > +va_end(ap);
> 
> Would strlcpy(buf, tmp, len); reduce the test below? vsnprintf() writes at most size bytes including the ‘\0’ to the buffer. Maybe able to reduce the code to this:
> 
> return strlcpy(buf, tmp, len);// strlcpy returns the total length of buf.
> 
> This means the ret is not really required, unless I missed something.
> 
> > +if (ret > 0 && ret < (int)sizeof(tmp) && ret < len) {
> > +strcpy(buf, tmp);
> > +return ret;
> > +}
> > +return 0; /* nothing written or modified */
> > +}
> > +
The intent here is that the buffer remains unmodified and the function
returns zero in the case that the printf fails to work. Truncation would
likely leave us with invalid json, so it's all or nothing, which is why we
can't just take the snprintf or strlcpy. [On overflow they still modify the
buffer, which means any closing "]" or "}" in the json would be
overwritten.]

/Bruce
  

Patch

diff --git a/lib/librte_telemetry/Makefile b/lib/librte_telemetry/Makefile
index 74a6e2d2f7..9012156c1b 100644
--- a/lib/librte_telemetry/Makefile
+++ b/lib/librte_telemetry/Makefile
@@ -29,5 +29,6 @@  SRCS-$(CONFIG_RTE_LIBRTE_TELEMETRY) += telemetry.c
 
 # export include files
 SYMLINK-$(CONFIG_RTE_LIBRTE_TELEMETRY)-include := rte_telemetry.h
+SYMLINK-$(CONFIG_RTE_LIBRTE_TELEMETRY)-include += rte_telemetry_json.h
 
 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_telemetry/meson.build b/lib/librte_telemetry/meson.build
index 710c119b7a..cc0cdeb5f6 100644
--- a/lib/librte_telemetry/meson.build
+++ b/lib/librte_telemetry/meson.build
@@ -5,7 +5,7 @@  includes = [global_inc]
 
 sources = files('rte_telemetry.c', 'rte_telemetry_parser.c', 'rte_telemetry_parser_test.c',
 	'telemetry.c')
-headers = files('rte_telemetry.h', 'rte_telemetry_internal.h', 'rte_telemetry_parser.h')
+headers = files('rte_telemetry.h', 'rte_telemetry_internal.h', 'rte_telemetry_parser.h', 'rte_telemetry_json.h')
 cflags += '-DALLOW_EXPERIMENTAL_API'
 includes += include_directories('../librte_metrics')
 
diff --git a/lib/librte_telemetry/rte_telemetry.h b/lib/librte_telemetry/rte_telemetry.h
index d0230d4544..8b3df04292 100644
--- a/lib/librte_telemetry/rte_telemetry.h
+++ b/lib/librte_telemetry/rte_telemetry.h
@@ -4,6 +4,7 @@ 
 
 #include <stdint.h>
 #include <rte_compat.h>
+#include <rte_telemetry_json.h>
 
 #ifndef _RTE_TELEMETRY_H_
 #define _RTE_TELEMETRY_H_
diff --git a/lib/librte_telemetry/rte_telemetry_json.h b/lib/librte_telemetry/rte_telemetry_json.h
new file mode 100644
index 0000000000..02fcafc73a
--- /dev/null
+++ b/lib/librte_telemetry/rte_telemetry_json.h
@@ -0,0 +1,205 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2020 Intel Corporation
+ */
+
+#ifndef _RTE_TELEMETRY_JSON_H_
+#define _RTE_TELEMETRY_JSON_H_
+
+#include <inttypes.h>
+#include <stdarg.h>
+#include <stdio.h>
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: all functions in this file may change without prior notice
+ *
+ * @file
+ * RTE Telemetry Utility Functions for Creating JSON Responses
+ *
+ * This file contains small inline functions to make it easier for applications
+ * to build up valid JSON responses to telemetry requests.
+ *
+ ***/
+
+/**
+ * @internal
+ *
+ * Copies a value into a buffer if the buffer has enough available space.
+ * Nothing written to buffer if an overflow ocurs.
+ * This function is not for use for values larger than 1k.
+ *
+ * @param buf
+ * Buffer for data to be appended to.
+ * @param len
+ * Length of buffer.
+ * @param format
+ * Format string.
+ * @param ...
+ * Optional arguments that may be required by the format string.
+ *
+ * @return
+ *  Number of characters added to buffer
+ */
+__attribute__((__format__(__printf__, 3, 4)))
+static inline int
+__json_snprintf(char *buf, const int len, const char *format, ...)
+{
+	char tmp[1024];
+	va_list ap;
+	int ret;
+
+	va_start(ap, format);
+	ret = vsnprintf(tmp, sizeof(tmp), format, ap);
+	va_end(ap);
+	if (ret > 0 && ret < (int)sizeof(tmp) && ret < len) {
+		strcpy(buf, tmp);
+		return ret;
+	}
+	return 0; /* nothing written or modified */
+}
+
+/**
+ * Copies an empty array into the provided buffer.
+ *
+ * @param buf
+ * Buffer to hold the empty array.
+ * @param len
+ * Length of buffer.
+ * @param used
+ * The number of used characters in the buffer.
+ *
+ * @return
+ *  Total number of characters in buffer.
+ */
+static inline int
+rte_tel_json_empty_array(char *buf, const int len, const int used)
+{
+	return used + __json_snprintf(buf + used, len - used, "[]");
+}
+
+/**
+ * Copies an empty object into the provided buffer.
+ *
+ * @param buf
+ * Buffer to hold the empty object.
+ * @param len
+ * Length of buffer.
+ * @param used
+ * The number of used characters in the buffer.
+ *
+ * @return
+ *  Total number of characters in buffer
+ */
+static inline int
+rte_tel_json_empty_obj(char *buf, const int len, const int used)
+{
+	return used + __json_snprintf(buf + used, len - used, "{}");
+}
+
+/**
+ * Copies a string into the provided buffer, in JSON format.
+ *
+ * @param buf
+ * Buffer to copy string into.
+ * @param len
+ * Length of buffer.
+ * @param used
+ * The number of used characters in the buffer.
+ * @param str
+ * String value to copy into buffer.
+ *
+ * @return
+ *  Total number of characters in buffer
+ */
+static inline int
+rte_tel_json_str(char *buf, const int len, const int used, const char *str)
+{
+	return used + __json_snprintf(buf + used, len - used, "\"%s\"", str);
+}
+
+/**
+ * Appends a string into the JSON array in the provided buffer.
+ *
+ * @param buf
+ * Buffer to append array string to.
+ * @param len
+ * Length of buffer.
+ * @param used
+ * The number of used characters in the buffer.
+ * @param str
+ * String value to append to buffer.
+ *
+ * @return
+ *  Total number of characters in buffer
+ */
+static inline int
+rte_tel_json_add_array_string(char *buf, const int len, const int used,
+		const char *str)
+{
+	int ret, end = used - 1; /* strip off final delimiter */
+	if (used <= 2) /* assume empty, since minimum is '[]' */
+		return __json_snprintf(buf, len, "[\"%s\"]", str);
+
+	ret = __json_snprintf(buf + end, len - end, ",\"%s\"]", str);
+	return ret == 0 ? used : end + ret;
+}
+
+/**
+ * Appends an integer into the JSON array in the provided buffer.
+ *
+ * @param buf
+ * Buffer to append array integer to.
+ * @param len
+ * Length of buffer.
+ * @param used
+ * The number of used characters in the buffer.
+ * @param val
+ * Integer value to append to buffer.
+ *
+ * @return
+ *  Total number of characters in buffer
+ */
+static inline int
+rte_tel_json_add_array_int(char *buf, const int len, const int used, int val)
+{
+	int ret, end = used - 1; /* strip off final delimiter */
+	if (used <= 2) /* assume empty, since minimum is '[]' */
+		return __json_snprintf(buf, len, "[%d]", val);
+
+	ret = __json_snprintf(buf + end, len - end, ",%d]", val);
+	return ret == 0 ? used : end + ret;
+}
+
+/**
+ * Add a new element with uint64_t value to the JSON object stored in the
+ * provided buffer.
+ *
+ * @param buf
+ * Buffer to append object element to.
+ * @param len
+ * Length of buffer.
+ * @param used
+ * The number of used characters in the buffer.
+ * @param name
+ * String for object element key.
+ * @param val
+ * Uint64_t for object element value.
+ *
+ * @return
+ *  Total number of characters in buffer
+ */
+static inline int
+rte_tel_json_add_obj_u64(char *buf, const int len, const int used,
+		const char *name, uint64_t val)
+{
+	int ret, end = used - 1;
+	if (used <= 2) /* assume empty, since minimum is '{}' */
+		return __json_snprintf(buf, len, "{\"%s\":%"PRIu64"}", name,
+				val);
+
+	ret = __json_snprintf(buf + end, len - end, ",\"%s\":%"PRIu64"}",
+			name, val);
+	return ret == 0 ? used : end + ret;
+}
+
+#endif /*_RTE_TELEMETRY_JSON_H_*/