Remove warnings when NDEBUG build option used

[earlephilhower]

When building using the new NDEBUG option recently added, the assert() macro is defined to nothing. This leaves a few variables unused in the WiFi stack causing compiler warnings. Add in empty casts to remove these warnings. Does not affect actual assert use when NDEBUG is not defined.

[igrr]

This doesn't look right to me. I think the definition of assert must be changed to something like

#ifdef NDEBUG           /* required by ANSI standard */
# define assert(__e) ((void) sizeof(__e))
#else
# define assert(__e) ((__e) ? (void)0 : __assert_func (PSTR(__FILE__), __LINE__, \
						       __ASSERT_FUNC, #__e))
// etc

in which case these (void) casts will not be required.

Edit: also, perhaps we can put #__e into PROGMEM as well?

[earlephilhower]

Howdy,

@igrr The existing macro has the same behavior as my native x86_64 gcc 5.4, which throws the warning on unused params when -NDEBUG is defined:

earle@server$ cat /tmp/a.cpp 
#include <stdio.h>
#include <string.h>
#include <assert.h>

void testassert(size_t a, const char *b)
{
	assert(a == strlen(b));
}

int main(int argc, char **argv)
{
	(void) argc;
	(void) argv;
	testassert(12, "test");
}

earle@server$ gcc -Wall -Wunused-parameter -DNDEBUG -o /tmp/a /tmp/a.cpp
/tmp/a.cpp:5:24: warning: unused parameter ‘a’ [-Wunused-parameter]
 void testassert(size_t a, const char *b)
                        ^
/tmp/a.cpp:5:39: warning: unused parameter ‘b’ [-Wunused-parameter]
 void testassert(size_t a, const char *b)
                                       ^
earle@server$ gcc -Wall -Wunused-parameter -o /tmp/a /tmp/a.cpp

earle@server$ gcc --version
gcc (Ubuntu 5.4.0-6ubuntu1~16.04.5) 5.4.0 20160609
Copyright (C) 2015 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Do you really want to change the assert() behavior just for the ESP8266 compiler? I'm fine doing the change and updating the PR to only change GCC SDK files, but I have a sinking sensation that this would cause weird stuff in other cases.

I could have sworn I tried and failed to get #__e into PMEM when I submitted the original asser() patch a while back, but after checking right now it seems to compile fine. I'll test the actual assert_func() to make sure it runs OK and then update the patch.

Given the standard gcc behavior above, what're your thoughts? Still want to update the assert() macro instead of the 2 spots (in the entire IDE/SDK/Arduino std. libs) where there's a problem and values actually aren't checked w/o the assert?

Thx
-EFP3

[earlephilhower]

It turns out that the __e bit is completely ignored in the current __assert_func! It's a simple addition to add it in and update the panic() stuff to detect an assert and print the condition as well as file/line.

...snip...

void __assert_func(const char *file, int line, const char *func, const char *what) {
    (void) what;
    s_panic_file = file;
    s_panic_line = line;
    s_panic_func = func;
    gdb_do_break();
}

[igrr]

Do you really want to change the assert() behavior just for the ESP8266 compiler?

We have been using such method in esp32 sdk for a while and have not seen any odd behavior.
At the same time, i don't mind keeping it the same as in the original newlib. We just need to build -DNDEBUG version in CI with -Werror to prevent new warnings from being introduced.

[earlephilhower]

Closing this due to some GIT troubles. Cleaned up, warning fix only (no core changes) in #4196.