diff mbox

eloop: add assert() on negative fd when using select() code path

Message ID 1395339918-4632-1-git-send-email-mbizon@freebox.fr
State Accepted
Headers show

Commit Message

Maxime Bizon March 20, 2014, 6:25 p.m. UTC 
Signed-off-by: Maxime Bizon <mbizon@freebox.fr>
---
 src/utils/eloop.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Dan Williams March 20, 2014, 6:41 p.m. UTC | #1 
On Thu, 2014-03-20 at 19:25 +0100, Maxime Bizon wrote:
> Signed-off-by: Maxime Bizon <mbizon@freebox.fr>
> ---
>  src/utils/eloop.c |    7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/src/utils/eloop.c b/src/utils/eloop.c
> index f83a232..e304c67 100644
> --- a/src/utils/eloop.c
> +++ b/src/utils/eloop.c
> @@ -13,8 +13,9 @@
>  #include "list.h"
>  #include "eloop.h"
>  
> -#ifdef CONFIG_ELOOP_POLL
>  #include <assert.h>
> +
> +#ifdef CONFIG_ELOOP_POLL
>  #include <poll.h>
>  #endif /* CONFIG_ELOOP_POLL */
>  
> @@ -374,8 +375,10 @@ static void eloop_sock_table_set_fds(struct eloop_sock_table *table,
>  	if (table->table == NULL)
>  		return;
>  
> -	for (i = 0; i < table->count; i++)
> +	for (i = 0; i < table->count; i++) {
> +		assert(table->table[i].sock >= 0);
>  		FD_SET(table->table[i].sock, fds);
> +	}
>  }

Wouldn't it be better to just ensure that no -1 fd gets added to the
eloop sock table in the first place?  Eg put an assert in
eloop_register_sock() because I cannot think of *any* case where it's
valid to register an invalid socket.

That said, adding the assert to select-based eloop_sock_table_set_fds()
is valid and does mirror what the poll-based eloop_sock_table_set_fds()
does.

Dan
Jouni Malinen March 27, 2014, 2:42 p.m. UTC | #2 
Thanks, applied with additional assert() added based on comment from
Dan.
diff mbox

Patch

diff --git a/src/utils/eloop.c b/src/utils/eloop.c
index f83a232..e304c67 100644
--- a/src/utils/eloop.c
+++ b/src/utils/eloop.c
@@ -13,8 +13,9 @@ 
 #include "list.h"
 #include "eloop.h"
 
-#ifdef CONFIG_ELOOP_POLL
 #include <assert.h>
+
+#ifdef CONFIG_ELOOP_POLL
 #include <poll.h>
 #endif /* CONFIG_ELOOP_POLL */
 
@@ -374,8 +375,10 @@  static void eloop_sock_table_set_fds(struct eloop_sock_table *table,
 	if (table->table == NULL)
 		return;
 
-	for (i = 0; i < table->count; i++)
+	for (i = 0; i < table->count; i++) {
+		assert(table->table[i].sock >= 0);
 		FD_SET(table->table[i].sock, fds);
+	}
 }