[lwip-devel] [patch #9350] Sockets API: use OS's sys/socket.h instead of lwip/sock

[Joan Lledó]

URL:
  <http://savannah.nongnu.org/patch/?9350>

                 Summary: Sockets API: use OS's sys/socket.h instead of
lwip/sock
                 Project: lwIP - A Lightweight TCP/IP stack
            Submitted by: jllledo
            Submitted on: Sun 21 May 2017 12:10:40 PM CEST
                Category: sockets/netconn
                Priority: 5 - Normal
                  Status: None
                 Privacy: Public
             Assigned to: None
        Originator Email: 
             Open/Closed: Open
         Discussion Lock: Any
         Planned Release: None

    _______________________________________________________

Details:

Hello,

This comes from[1].

This submission includes the patch but, after working a bit more on the issue,
I think the patch shouldn't be committed. Following are the problems I found:

1- The patch only covers the macros and structures listed in [2], but those
are only a little subset of the macros that are actually used in the real
world. For instance, PF_INET is not listed. This means all the macros in
sys/socket.h but not in the list have to be wrapped with #ifndef - #endif, and
we don't have a clear line to separate what should be wrapped and what not, so
the only solution is to wrap everything.

2- An OS's sockets implementation is not limited to sys/sockets.h, there are
more headers like netinet/in.h that are not considered in the patch. The patch
as it's submitted here works for the Hurd, but fails on Linux. There are many
compiler errors because many types in sys/types.h doesn't match the types lwip
uses internally, so to make this work, we should change a lot of variable
definitions to be something like "typeof(other_variable) var;". 

3- Even doing the effort of making points 1 and 2 work, unit tests show that
applying the patch breaks some functions like lwip_connect(). Probably because
some macro comparison still doesn't match the proper value.

Summarizing, in [1] I thought supporting sys/sockets.h would be enough but
it's not, so the patch is incomplete and the gain is not worth the work of
research and writing the changes.

Regards.




    _______________________________________________________

File Attachments:


-------------------------------------------------------
Date: Sun 21 May 2017 12:10:40 PM CEST  Name:
0001-Sockets-API-use-OS-s-sys-socket.h-instead-of-lwip-so.patch  Size: 11kB  
By: jllledo

<http://savannah.nongnu.org/patch/download.php?file_id=40750>

    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/patch/?9350>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/