DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] telemetry: rework code to avoid compiler warnings
@ 2023-02-08 14:37 Bruce Richardson
  2023-02-08 15:00 ` Morten Brørup
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Bruce Richardson @ 2023-02-08 14:37 UTC (permalink / raw)
  To: dev; +Cc: Huisong Li, Chengwen Feng, Morten Brørup, Bruce Richardson

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] telemetry: rework code to avoid compiler warnings
  2023-02-08 14:37 [PATCH] telemetry: rework code to avoid compiler warnings Bruce Richardson
@ 2023-02-08 15:00 ` Morten Brørup
  2023-02-08 15:05 ` Wiles, Keith
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Morten Brørup @ 2023-02-08 15:00 UTC (permalink / raw)
  To: Bruce Richardson, dev; +Cc: Huisong Li, Chengwen Feng

> 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>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] telemetry: rework code to avoid compiler warnings
  2023-02-08 14:37 [PATCH] telemetry: rework code to avoid compiler warnings Bruce Richardson
  2023-02-08 15:00 ` Morten Brørup
@ 2023-02-08 15:05 ` Wiles, Keith
  2023-02-09  1:07 ` fengchengwen
  2023-02-09  7:45 ` David Marchand
  3 siblings, 0 replies; 6+ messages in thread
From: Wiles, Keith @ 2023-02-08 15:05 UTC (permalink / raw)
  To: Richardson, Bruce, dev; +Cc: Huisong Li, Chengwen Feng, Morten Brørup

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>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] telemetry: rework code to avoid compiler warnings
  2023-02-08 14:37 [PATCH] telemetry: rework code to avoid compiler warnings Bruce Richardson
  2023-02-08 15:00 ` Morten Brørup
  2023-02-08 15:05 ` Wiles, Keith
@ 2023-02-09  1:07 ` fengchengwen
  2023-02-09  2:03   ` lihuisong (C)
  2023-02-09  7:45 ` David Marchand
  3 siblings, 1 reply; 6+ messages in thread
From: fengchengwen @ 2023-02-09  1:07 UTC (permalink / raw)
  To: Bruce Richardson, dev; +Cc: Huisong Li, Morten Brørup

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>
> ---

...

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] telemetry: rework code to avoid compiler warnings
  2023-02-09  1:07 ` fengchengwen
@ 2023-02-09  2:03   ` lihuisong (C)
  0 siblings, 0 replies; 6+ messages in thread
From: lihuisong (C) @ 2023-02-09  2:03 UTC (permalink / raw)
  To: Bruce Richardson, dev; +Cc: Morten Brørup, fengchengwen


在 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>
>> ---
> ...
> .

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] telemetry: rework code to avoid compiler warnings
  2023-02-08 14:37 [PATCH] telemetry: rework code to avoid compiler warnings Bruce Richardson
                   ` (2 preceding siblings ...)
  2023-02-09  1:07 ` fengchengwen
@ 2023-02-09  7:45 ` David Marchand
  3 siblings, 0 replies; 6+ messages in thread
From: David Marchand @ 2023-02-09  7:45 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev, Huisong Li, Chengwen Feng, Morten Brørup

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.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-02-09  7:45 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-08 14:37 [PATCH] telemetry: rework code to avoid compiler warnings Bruce Richardson
2023-02-08 15:00 ` Morten Brørup
2023-02-08 15:05 ` Wiles, Keith
2023-02-09  1:07 ` fengchengwen
2023-02-09  2:03   ` lihuisong (C)
2023-02-09  7:45 ` David Marchand

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).