lwip2 include ip4_addr.h has wrong defines?

[gitpeut]

Basic Infos

• This issue complies with the issue POLICY doc.
• I have read the documentation at readthedocs and the issue is not addressed there.
• I have tested that the issue is present in current master branch (aka latest git).
• I have searched the issue tracker for a similar issue.
• If there is a stack dump, I have decoded it.
• I have filled out all fields below.

Platform

• Hardware: Wemos mini
• Core Version: Arduino release 2.4.1 - SDK
• Development Env: [Arduino IDE]
• Operating System: [Windows]

Settings in IDE

• Module: [Wemos D1 mini r2]
• Flash Mode: [qio|dio|other]
• Flash Size: [4MB/1MB]
• lwip Variant: [v1.4|v2 see text below]
• Reset Method: [N/A]
• Flash Frequency: [40Mhz]
• CPU Frequency: [80Mhz]
• Upload Using: [SERIAL]
• Upload Speed: [115200] (serial upload only)

Problem Description

Attached reproducer using lwip2 ( both low memory and high bandwidth) will not compile.
Code compiles using lwip 1.4. IP2STR was used in some code I was re-using.
Reason seems to be wrong define of ip4_addr* in lwip2/include/ip4_addr.h.
Replacing defines with the ones from lwip 1.4 successfully compiles and works.

I am not sure if these defines in lwip2 are wrong by design or if this is a bug. As the 1.4 lwip
version seems to work I suspect the latter.

Reproducer:

ip4_addr_reproducer.ino.txt

Compile Errors:

compile_errors.txt

Fix?
Below the changed lines in lwip2/include/ip4_addr.h. Commented out the original defines that would not compile and added the same define from the lwip/include/ip4_addr.h.

//******************* WRONG ******************
/* Get one byte from the 4-byte address /
//#define ip4_addr1(ipaddr) (((const u8_t)(&(ipaddr)->addr))[0])
//#define ip4_addr2(ipaddr) (((const u8_t*)(&(ipaddr)->addr))[1])
//#define ip4_addr3(ipaddr) (((const u8_t*)(&(ipaddr)->addr))[2])
//#define ip4_addr4(ipaddr) (((const u8_t*)(&(ipaddr)->addr))[3])
//*END WRONG ****************
//RIGHT
#define ip4_addr1(ipaddr) (((u8_t)(ipaddr))[0])
#define ip4_addr2(ipaddr) (((u8_t)(ipaddr))[1])
#define ip4_addr3(ipaddr) (((u8_t)(ipaddr))[2])
#define ip4_addr4(ipaddr) (((u8_t)(ipaddr))[3])
//END RIGHT

[MauiJerry]

The given fix is incorrect on a couple points:

1. The RIGHT version should use (u8_t*) - a pointer to u8_t
   rather than a (u8_t) itself.
2. There needs to be a Close Comment */ at the end of END WRONG line

Adding these to the file
$(HOME)/Library/Arduino15/packages/esp8266/hardware/esp8266/2.4.1/tools/sdk/lwip2/include/lwip/ip_addr.h
allowed my application (using painlessmesh library) to compile.

[gitpeut]

1. Maybe. Not my application, though. Funny how this changed then between lwip versions and
   previous non-lwip esp8266 versions. Hopefully the gurus will decide wisely.
2. No. // at the start of the line make this whole line a comment, never mind the asterisks

[d-a-v]

ip4_addrN() macros are lwIP's source code and not to blame.
IP2STR() is espressif's and the right way to use it is

ip_addr_t ip = { WiFi.localIP() };
...printf(..., IP2STR(&ip));

or

ip_addr_t ip;
ip.addr = WiFi.localIP();
...printf(..., IP2STR(&ip));

The only "official" use of IP2STR() I found is in examples (link).
Although your code is working well with lwIP-v1.4, it does not with lwIP-v2 because the lwIP folks adapted their structures to be the best possible compatible whether IPv6 is enabled or not.
Note that the type ip_addr_t is no longer officially existing in lwIP-v2, and is still provided with our core's lwip2 for backward compatibility.

edit: fixed the second example