[dpdk-dev] lib/librte_eal: change type of tmp

Message ID 152690080971.12755.9675572999031608037.stgit@localhost.localdomain (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK

Commit Message

Andy Green May 21, 2018, 11:06 a.m. UTC
  /projects/lagopus/src/dpdk/build/include/rte_memcpy.h:
793:2: note: in expansion of macro 'MOVEUNALIGNED_LEFT47'
  MOVEUNALIGNED_LEFT47(dst, src, n, srcofs);
  ^~~~~~~~~~~~~~~~~~~~
/projects/lagopus/src/dpdk/build/include/rte_memcpy.h:
649:51: warning: conversion from 'size_t' {aka 'long
unsigned int'} to 'int' may change value [-Wconversion]
     case 0x0B: MOVEUNALIGNED_LEFT47_IMM(dst, src,
n, 0x0B); break;    \
                                                   ^
/projects/lagopus/src/dpdk/build/include/rte_memcpy.h:
616:15: note: in definition of macro 'MOVEUNALIGNED_LEFT47_IMM'
         tmp = len;                                                                                          \
               ^~~
/projects/lagopus/src/dpdk/build/include/rte_memcpy.h:
793:2: note: in expansion of macro 'MOVEUNALIGNED_LEFT47'
  MOVEUNALIGNED_LEFT47(dst, src, n, srcofs);
  ^~~~~~~~~~~~~~~~~~~~
/projects/lagopus/src/dpdk/build/include/rte_memcpy.h:
618:13: warning: conversion to 'size_t' {aka 'long
unsigned int'} from 'int' may change the sign of the
result [-Wsign-conversion]
         tmp -= len;                                                                                         \
             ^~
/projects/lagopus/src/dpdk/build/include/rte_memcpy.h:
649:16: note: in expansion of macro 'MOVEUNALIGNED_LEFT47_IMM'
     case 0x0B: MOVEUNALIGNED_LEFT47_IMM(dst, src,
n, 0x0B); break;    \
                ^~~~~~~~~~~~~~~~~~~~~~~~
/projects/lagopus/src/dpdk/build/include/rte_memcpy.h:
793:2: note: in expansion of macro 'MOVEUNALIGNED_LEFT47'
  MOVEUNALIGNED_LEFT47(dst, src, n, srcofs);
  ^~~~~~~~~~~~~~~~~~~~

    /projects/lagopus/src/dpdk/build/include/rte_memcpy.h:
    618:13: warning: conversion to 'size_t' {aka 'long
    unsigned int'} from 'int' may change the sign of the
    result [-Wsign-conversion]
             tmp -= len;
                 ^~

We can eliminate the problems by setting the type of tmp to
size_t in the first place.

After a suggestion by Bruce Richardson

Signed-off-by: Andy Green <andy@warmcat.com>
Fixes: d35cc1fe6a ("eal/x86: revert select optimized memcpy at run-time")
---
 .../common/include/arch/x86/rte_memcpy.h           |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Bruce Richardson May 21, 2018, 12:07 p.m. UTC | #1
On Mon, May 21, 2018 at 07:06:49PM +0800, Andy Green wrote:
> /projects/lagopus/src/dpdk/build/include/rte_memcpy.h:
> 793:2: note: in expansion of macro 'MOVEUNALIGNED_LEFT47'
>   MOVEUNALIGNED_LEFT47(dst, src, n, srcofs);
>   ^~~~~~~~~~~~~~~~~~~~
> /projects/lagopus/src/dpdk/build/include/rte_memcpy.h:
> 649:51: warning: conversion from 'size_t' {aka 'long
> unsigned int'} to 'int' may change value [-Wconversion]
>      case 0x0B: MOVEUNALIGNED_LEFT47_IMM(dst, src,
> n, 0x0B); break;    \
>                                                    ^
> /projects/lagopus/src/dpdk/build/include/rte_memcpy.h:
> 616:15: note: in definition of macro 'MOVEUNALIGNED_LEFT47_IMM'
>          tmp = len;                                                                                          \
>                ^~~
> /projects/lagopus/src/dpdk/build/include/rte_memcpy.h:
> 793:2: note: in expansion of macro 'MOVEUNALIGNED_LEFT47'
>   MOVEUNALIGNED_LEFT47(dst, src, n, srcofs);
>   ^~~~~~~~~~~~~~~~~~~~
> /projects/lagopus/src/dpdk/build/include/rte_memcpy.h:
> 618:13: warning: conversion to 'size_t' {aka 'long
> unsigned int'} from 'int' may change the sign of the
> result [-Wsign-conversion]
>          tmp -= len;                                                                                         \
>              ^~
> /projects/lagopus/src/dpdk/build/include/rte_memcpy.h:
> 649:16: note: in expansion of macro 'MOVEUNALIGNED_LEFT47_IMM'
>      case 0x0B: MOVEUNALIGNED_LEFT47_IMM(dst, src,
> n, 0x0B); break;    \
>                 ^~~~~~~~~~~~~~~~~~~~~~~~
> /projects/lagopus/src/dpdk/build/include/rte_memcpy.h:
> 793:2: note: in expansion of macro 'MOVEUNALIGNED_LEFT47'
>   MOVEUNALIGNED_LEFT47(dst, src, n, srcofs);
>   ^~~~~~~~~~~~~~~~~~~~
> 
>     /projects/lagopus/src/dpdk/build/include/rte_memcpy.h:
>     618:13: warning: conversion to 'size_t' {aka 'long
>     unsigned int'} from 'int' may change the sign of the
>     result [-Wsign-conversion]
>              tmp -= len;
>                  ^~
> 
> We can eliminate the problems by setting the type of tmp to
> size_t in the first place.
> 
> After a suggestion by Bruce Richardson
> 
> Signed-off-by: Andy Green <andy@warmcat.com>
> Fixes: d35cc1fe6a ("eal/x86: revert select optimized memcpy at run-time")
> ---

Thanks, this looks a better fix than loads of type-casting.

Acked-by: Bruce Richardson <bruce.richardson@intel.com>
  

Patch

diff --git a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
index 5ead68ab2..7b758094d 100644
--- a/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
+++ b/lib/librte_eal/common/include/arch/x86/rte_memcpy.h
@@ -574,7 +574,7 @@  rte_mov256(uint8_t *dst, const uint8_t *src)
  */
 #define MOVEUNALIGNED_LEFT47_IMM(dst, src, len, offset)                                                     \
 __extension__ ({                                                                                            \
-    int tmp;                                                                                                \
+    size_t tmp;                                                                                                \
     while (len >= 128 + 16 - offset) {                                                                      \
         xmm0 = _mm_loadu_si128((const __m128i *)((const uint8_t *)src - offset + 0 * 16));                  \
         len -= 128;                                                                                         \