espressif / esp-idf

Espressif IoT Development Framework. Official development framework for Espressif SoCs.
Apache License 2.0
12.98k stars 7.12k forks source link

[TW#15973] select() FD_GET/FD_SET macros are incorrectly defined #1141

Closed karawin closed 6 years ago

karawin commented 6 years ago

On SDK v3.0-dev-942-g2e8441df-dirty Sockets are allocated from 4096. This break the FD_xxxx and far from FD_SETSIZE So no select() can work.

Why this change?

igrr commented 6 years ago

LwIP sockets start at LWIP_SOCKET_OFFSET, and FD_SET/FD_GET macros take this into account when operating on fd sets: LWIP_SOCKET_OFFSET is subtracted from fd number, and the resulting value as used as bit offset in fd set.

Previously, LWIP_SOCKET_OFFSET was always zero. With the latest changes, LWIP_SOCKET_OFFSET is defined to lwip_socket_offset global variable. When LwIP is initialized, the value of lwip_socket_offset is determined dynamically.

It is unlikely, but if you happen to do operations on fd sets before LwIP is initialized, you may get incorrect behavior. Otherwise, lwip_socket_offset should be taken into account correctly.

Perhaps you could share the code which you are having an issue with?

karawin commented 6 years ago

The code is at https://github.com/karawin/Ka-Radio32 main/servers.c

It works well on esp8266 and worked well in esp32 before the last release.

How can i know if lwip_socket_offset is initialized?

karawin commented 6 years ago

Something is wrong in FD_XXXX because I (8999) servers: telnetServer_sock socket: 4096, errno: 0 I (9009) servers: Webserver socket: 4097, errno: 0 ... V (54889) servers: Activity 1, max_fd: 4097 V (54889) servers: Server web accept, socket: 4097. V (54909) servers: telnetServer_sock accept. Socket: 4096 Two FD_ISSET for one activity. And i am sure that the telnetServer_soc socket is not awaked.

        if (FD_ISSET(telnetServer_sock, &readfds)) 
        {
            FD_CLR(telnetServer_sock , &readfds); 
            ESP_LOGV(TAG,"telnetServer_sock accept. Socket: %d",telnetServer_sock);                 
            if ((telnetClient_sock = accept(telnetServer_sock, (struct sockaddr *) &tenetclient_addr, &telnetSin_size)) < 0) 
            {
                ESP_LOGE(TAG,strsTELNET,"accept",errno);
                close(telnetClient_sock);
                vTaskDelay(50);                 
            } else
karawin commented 6 years ago

It seems that i am not able to include the right FD_XXXX in lwip/sockets.h And no error msg ` / FD_SET used for lwip_select /

ifndef FD_SET

undef FD_SETSIZE

/ Make FD_SETSIZE match NUM_SOCKETS in socket.c /

define FD_SETSIZE MEMP_NUM_NETCONN

define FDSETSAFESET(n, code) do { \

if (n >= LWIP_SOCKET_OFFSET && ((n) - LWIP_SOCKET_OFFSET < MEMP_NUM_NETCONN) && (((int)(n) - LWIP_SOCKET_OFFSET) >= 0)) { \ code; }} while(0)

define FDSETSAFEGET(n, code) (n >= LWIP_SOCKET_OFFSET && ((n) - LWIP_SOCKET_OFFSET < MEMP_NUM_NETCONN) && (((int)(n) - LWIP_SOCKET_OFFSET) >= 0) ?\

(code) : 0)

define FD_SET(n, p) FDSETSAFESET(n, (p)->fd_bits[((n)-LWIP_SOCKET_OFFSET)/8] |= (1 << (((n)-LWIP_SOCKET_OFFSET) & 7)))

define FD_CLR(n, p) FDSETSAFESET(n, (p)->fd_bits[((n)-LWIP_SOCKET_OFFSET)/8] &= ~(1 << (((n)-LWIP_SOCKET_OFFSET) & 7)))

define FD_ISSET(n,p) FDSETSAFEGET(n, (p)->fd_bits[((n)-LWIP_SOCKET_OFFSET)/8] & (1 << (((n)-LWIP_SOCKET_OFFSET) & 7)))

define FD_ZERO(p) memset((void)(p), 0, sizeof((p)))

typedef struct fd_set { unsigned char fd_bits [(FD_SETSIZE+7)/8]; } fd_set;

elif LWIP_SOCKET_OFFSET

error LWIP_SOCKET_OFFSET does not work with external FD_SET!

endif / FD_SET /

`

karawin commented 6 years ago

Succeeded with these modifications in lwip/sockets.h Not clean. Please fix it.

/ FD_SET used for lwip_select / //#ifndef FD_SET

ifdef LWIP_SOCKET_OFFSET

undef FD_SET

undef FD_CLR

undef FD_ISSET

undef FD_ZERO

undef _types_fd_set

undef fd_set

projectgus commented 6 years ago

Thanks for reporting this @karawin.

There was a second set of FD_SET/etc macros in newlib sys/types.h that were unexpectedly overriding the LWIP ones. This is why the workaround you have is working as well.

A fix to the newlib header is on the way, in the mean time the workaround you have will fix the problem.

karawin commented 6 years ago

Great, Thanks