Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
karawin opened this issue Oct 19, 2017 · 7 comments
Closed

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

karawin opened this issue Oct 19, 2017 · 7 comments

Comments

@karawin
Copy link

karawin commented Oct 19, 2017

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
Copy link
Member

igrr commented Oct 19, 2017

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
Copy link
Author

karawin commented Oct 19, 2017

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
Copy link
Author

karawin commented Oct 19, 2017

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
Copy link
Author

karawin commented Oct 19, 2017

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
Copy link
Author

karawin commented Oct 19, 2017

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 projectgus changed the title Problem with sockets select() FD_GET/FD_SET macros are incorrectly defined Oct 20, 2017
@projectgus
Copy link
Contributor

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
Copy link
Author

karawin commented Oct 20, 2017

Great, Thanks

@FayeY FayeY changed the title select() FD_GET/FD_SET macros are incorrectly defined [TW#15973] select() FD_GET/FD_SET macros are incorrectly defined Oct 23, 2017
@igrr igrr closed this as completed in f76a3c4 Oct 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants