Re: [Minios-devel] [UNIKRAFT/LWIP PATCH] sockets.c: Make select() not return errors when using it with file descriptor types other than sockets

[Stefan Teodorescu <stefanl.teodorescu@xxxxxxxxx>]

Everything looks good, but I would write a help message for this
option that makes it more clear to the user that their file descriptor
(anything else than what is supported by lwip) *will be ignored*.
The current message might make them understand that select() will
support the fds not supported by lwip.

On Sun, Nov 17, 2019 at 1:46 AM Costin Lupu <costin.lupu@xxxxxxxxx> wrote:
>
> lwip's select() implementation supports only sockets. Up until now, we 
> returned
> errors when using select() with other file descriptor types. These changes 
> make
> it possible to use other file descriptor types as well, even though we won't
> receive any notifications for those.
>
> Signed-off-by: Costin Lupu <costin.lupu@xxxxxxxxx>
> ---
>  Config.uk | 10 ++++++++++
>  sockets.c | 24 ++++++++++++++++++++++++
>  2 files changed, 34 insertions(+)
>
> diff --git a/Config.uk b/Config.uk
> index 92df4d3..8f501fc 100644
> --- a/Config.uk
> +++ b/Config.uk
> @@ -162,6 +162,16 @@ config LWIP_SOCKET
>         depends on LWIP_THREADS && (LWIP_UDP || LWIP_TCP)
>         default y
>
> +if LWIP_SOCKET
> +       config LWIP_SOCKET_SELECT_GENERIC_FDS
> +               bool "Use select() with any file descriptor type"
> +               default y
> +               help
> +                       lwip's select() implementation supports only sockets. 
> This
> +                       configuration option makes it possible to use other 
> file descriptor
> +                       types as well, even though they are not supported by 
> lwip.
> +endif
> +
>  menuconfig LWIP_DEBUG
>         bool "Debug messages"
>         default n
> diff --git a/sockets.c b/sockets.c
> index cd84b97..c2cf57f 100644
> --- a/sockets.c
> +++ b/sockets.c
> @@ -497,12 +497,20 @@ int select(int nfds, fd_set *readfds, fd_set *writefds, 
> fd_set *exceptfds,
>                 if (readfds && FD_ISSET(i, readfds)) {
>                         file = sock_net_file_get(i);
>                         if (PTRISERR(file)) {
> +#if CONFIG_LWIP_SOCKET_SELECT_GENERIC_FDS
> +                               /* We allow other fd types, but we don't 
> support them */
> +                               if (PTR2ERR(file) == -EBADF) {
> +                                       FD_CLR(i, readfds);
> +                                       continue;
> +                               }
> +#else
>                                 LWIP_DEBUGF(SOCKETS_DEBUG,
>                                             ("failed to identify socket 
> descriptor\n"));
>                                 ret = -1;
>                                 /* Setting the errno */
>                                 SOCK_NET_SET_ERRNO(PTR2ERR(file));
>                                 goto EXIT;
> +#endif
>                         }
>                         if (maxfd < file->sock_fd)
>                                 maxfd = file->sock_fd;
> @@ -512,12 +520,20 @@ int select(int nfds, fd_set *readfds, fd_set *writefds, 
> fd_set *exceptfds,
>                 if (writefds && FD_ISSET(i, writefds)) {
>                         file = sock_net_file_get(i);
>                         if (PTRISERR(file)) {
> +#if CONFIG_LWIP_SOCKET_SELECT_GENERIC_FDS
> +                               /* We allow other fd types, but we don't 
> support them */
> +                               if (PTR2ERR(file) == -EBADF) {
> +                                       FD_CLR(i, writefds);
> +                                       continue;
> +                               }
> +#else
>                                 LWIP_DEBUGF(SOCKETS_DEBUG,
>                                             ("failed to identify socket 
> descriptor\n"));
>                                 ret = -1;
>                                 /* Setting the errno */
>                                 SOCK_NET_SET_ERRNO(PTR2ERR(file));
>                                 goto EXIT;
> +#endif
>                         }
>                         if (maxfd < file->sock_fd)
>                                 maxfd = file->sock_fd;
> @@ -527,12 +543,20 @@ int select(int nfds, fd_set *readfds, fd_set *writefds, 
> fd_set *exceptfds,
>                 if (exceptfds && FD_ISSET(i, exceptfds)) {
>                         file = sock_net_file_get(i);
>                         if (PTRISERR(file)) {
> +#if CONFIG_LWIP_SOCKET_SELECT_GENERIC_FDS
> +                               /* We allow other fd types, but we don't 
> support them */
> +                               if (PTR2ERR(file) == -EBADF) {
> +                                       FD_CLR(i, exceptfds);
> +                                       continue;
> +                               }
> +#else
>                                 LWIP_DEBUGF(SOCKETS_DEBUG,
>                                             ("failed to identify socket 
> descriptor\n"));
>                                 ret = -1;
>                                 /* Setting the errno */
>                                 SOCK_NET_SET_ERRNO(PTR2ERR(file));
>                                 goto EXIT;
> +#endif
>                         }
>                         if (maxfd < file->sock_fd)
>                                 maxfd = file->sock_fd;
> --
> 2.20.1
>
>
> _______________________________________________
> Minios-devel mailing list
> Minios-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/mailman/listinfo/minios-devel

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel