telemetry: rework code to avoid compiler warnings

  When printing values as hex strings, the telemetry code temporarily
disabled warnings about non-literal format strings. This was because the
actual format string was built-up programmatically to ensure the output
was of the desired bitwidth.

However, this code can be reworked and shortened by taking advantage of
the "*" printf flag, which is used to specify that the output width is
given by a separate printf parameter. This allows the format to be a
literal string in all cases, and also allows the code in the function to
be shortened considerably.

Note: the type of the width should be an "int" variable, which is why
this patch changes the type of the existing variable. Also, while we
could shorten the format string by using the "#" flag in place of an
explicit "0x", this would make the code more confusing because it would
mean that the "0x" would be included in the specified with, forcing us
to add 2 to the existing computed width.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/telemetry/telemetry_data.c | 38 +++++++---------------------------
 1 file changed, 7 insertions(+), 31 deletions(-)
  

> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> Sent: Wednesday, 8 February 2023 15.38
> To: dev@dpdk.org
> Cc: Huisong Li; Chengwen Feng; Morten Brørup; Bruce Richardson
> Subject: [PATCH] telemetry: rework code to avoid compiler warnings
> 
> When printing values as hex strings, the telemetry code temporarily
> disabled warnings about non-literal format strings. This was because
> the
> actual format string was built-up programmatically to ensure the output
> was of the desired bitwidth.
> 
> However, this code can be reworked and shortened by taking advantage of
> the "*" printf flag, which is used to specify that the output width is
> given by a separate printf parameter. This allows the format to be a
> literal string in all cases, and also allows the code in the function
> to
> be shortened considerably.
> 
> Note: the type of the width should be an "int" variable, which is why
> this patch changes the type of the existing variable. Also, while we
> could shorten the format string by using the "#" flag in place of an
> explicit "0x", this would make the code more confusing because it would
> mean that the "0x" would be included in the specified with, forcing us
> to add 2 to the existing computed width.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---

Beautiful.

Acked-by: Morten Brørup <mb@smartsharesystems.com>
  

From: Bruce Richardson <bruce.richardson@intel.com>
Sent: Wednesday, February 8, 2023 8:37 AM
To: dev@dpdk.org
Cc: Huisong Li; Chengwen Feng; Morten Brørup; Richardson, Bruce
Subject: [PATCH] telemetry: rework code to avoid compiler warnings

When printing values as hex strings, the telemetry code temporarily
disabled warnings about non-literal format strings. This was because the
actual format string was built-up programmatically to ensure the output
was of the desired bitwidth.

However, this code can be reworked and shortened by taking advantage of
the "*" printf flag, which is used to specify that the output width is
given by a separate printf parameter. This allows the format to be a
literal string in all cases, and also allows the code in the function to
be shortened considerably.

Note: the type of the width should be an "int" variable, which is why
this patch changes the type of the existing variable. Also, while we
could shorten the format string by using the "#" flag in place of an
explicit "0x", this would make the code more confusing because it would
mean that the "0x" would be included in the specified with, forcing us
to add 2 to the existing computed width.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/telemetry/telemetry_data.c | 38 +++++++---------------------------
 1 file changed, 7 insertions(+), 31 deletions(-)

diff --git a/lib/telemetry/telemetry_data.c b/lib/telemetry/telemetry_data.c
index eb3cb98176..2bac2de2c2 100644
--- a/lib/telemetry/telemetry_data.c
+++ b/lib/telemetry/telemetry_data.c
@@ -119,45 +119,21 @@ rte_tel_data_add_array_container(struct rte_tel_data *d,
        return 0;
 }

-/* 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, size_t buf_len, uint64_t val,
                                uint8_t display_bitwidth)
 {
-#define RTE_TEL_HEX_FORMAT_LEN 16
-
-       uint8_t spec_hex_width = (display_bitwidth + 3) / 4;
-       char format[RTE_TEL_HEX_FORMAT_LEN];
-
-       if (display_bitwidth != 0) {
-               if (snprintf(format, RTE_TEL_HEX_FORMAT_LEN, "0x%%0%u" PRIx64,
-                               spec_hex_width) >= RTE_TEL_HEX_FORMAT_LEN)
-                       return -EINVAL;
+       int spec_hex_width = (display_bitwidth + 3) / 4;
+       int len;

-               if (snprintf(buf, buf_len, format, val) >= (int)buf_len)
-                       return -EINVAL;
-       } else {
-               if (snprintf(buf, buf_len, "0x%" PRIx64, val) >= (int)buf_len)
-                       return -EINVAL;
-       }
+       if (display_bitwidth != 0)
+               len = snprintf(buf, buf_len, "0x%0*" PRIx64, spec_hex_width, val);
+       else
+               len = snprintf(buf, buf_len, "0x%" PRIx64, val);

-       return 0;
+       return len < (int)buf_len ? 0 : -EINVAL;
 }

-#if defined(RTE_TOOLCHAIN_GCC)
-#pragma GCC diagnostic pop
-#elif defined(RTE_TOOLCHAIN_CLANG)
-#pragma clang diagnostic pop
-#endif
-
 int
 rte_tel_data_add_array_uint_hex(struct rte_tel_data *d, uint64_t val,
                                uint8_t display_bitwidth)
--
2.37.2

Acked-by: Keith Wiles <keith.wiles@intel.com>
  

Beatutiful +1

Acked-by: Chengwen Feng <fengchengwen@huawei.com>

On 2023/2/8 22:37, Bruce Richardson wrote:
> When printing values as hex strings, the telemetry code temporarily
> disabled warnings about non-literal format strings. This was because the
> actual format string was built-up programmatically to ensure the output
> was of the desired bitwidth.
> 
> However, this code can be reworked and shortened by taking advantage of
> the "*" printf flag, which is used to specify that the output width is
> given by a separate printf parameter. This allows the format to be a
> literal string in all cases, and also allows the code in the function to
> be shortened considerably.
> 
> Note: the type of the width should be an "int" variable, which is why
> this patch changes the type of the existing variable. Also, while we
> could shorten the format string by using the "#" flag in place of an
> explicit "0x", this would make the code more confusing because it would
> mean that the "0x" would be included in the specified with, forcing us
> to add 2 to the existing computed width.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---

...
  

在 2023/2/9 9:07, fengchengwen 写道:
> Beatutiful +1
>
> Acked-by: Chengwen Feng <fengchengwen@huawei.com>

Perfect

Tested-by: Huisong Li <lihuisong@huawei.com>

>
> On 2023/2/8 22:37, Bruce Richardson wrote:
>> When printing values as hex strings, the telemetry code temporarily
>> disabled warnings about non-literal format strings. This was because the
>> actual format string was built-up programmatically to ensure the output
>> was of the desired bitwidth.
>>
>> However, this code can be reworked and shortened by taking advantage of
>> the "*" printf flag, which is used to specify that the output width is
>> given by a separate printf parameter. This allows the format to be a
>> literal string in all cases, and also allows the code in the function to
>> be shortened considerably.
>>
>> Note: the type of the width should be an "int" variable, which is why
>> this patch changes the type of the existing variable. Also, while we
>> could shorten the format string by using the "#" flag in place of an
>> explicit "0x", this would make the code more confusing because it would
>> mean that the "0x" would be included in the specified with, forcing us
>> to add 2 to the existing computed width.
>>
>> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
>> ---
> ...
> .
  

On Wed, Feb 8, 2023 at 3:38 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> When printing values as hex strings, the telemetry code temporarily
> disabled warnings about non-literal format strings. This was because the
> actual format string was built-up programmatically to ensure the output
> was of the desired bitwidth.
>
> However, this code can be reworked and shortened by taking advantage of
> the "*" printf flag, which is used to specify that the output width is
> given by a separate printf parameter. This allows the format to be a
> literal string in all cases, and also allows the code in the function to
> be shortened considerably.
>
> Note: the type of the width should be an "int" variable, which is why
> this patch changes the type of the existing variable. Also, while we
> could shorten the format string by using the "#" flag in place of an
> explicit "0x", this would make the code more confusing because it would
> mean that the "0x" would be included in the specified with, forcing us
> to add 2 to the existing computed width.
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

Applied, thanks.