Skip to content

assert: change the do-while of assert to a conditional expression#11734

Merged
acassis merged 3 commits into
apache:masterfrom
Gary-Hobson:assert
Feb 21, 2024
Merged

assert: change the do-while of assert to a conditional expression#11734
acassis merged 3 commits into
apache:masterfrom
Gary-Hobson:assert

Conversation

@Gary-Hobson

Copy link
Copy Markdown
Contributor

Summary

assert: change the do-while of assert to a conditional expression

According to the standard definition, assert should return a void expression
https://pubs.opengroup.org/onlinepubs/007904875/basedefs/assert.h.html

This patch involves two changes:

If you define the NDEBUG macro, assert does not use any parameters and directly returns a void expression.
assert should return a void expression and cannot use do-while statements.
If the following code , a compilation error will occur.

Impact

Testing

sim

According to the standard definition, assert should return a void expression
https://pubs.opengroup.org/onlinepubs/007904875/basedefs/assert.h.html

This patch involves two changes:

If you define the NDEBUG macro, assert does not use any parameters and directly returns a void expression.
assert should return a void expression and cannot use do-while statements.
If the following code , a compilation error will occur.

Signed-off-by: yinshengkai <yinshengkai@xiaomi.com>
The implementation of assert in linux is as follows:
define assert(expr)        (__ASSERT_VOID_CAST (0))

After defining NDEBUG in sqlite, some variables will be undefined,
and referencing variables in assert will cause compilation errors.

sqlite3.c:28729:23: error: ‘mutexIsInit’ undeclared (first use in this function)
28729 |   assert( GLOBAL(int, mutexIsInit) );

Signed-off-by: yinshengkai <yinshengkai@xiaomi.com>
These variables will trigger variable 'ret' set but not used warnings due to different configurations.

Signed-off-by: yinshengkai <yinshengkai@xiaomi.com>
@acassis acassis merged commit d2d93ba into apache:master Feb 21, 2024
@masayuki2009

Copy link
Copy Markdown
Contributor

I encountered a build error with qemu-armv8a:netnsh_smp_hv

CC:  common/arm64_arch_timer.c
aarch64-none-elf-gcc -c -D_LDBL_EQ_DBL -fno-common -Wall -Wstrict-prototypes -Wshadow -Wundef -Werror -Wno-attributes -Wno-unknown-pragmas -Wno-psabi -Os -fno-strict-aliasing -fomit-frame-pointer -ffunction-sections -fdata-sections -g -march=armv8-a -mcpu=cortex-a53 -isystem /mnt/m2ssd/opensource/RTOS/nuttx/nuttx/include -D__NuttX__ -DNDEBUG -D__KERNEL__  -pipe -I /mnt/m2ssd/opensource/RTOS/nuttx/nuttx/arch/arm64/src/chip -I /mnt/m2ssd/opensource/RTOS/nuttx/nuttx/arch/arm64/src/common -I /mnt/m2ssd/opensource/RTOS/nuttx/nuttx/sched    common/arm64_arch_timer.c -o  arm64_arch_timer.o
common/arm64_arch_timer.c: In function 'arm64_tick_cancel':
common/arm64_arch_timer.c:209:37: error: unused variable 'priv' [-Werror=unused-variable]
  209 |   struct arm64_oneshot_lowerhalf_s *priv =
      |                                     ^~~~
cc1: all warnings being treated as errors

@masayuki2009

masayuki2009 commented Feb 22, 2024

Copy link
Copy Markdown
Contributor

rv-virt:nsh

riscv-none-elf-gcc -c -fno-common -Wall -Wstrict-prototypes -Wshadow -Wundef -Wno-attributes -Wno-unknown-pragmas -Wno-psabi -Os -fno-strict-aliasing -fomit-frame-pointer -ffunction-sections -fdata-sections -g -march=rv32imafdc_zicsr_zifencei -mabi=ilp32d -isystem /mnt/m2ssd/opensource/gh_masayuki2009/apache-nuttx-tmp/nuttx/include -D__NuttX__ -DNDEBUG  -pipe -I /mnt/m2ssd/opensource/gh_masayuki2009/apache-nuttx-tmp/nuttx/libs/libc    machine/risc-v/arch_elf.c -o  bin/arch_elf.o
machine/risc-v/arch_elf.c: In function 'up_relocateadd':
machine/risc-v/arch_elf.c:481:20: warning: variable 'insn' set but not used [-Wunused-but-set-variable]
  481 |           uint32_t insn;
      |                    ^~~~

esp32-devkitc:smp

xtensa-esp32-elf-gcc -c -fno-common -Wall -Wstrict-prototypes -Wshadow -Wundef -Wno-attributes -Wno-unknown-pragmas -Wno-psabi -Os -fno-strict-aliasing -fomit-frame-pointer -ffunction-sections -fdata-sections -g -mlongcalls -isystem /mnt/m2ssd/opensource/gh_masayuki2009/apache-nuttx-tmp/nuttx/include -D__NuttX__ -DNDEBUG -D__KERNEL__  -pipe -Wno-undef -Wno-unused-variable -I /mnt/m2ssd/opensource/gh_masayuki2009/apache-nuttx-tmp/nuttx/arch/xtensa/src/chip -I /mnt/m2ssd/opensource/gh_masayuki2009/apache-nuttx-tmp/nuttx/arch/xtensa/src/common -I /mnt/m2ssd/opensource/gh_masayuki2009/apache-nuttx-tmp/nuttx/arch/xtensa/src/lx6 -I /mnt/m2ssd/opensource/gh_masayuki2009/apache-nuttx-tmp/nuttx/sched    chip/esp32_irq.c -o  esp32_irq.o
chip/esp32_irq.c: In function 'xtensa_attach_fromcpu1_interrupt':
chip/esp32_irq.c:238:7: warning: variable 'cpuint' set but not used [-Wunused-but-set-variable]
  238 |   int cpuint;
      |       ^~~~~~

@masayuki2009

Copy link
Copy Markdown
Contributor

I think we should revert this PR and fix all warnings before merging this PR.

@xiaoxiang781216

Copy link
Copy Markdown
Contributor

@acassis the ci doesn't pass yet:(.

@Gary-Hobson

Copy link
Copy Markdown
Contributor Author

@masayuki2009 I submitted a repair patch, which involves two repositories
apache/nuttx-apps#2302
#11742

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants