diff mbox series

[v3,4/5] net/intel: use portable version of __builtin_add_overflow

Message ID 1741192690-26243-5-git-send-email-andremue@linux.microsoft.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series add portable version of __builtin_add_overflow |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Andre Muezerie March 5, 2025, 4:38 p.m. UTC 
__builtin_add_overflow is gcc specific. It should be replaced with
a portable version that can also be used with other compilers.

Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
---
 drivers/net/intel/ice/base/ice_nvm.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

Comments

Bruce Richardson March 5, 2025, 4:52 p.m. UTC | #1 
On Wed, Mar 05, 2025 at 08:38:09AM -0800, Andre Muezerie wrote:
> __builtin_add_overflow is gcc specific. It should be replaced with
> a portable version that can also be used with other compilers.
> 
> Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> ---
>  drivers/net/intel/ice/base/ice_nvm.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/intel/ice/base/ice_nvm.c b/drivers/net/intel/ice/base/ice_nvm.c
> index 56c6c96a95..1002a6b59f 100644
> --- a/drivers/net/intel/ice/base/ice_nvm.c
> +++ b/drivers/net/intel/ice/base/ice_nvm.c
> @@ -3,6 +3,7 @@
>   */
>  
>  #include "ice_common.h"
> +#include <rte_math.h>
>  
>  #define GL_MNG_DEF_DEVID 0x000B611C
>  
> @@ -469,8 +470,6 @@ int ice_read_sr_word(struct ice_hw *hw, u16 offset, u16 *data)
>  	return status;
>  }
>  
> -#define check_add_overflow __builtin_add_overflow
> -

Rather than modifying the base code, can you instead add a #define to the
osdep.h file in the base directory to alias the new function to
__builtin_add_overflow for MSVC. The other files (other than osdep.h) in
the base directory, come from a common/shared source that is not DPDK
specific, so we try to avoid modifying them where possible.

/Bruce
Andre Muezerie March 11, 2025, 6:39 p.m. UTC | #2 
On Wed, Mar 05, 2025 at 04:52:09PM +0000, Bruce Richardson wrote:
> On Wed, Mar 05, 2025 at 08:38:09AM -0800, Andre Muezerie wrote:
> > __builtin_add_overflow is gcc specific. It should be replaced with
> > a portable version that can also be used with other compilers.
> > 
> > Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> > ---
> >  drivers/net/intel/ice/base/ice_nvm.c | 9 ++++-----
> >  1 file changed, 4 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/net/intel/ice/base/ice_nvm.c b/drivers/net/intel/ice/base/ice_nvm.c
> > index 56c6c96a95..1002a6b59f 100644
> > --- a/drivers/net/intel/ice/base/ice_nvm.c
> > +++ b/drivers/net/intel/ice/base/ice_nvm.c
> > @@ -3,6 +3,7 @@
> >   */
> >  
> >  #include "ice_common.h"
> > +#include <rte_math.h>
> >  
> >  #define GL_MNG_DEF_DEVID 0x000B611C
> >  
> > @@ -469,8 +470,6 @@ int ice_read_sr_word(struct ice_hw *hw, u16 offset, u16 *data)
> >  	return status;
> >  }
> >  
> > -#define check_add_overflow __builtin_add_overflow
> > -
> 
> Rather than modifying the base code, can you instead add a #define to the
> osdep.h file in the base directory to alias the new function to
> __builtin_add_overflow for MSVC. The other files (other than osdep.h) in
> the base directory, come from a common/shared source that is not DPDK
> specific, so we try to avoid modifying them where possible.
> 
> /Bruce

The problem is that the file below defines __builtin_add_overflow itself, so
it does not look like we can completely avoid changing this file.
I can still make the change you asked for, but the patch will have to remove
that define from the file under base.

drivers\net\intel\ice\base\ice_nvm.c
--
Andre Muezerie
diff mbox series

Patch

diff --git a/drivers/net/intel/ice/base/ice_nvm.c b/drivers/net/intel/ice/base/ice_nvm.c
index 56c6c96a95..1002a6b59f 100644
--- a/drivers/net/intel/ice/base/ice_nvm.c
+++ b/drivers/net/intel/ice/base/ice_nvm.c
@@ -3,6 +3,7 @@ 
  */
 
 #include "ice_common.h"
+#include <rte_math.h>
 
 #define GL_MNG_DEF_DEVID 0x000B611C
 
@@ -469,8 +470,6 @@  int ice_read_sr_word(struct ice_hw *hw, u16 offset, u16 *data)
 	return status;
 }
 
-#define check_add_overflow __builtin_add_overflow
-
 /**
  * ice_get_pfa_module_tlv - Reads sub module TLV from NVM PFA
  * @hw: pointer to hardware structure
@@ -500,7 +499,7 @@  ice_get_pfa_module_tlv(struct ice_hw *hw, u16 *module_tlv, u16 *module_tlv_len,
 		return status;
 	}
 
-	if (check_add_overflow(pfa_ptr, (u16)(pfa_len - 1), &max_tlv)) {
+	if (rte_add_overflow(pfa_ptr, (u16)(pfa_len - 1), &max_tlv)) {
 		ice_debug(hw, ICE_DBG_INIT, "PFA starts at offset %u. PFA length of %u caused 16-bit arithmetic overflow.\n",
 				  pfa_ptr, pfa_len);
 		return ICE_ERR_INVAL_SIZE;
@@ -541,8 +540,8 @@  ice_get_pfa_module_tlv(struct ice_hw *hw, u16 *module_tlv, u16 *module_tlv_len,
 			return ICE_ERR_INVAL_SIZE;
 		}
 
-		if (check_add_overflow(next_tlv, (u16)2, &next_tlv) ||
-		    check_add_overflow(next_tlv, tlv_len, &next_tlv)) {
+		if (rte_add_overflow(next_tlv, (u16)2, &next_tlv) ||
+		    rte_add_overflow(next_tlv, tlv_len, &next_tlv)) {
 			ice_debug(hw, ICE_DBG_INIT, "TLV of type %u and length 0x%04x caused 16-bit arithmetic overflow. The PFA starts at 0x%04x and has length of 0x%04x\n",
 					  tlv_sub_module_type, tlv_len, pfa_ptr, pfa_len);
 			return ICE_ERR_INVAL_SIZE;