[PATCH V6 6/8] telemetry: support adding integer value as hexadecimal

[Bruce Richardson]

On Thu, Dec 15, 2022 at 06:31:44PM +0800, Huisong Li wrote:
> Sometimes displaying a unsigned integer value as hexadecimal encoded style
> is more expected for human consumption, such as, offload capability and
> device flag. This patch introduces two APIs to add unsigned integer value
> as hexadecimal encoded string to array or dictionary. And user can choose
> whether the stored value is padding to the specified width.
> 
> Signed-off-by: Huisong Li <lihuisong at huawei.com>
> Acked-by: Morten Brørup <mb at smartsharesystems.com>
> Acked-by: Chengwen Feng <fengchengwen at huawei.com>
> ---
>  lib/telemetry/rte_telemetry.h  | 47 ++++++++++++++++++++++
>  lib/telemetry/telemetry_data.c | 72 ++++++++++++++++++++++++++++++++++
>  lib/telemetry/version.map      |  9 +++++
>  3 files changed, 128 insertions(+)
> 
<snip>
> +/* To suppress compiler warning about format string. */
> +#if defined(RTE_TOOLCHAIN_GCC)
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wformat-nonliteral"
> +#elif defined(RTE_TOOLCHAIN_CLANG)
> +#pragma clang diagnostic push
> +#pragma clang diagnostic ignored "-Wformat-nonliteral"
> +#endif
> +
> +static int
> +rte_tel_uint_to_hex_encoded_str(char *buf, uint16_t len, uint64_t val,
> +				uint8_t display_bitwidth)
> +{
> +#define RTE_TEL_UINT_HEX_FORMAT_LEN 16
> +
> +	uint8_t spec_hex_width = (display_bitwidth + 3) / 4;
> +	char format[RTE_TEL_UINT_HEX_FORMAT_LEN];
> +
> +	/* Buffer needs to have room to contain the prefix '0x' and '\0'. */
> +	if (len < spec_hex_width + 3)
> +		return -EINVAL;
> +

This check is not sufficient, as display_bitwidth could be 0 for example,
and the actual printed number have up to 16 characters.

> +	if (display_bitwidth != 0) {
> +		sprintf(format, "0x%%0%u" PRIx64, spec_hex_width);
> +		sprintf(buf, format, val);
> +	} else {
> +		sprintf(buf, "0x%" PRIx64, val);
> +	}
> +

snprintf should always be used when printing to the buffer so as to avoid
overflow. The return value after snprintf should always be checked too.

Thanks,
/Bruce