Date: Wed, 19 Sep 2012 11:19:15 +0200 From: Erik Cederstrand <erik@cederstrand.dk> To: FreeBSD Hackers <hackers@FreeBSD.org> Subject: NDEBUG and assert() Message-ID: <63FCC95F-7D73-423D-892D-DB5D75BCEBE7@cederstrand.dk>
next in thread | raw e-mail | index | archive | help
Hello, I'm sifting through the Clang Analyzer reports and decided to take a = look at this one: = http://scan.freebsd.your.org/freebsd-head/lib.ncurses.menu/2012-09-16-amd6= 4/report-3vc5Zu.html#Path5 If you mouseover the assert() in line 236, the problem is that assert() = resolves to "(void)0" so the analyzer doesn't know that item is non-null = later on. Since this is contrib code, my best bet to fix it is to enable = assert()'s in our build code for ncurses. That way, users will get an = assertion error describing the real problem, instead of a null pointer = dereference or garbage data. My assumption here is that nurses is not = performance-sensitive so an extra assert() here and there is OK. lib/ncurses/config.mk has "CFLAGS+=3D -DNDEBUG". If I remove this, = contrib/ncurses/include/MKncurses_def.sh just makes sure NDEBUG is set = to 0 instead of 1. However, include/assert.h just tests for "ifdef = NDEBUG" to decide whether to actually assert or just return (void)0, so = it also returns (void)0 when NDEBUG=3D0. This was obviously not what the = ncurses author intended. By changing "ifdef NDEBUG" to "if NDEBUG" the = value of NDEBUG is evaluated to true or false. The below below patch will let the analyzer reason correctly about the = code, and removes the report mentioned above (and a handful others in = ncurses). It doesn't touch contrib code, but I'm not happy about = changing include/assert.h since it's used so many other places. Any = other ideas for how to best solve this? Kind regards, Erik Cederstrand Index: lib/ncurses/ncurses/ncurses_cfg.h =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- lib/ncurses/ncurses/ncurses_cfg.h (revision 240638) +++ lib/ncurses/ncurses/ncurses_cfg.h (working copy) @@ -145,7 +145,7 @@ #define NCURSES_NO_PADDING 1 #define NCURSES_PATHSEP ':' #define NCURSES_VERSION_STRING "5.7.20081102" -#define NDEBUG 1 +#define NDEBUG 0 #define RETSIGTYPE void #define SIG_ATOMIC_T volatile sig_atomic_t #define SIZEOF_SIGNED_CHAR 1 Index: lib/ncurses/config.mk =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- lib/ncurses/config.mk (revision 240638) +++ lib/ncurses/config.mk (working copy) @@ -27,8 +27,6 @@ =20 CFLAGS+=3D -Wall =20 -CFLAGS+=3D -DNDEBUG - CFLAGS+=3D -DHAVE_CONFIG_H =20 # everyone needs this Index: include/assert.h =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- include/assert.h (revision 240638) +++ include/assert.h (working copy) @@ -45,7 +45,7 @@ #undef assert #undef _assert =20 -#ifdef NDEBUG +#if NDEBUG #define assert(e) ((void)0) #define _assert(e) ((void)0) #else
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?63FCC95F-7D73-423D-892D-DB5D75BCEBE7>