Re: [lwip-devel] Summary of changes after porting to Linux and Hurd

[goldsimon]

Am 7. Juli 2019 09:56:06 MESZ schrieb "Joan Lledó" <address@hidden>:
>Hello lwip,
>
>I recently updated the lwip debian package for the Hurd and Linux to
>2.1.2, and during
>the process I found some issues I'd like to fix, if you agree:
>
>1.- include/lwip/if_api.h: IF_NAMESIZE could be defined by external
>socket headers, so
>    it should be defined only if it's not defined before.
>
>2.- contrib/ports/unix/lib/lwipopts.h: sys_check_core_locking() and
>pals are declared
>in lwipopts.h, is this the proper place? why aren't they declared in
>sys_arch.h?
>
>3.- contrib/ports/unix/port/sys_arch.c: sys_arch_sem_wait() and
>cond_wait():
>- On the Hurd, pthread_cond_wait() and pthread_cond_timedwait() are
>awakened when
>glibc cancels the RPC that called them. I wrote some code to handle
>this case.
>  - contrib/ports/unix/port/include/arch/sys_arch.h: I defined
>SYS_ARCH_INTR to be
>returned by sys_arch_sem_wait() when pthread_cond_wait() was awakened
>on the Hurd.
>
>4.- contrib/ports/unix/check/CMakeLists.txt: Add support for the Hurd:
>  - Line 7: add AND NOT CMAKE_SYSTEM_NAME STREQUAL GNU
>- Line 12: set(LWIP_USE_SANITIZERS true) only if (NOT CMAKE_SYSTEM_NAME
>STREQUAL GNU)
>  - Line 48: add OR CMAKE_SYSTEM_NAME STREQUAL GNU
>
>5.- apps/tftp/tftp.c: recv() could be declared by external socket
>headers. Rename it to
>    tftp_recv()
>  - Lines 243 and 471
>
>6.- test/unit/api/test_sockets.c:
>- Line 156: write() could be declared by external socket headers. Call
>lwip_write()
>    instead.
>- Line 333: it expects fcntl() to return 6, but O_RDWR could have
>another value if
>    external socket headers are present. Replace 6 by O_RDWR.
>- In Linux, when using the system's sockets headers, struct
>msghdr->msg_iovlen is not
>    defined as int.
>    - POSIX[1] says it must be int
>    - My solution was:
>      - add a typedef int msg_iovlen_t; in lwip's sockets.h
>- declare msg_iovlen as msg_iovlen_t inside struct msghdr in lwip's
>sockets.h
>- let the user typedef msg_iovlen_t as the type they need for their
>system
>      - Line 253: declare i as msg_iovlen_t i;
>
>7.- src/api/tcpip.c:
>- New function tcpip_callback_wait() to call a function inside the
>tcpip thread
>    and block the calling thread until the callback finishes.
>  - I already wrote a patch[2] for this and am waiting for its review.

I'll see to find the time to review it, sorry for the delay.

>8.- src/apps/lwiperf/lwiperf.c:
>  - Line 773: Calls htonl() but lwip/inet.h is not included
>- There are other apps and unit tests where htons and pals are being
>called w/o
>    including lwip/inet.h
>
>9.- src/include/lwip/sockets.h:
>- Lines 536 and following lines until the end: I think all definitions
>of the socket
>functions should be moved outside the #define
>LWIP_SOCKET_EXTERNAL_HEADERS, because
>a user that has set LWIP_SOCKETS to 1 will need them regardless they
>use lwip's or
>    their own socket headers.
>- struct sockaddr includes sa_len, but some systems like Linux doesn't
>have this filed,
>    which produces many compilation problems.
>    - My solution was:
>  - Create macros to write and read from sa_len/sin_len/sin6_len fields
>      - Replace all occurrences this fields for the proper macro
>- Declare write macros as empty and read macros as default values on
>systems where
>        sa_len and pals are not supported
>      - Use _HAVE_SA_LEN to identify whether the system supports sa_len
>
>I'll write patches for all this issues if you think they should be
>upstream.

I think those suggestions overall look ok. Just post the patches.

Regards,
Simon