drivers/common: avoid truncation of constant value

Message ID 1734537913-1159-1-git-send-email-andremue@linux.microsoft.com (mailing list archive)
State New
Delegated to: Bruce Richardson
Headers
Series drivers/common: avoid truncation of constant value |

Checks

Context Check Description
ci/loongarch-compilation success Compilation OK
ci/checkpatch success coding style OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/intel-Functional success Functional PASS
ci/github-robot: build success github build: passed
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-marvell-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS

Commit Message

Andre Muezerie Dec. 18, 2024, 4:05 p.m. UTC
This issue was flagged by MSVC warning below:

drivers\common\idpf\base/virtchnl2.h(269): warning C4309:
    'initializing': truncation of constant value

The problem is that 64-bit numbers are initialized in an enum.

The C11 standard states: The expression that defines the value of an
enumeration constant shall be an integer constant expression that
has a value representable as an int. [Section 6.7.2.2]

As a result compilers end up truncating these numbers. MSVC and Clang
do that. Gcc has an extension that makes it use a 64-bit size for the
enum to avoid the truncation.

At the moment this truncation is only a real problem for
VIRTCHNL2_CAP_OEM, but clearly all these capability flags are
intended to be 64 bits long and therefore should better not be defined
in an enum.

The fix is to use "defines" for them, as the code was using about
7 months ago before enums were introduced here. In this patch the
flag names and values are being preserved, only the enum is being
removed.

Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
---
 drivers/common/idpf/base/virtchnl2.h | 59 ++++++++++++++--------------
 1 file changed, 30 insertions(+), 29 deletions(-)
  

Comments

Andre Muezerie March 4, 2025, 2:15 a.m. UTC | #1
On Wed, Dec 18, 2024 at 08:05:12AM -0800, Andre Muezerie wrote:
> This issue was flagged by MSVC warning below:
> 
> drivers\common\idpf\base/virtchnl2.h(269): warning C4309:
>     'initializing': truncation of constant value
> 
> The problem is that 64-bit numbers are initialized in an enum.
> 
> The C11 standard states: The expression that defines the value of an
> enumeration constant shall be an integer constant expression that
> has a value representable as an int. [Section 6.7.2.2]
> 
> As a result compilers end up truncating these numbers. MSVC and Clang
> do that. Gcc has an extension that makes it use a 64-bit size for the
> enum to avoid the truncation.
> 
> At the moment this truncation is only a real problem for
> VIRTCHNL2_CAP_OEM, but clearly all these capability flags are
> intended to be 64 bits long and therefore should better not be defined
> in an enum.
> 
> The fix is to use "defines" for them, as the code was using about
> 7 months ago before enums were introduced here. In this patch the
> flag names and values are being preserved, only the enum is being
> removed.
> 
> Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> ---
>  drivers/common/idpf/base/virtchnl2.h | 59 ++++++++++++++--------------
>  1 file changed, 30 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/common/idpf/base/virtchnl2.h b/drivers/common/idpf/base/virtchnl2.h
> index 3285a2b674..b6e28ade97 100644
> --- a/drivers/common/idpf/base/virtchnl2.h
> +++ b/drivers/common/idpf/base/virtchnl2.h
> @@ -239,35 +239,36 @@ enum virtchnl2_cap_rsc {
>  	VIRTCHNL2_CAP_RSC_IPV6_SCTP		= BIT(3),
>  };
>  
> -/* Other capability flags */
> -enum virtchnl2_cap_other {
> -	VIRTCHNL2_CAP_RDMA			= BIT_ULL(0),
> -	VIRTCHNL2_CAP_SRIOV			= BIT_ULL(1),
> -	VIRTCHNL2_CAP_MACFILTER			= BIT_ULL(2),
> -	VIRTCHNL2_CAP_FLOW_DIRECTOR		= BIT_ULL(3),
> -	VIRTCHNL2_CAP_SPLITQ_QSCHED		= BIT_ULL(4),
> -	VIRTCHNL2_CAP_CRC			= BIT_ULL(5),
> -	VIRTCHNL2_CAP_FLOW_STEER		= BIT_ULL(6),
> -	VIRTCHNL2_CAP_WB_ON_ITR			= BIT_ULL(7),
> -	VIRTCHNL2_CAP_PROMISC			= BIT_ULL(8),
> -	VIRTCHNL2_CAP_LINK_SPEED		= BIT_ULL(9),
> -	VIRTCHNL2_CAP_INLINE_IPSEC		= BIT_ULL(10),
> -	VIRTCHNL2_CAP_LARGE_NUM_QUEUES		= BIT_ULL(11),
> -	/* Require additional info */
> -	VIRTCHNL2_CAP_VLAN			= BIT_ULL(12),
> -	VIRTCHNL2_CAP_PTP			= BIT_ULL(13),
> -	VIRTCHNL2_CAP_ADV_RSS			= BIT_ULL(15),
> -	VIRTCHNL2_CAP_FDIR			= BIT_ULL(16),
> -	VIRTCHNL2_CAP_RX_FLEX_DESC		= BIT_ULL(17),
> -	VIRTCHNL2_CAP_PTYPE			= BIT_ULL(18),
> -	VIRTCHNL2_CAP_LOOPBACK			= BIT_ULL(19),
> -	/* Enable miss completion types plus ability to detect a miss completion
> -	 * if a reserved bit is set in a standard completion's tag.
> -	 */
> -	VIRTCHNL2_CAP_MISS_COMPL_TAG		= BIT_ULL(20),
> -	/* This must be the last capability */
> -	VIRTCHNL2_CAP_OEM			= BIT_ULL(63),
> -};
> +/* Other capability flags.
> + * Note: Not using an enum for these flags because these are 64-bit numbers,
> + * which might not fit in an int the enum maps to.
> + */
> +#define VIRTCHNL2_CAP_RDMA			RTE_BIT64(0)
> +#define VIRTCHNL2_CAP_SRIOV			RTE_BIT64(1)
> +#define VIRTCHNL2_CAP_MACFILTER			RTE_BIT64(2)
> +#define VIRTCHNL2_CAP_FLOW_DIRECTOR		RTE_BIT64(3)
> +#define VIRTCHNL2_CAP_SPLITQ_QSCHED		RTE_BIT64(4)
> +#define VIRTCHNL2_CAP_CRC			RTE_BIT64(5)
> +#define VIRTCHNL2_CAP_FLOW_STEER		RTE_BIT64(6)
> +#define VIRTCHNL2_CAP_WB_ON_ITR			RTE_BIT64(7)
> +#define VIRTCHNL2_CAP_PROMISC			RTE_BIT64(8)
> +#define VIRTCHNL2_CAP_LINK_SPEED		RTE_BIT64(9)
> +#define VIRTCHNL2_CAP_INLINE_IPSEC		RTE_BIT64(10)
> +#define VIRTCHNL2_CAP_LARGE_NUM_QUEUES		RTE_BIT64(11)
> +/* Require additional info */
> +#define VIRTCHNL2_CAP_VLAN			RTE_BIT64(12)
> +#define VIRTCHNL2_CAP_PTP			RTE_BIT64(13)
> +#define VIRTCHNL2_CAP_ADV_RSS			RTE_BIT64(15)
> +#define VIRTCHNL2_CAP_FDIR			RTE_BIT64(16)
> +#define VIRTCHNL2_CAP_RX_FLEX_DESC		RTE_BIT64(17)
> +#define VIRTCHNL2_CAP_PTYPE			RTE_BIT64(18)
> +#define VIRTCHNL2_CAP_LOOPBACK			RTE_BIT64(19)
> +/* Enable miss completion types plus ability to detect a miss completion
> + * if a reserved bit is set in a standard completion's tag.
> + */
> +#define VIRTCHNL2_CAP_MISS_COMPL_TAG		RTE_BIT64(20)
> +/* This must be the last capability */
> +#define VIRTCHNL2_CAP_OEM			RTE_BIT64(63)
>  
>  /**
>   * enum virtchnl2_action_types - Available actions for sideband flow steering
> -- 
> 2.47.0.vfs.0.3

Hi,

Please review this patch when you get a chance.

Thanks,

Andre Muezerie
  

Patch

diff --git a/drivers/common/idpf/base/virtchnl2.h b/drivers/common/idpf/base/virtchnl2.h
index 3285a2b674..b6e28ade97 100644
--- a/drivers/common/idpf/base/virtchnl2.h
+++ b/drivers/common/idpf/base/virtchnl2.h
@@ -239,35 +239,36 @@  enum virtchnl2_cap_rsc {
 	VIRTCHNL2_CAP_RSC_IPV6_SCTP		= BIT(3),
 };
 
-/* Other capability flags */
-enum virtchnl2_cap_other {
-	VIRTCHNL2_CAP_RDMA			= BIT_ULL(0),
-	VIRTCHNL2_CAP_SRIOV			= BIT_ULL(1),
-	VIRTCHNL2_CAP_MACFILTER			= BIT_ULL(2),
-	VIRTCHNL2_CAP_FLOW_DIRECTOR		= BIT_ULL(3),
-	VIRTCHNL2_CAP_SPLITQ_QSCHED		= BIT_ULL(4),
-	VIRTCHNL2_CAP_CRC			= BIT_ULL(5),
-	VIRTCHNL2_CAP_FLOW_STEER		= BIT_ULL(6),
-	VIRTCHNL2_CAP_WB_ON_ITR			= BIT_ULL(7),
-	VIRTCHNL2_CAP_PROMISC			= BIT_ULL(8),
-	VIRTCHNL2_CAP_LINK_SPEED		= BIT_ULL(9),
-	VIRTCHNL2_CAP_INLINE_IPSEC		= BIT_ULL(10),
-	VIRTCHNL2_CAP_LARGE_NUM_QUEUES		= BIT_ULL(11),
-	/* Require additional info */
-	VIRTCHNL2_CAP_VLAN			= BIT_ULL(12),
-	VIRTCHNL2_CAP_PTP			= BIT_ULL(13),
-	VIRTCHNL2_CAP_ADV_RSS			= BIT_ULL(15),
-	VIRTCHNL2_CAP_FDIR			= BIT_ULL(16),
-	VIRTCHNL2_CAP_RX_FLEX_DESC		= BIT_ULL(17),
-	VIRTCHNL2_CAP_PTYPE			= BIT_ULL(18),
-	VIRTCHNL2_CAP_LOOPBACK			= BIT_ULL(19),
-	/* Enable miss completion types plus ability to detect a miss completion
-	 * if a reserved bit is set in a standard completion's tag.
-	 */
-	VIRTCHNL2_CAP_MISS_COMPL_TAG		= BIT_ULL(20),
-	/* This must be the last capability */
-	VIRTCHNL2_CAP_OEM			= BIT_ULL(63),
-};
+/* Other capability flags.
+ * Note: Not using an enum for these flags because these are 64-bit numbers,
+ * which might not fit in an int the enum maps to.
+ */
+#define VIRTCHNL2_CAP_RDMA			RTE_BIT64(0)
+#define VIRTCHNL2_CAP_SRIOV			RTE_BIT64(1)
+#define VIRTCHNL2_CAP_MACFILTER			RTE_BIT64(2)
+#define VIRTCHNL2_CAP_FLOW_DIRECTOR		RTE_BIT64(3)
+#define VIRTCHNL2_CAP_SPLITQ_QSCHED		RTE_BIT64(4)
+#define VIRTCHNL2_CAP_CRC			RTE_BIT64(5)
+#define VIRTCHNL2_CAP_FLOW_STEER		RTE_BIT64(6)
+#define VIRTCHNL2_CAP_WB_ON_ITR			RTE_BIT64(7)
+#define VIRTCHNL2_CAP_PROMISC			RTE_BIT64(8)
+#define VIRTCHNL2_CAP_LINK_SPEED		RTE_BIT64(9)
+#define VIRTCHNL2_CAP_INLINE_IPSEC		RTE_BIT64(10)
+#define VIRTCHNL2_CAP_LARGE_NUM_QUEUES		RTE_BIT64(11)
+/* Require additional info */
+#define VIRTCHNL2_CAP_VLAN			RTE_BIT64(12)
+#define VIRTCHNL2_CAP_PTP			RTE_BIT64(13)
+#define VIRTCHNL2_CAP_ADV_RSS			RTE_BIT64(15)
+#define VIRTCHNL2_CAP_FDIR			RTE_BIT64(16)
+#define VIRTCHNL2_CAP_RX_FLEX_DESC		RTE_BIT64(17)
+#define VIRTCHNL2_CAP_PTYPE			RTE_BIT64(18)
+#define VIRTCHNL2_CAP_LOOPBACK			RTE_BIT64(19)
+/* Enable miss completion types plus ability to detect a miss completion
+ * if a reserved bit is set in a standard completion's tag.
+ */
+#define VIRTCHNL2_CAP_MISS_COMPL_TAG		RTE_BIT64(20)
+/* This must be the last capability */
+#define VIRTCHNL2_CAP_OEM			RTE_BIT64(63)
 
 /**
  * enum virtchnl2_action_types - Available actions for sideband flow steering