You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/10/29 11:54:23 UTC
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #7468: Fix warning reported by clang
pkarashchenko commented on code in PR #7468:
URL: https://github.com/apache/incubator-nuttx/pull/7468#discussion_r1008686769
##########
arch/arm/src/gd32f4/gd32f4xx_syscfg.c:
##########
@@ -107,7 +107,7 @@ void gd32_syscfg_exmc_swap_config(uint32_t syscfg_exmc_swap)
void gd32_syscfg_exti_line_config(uint8_t exti_port, uint8_t exti_pin)
{
uint32_t regval;
- uint32_t regaddr;
+ uint32_t regaddr = 0;
Review Comment:
Let's also add `DEBUGASSERT(false);` in the `default` case?
##########
include/endian.h:
##########
@@ -52,36 +52,36 @@
/* Common byte swapping macros */
#ifdef CONFIG_HAVE_BUILTIN_BSWAP16
-# define __swap_uint16 __builtin_bswap16
+# define __swap_uint16(n) ((uint16_t)__builtin_bswap16(n))
#else
# define __swap_uint16(n) \
- (uint16_t)(((((uint16_t)(n)) & 0x00ff) << 8) | \
- ((((uint16_t)(n)) >> 8) & 0x00ff))
+ ((uint16_t)(((((uint16_t)(n)) & 0x00ff) << 8) | \
+ ((((uint16_t)(n)) >> 8) & 0x00ff)))
#endif
#ifdef CONFIG_HAVE_BUILTIN_BSWAP32
-# define __swap_uint32 __builtin_bswap32
+# define __swap_uint32(n) ((uint32_t)__builtin_bswap32(n))
#else
# define __swap_uint32(n) \
- (uint32_t)(((((uint32_t)(n)) & 0x000000ffUL) << 24) | \
- ((((uint32_t)(n)) & 0x0000ff00UL) << 8) | \
- ((((uint32_t)(n)) & 0x00ff0000UL) >> 8) | \
- ((((uint32_t)(n)) & 0xff000000UL) >> 24))
+ ((uint32_t)(((((uint32_t)(n)) & 0x000000ffUL) << 24) | \
+ ((((uint32_t)(n)) & 0x0000ff00UL) << 8) | \
+ ((((uint32_t)(n)) & 0x00ff0000UL) >> 8) | \
+ ((((uint32_t)(n)) & 0xff000000UL) >> 24)))
#endif
#ifdef CONFIG_HAVE_LONG_LONG
# ifdef CONFIG_HAVE_BUILTIN_BSWAP64
-# define __swap_uint64 __builtin_bswap64
+# define __swap_uint64(n) ((uint64_t)__builtin_bswap64(n))
Review Comment:
Ditto
##########
arch/arm/src/efm32/efm32_clockconfig.c:
##########
@@ -718,7 +689,7 @@ static inline uint32_t efm32_lfaclk_config(uint32_t lfaclksel, bool ulfrco,
static inline uint32_t efm32_lfbclk_config(uint32_t lfbclksel, bool ulfrco,
uint32_t hfcoreclk)
{
- uint32_t lfbclk;
+ uint32_t lfbclk = 0;
Review Comment:
Maybe better to add assignment to LFRCO case below? Is `BOARD_LFRCO_FREQUENCY` not defined?
##########
include/endian.h:
##########
@@ -52,36 +52,36 @@
/* Common byte swapping macros */
#ifdef CONFIG_HAVE_BUILTIN_BSWAP16
-# define __swap_uint16 __builtin_bswap16
+# define __swap_uint16(n) ((uint16_t)__builtin_bswap16(n))
#else
# define __swap_uint16(n) \
- (uint16_t)(((((uint16_t)(n)) & 0x00ff) << 8) | \
- ((((uint16_t)(n)) >> 8) & 0x00ff))
+ ((uint16_t)(((((uint16_t)(n)) & 0x00ff) << 8) | \
+ ((((uint16_t)(n)) >> 8) & 0x00ff)))
#endif
#ifdef CONFIG_HAVE_BUILTIN_BSWAP32
-# define __swap_uint32 __builtin_bswap32
+# define __swap_uint32(n) ((uint32_t)__builtin_bswap32(n))
Review Comment:
Ditto
##########
include/endian.h:
##########
@@ -52,36 +52,36 @@
/* Common byte swapping macros */
#ifdef CONFIG_HAVE_BUILTIN_BSWAP16
-# define __swap_uint16 __builtin_bswap16
+# define __swap_uint16(n) ((uint16_t)__builtin_bswap16(n))
Review Comment:
This change is super strange. What is the warning here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org