[PATCH] socket_wrapper cleanups

[Michael Adam]

On 2016-08-11 at 23:00 +0530, Anoop C S wrote:
> Hi,
> 
> Attached are some cleanup patches for socket_wrapper.
> FYI: make test ran successfully with attached patches.
> 
> Thanks to Michael for all his help..!!
> 
> Regards,
> --Anoop C S.

Thanks, Anoop for polishing and sending the patches!


> From e60f3c324ed843ce9a8d7f0de6e013aa114c9397 Mon Sep 17 00:00:00 2001
> From: Anoop C S <anoopcs at redhat.com>
> Date: Thu, 11 Aug 2016 19:27:17 +0530
> Subject: [PATCH 1/4] swrap: Simplify swrap_remove_stale by early return
> 
> Pair-programmed-with: Michael Adam <obnox at samba.org>
> Signed-off-by: Anoop C S <anoopcs at redhat.com>
> ---
>  src/socket_wrapper.c | 30 ++++++++++++++++--------------
>  1 file changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/src/socket_wrapper.c b/src/socket_wrapper.c
> index ba289e6..e944a3b 100644
> --- a/src/socket_wrapper.c
> +++ b/src/socket_wrapper.c
> @@ -1469,23 +1469,25 @@ static void swrap_remove_stale(int fd)
>  	struct socket_info *si = find_socket_info(fd);
>  	struct socket_info_fd *fi;
>  
> -	if (si != NULL) {
> -		for (fi = si->fds; fi; fi = fi->next) {
> -			if (fi->fd == fd) {
> -				SWRAP_LOG(SWRAP_LOG_TRACE, "remove stale wrapper for %d", fd);
> -				SWRAP_DLIST_REMOVE(si->fds, fi);
> -				free(fi);
> -				break;
> -			}
> +	if (si == NULL) {
> +		return;
> +	}
> +
> +	for (fi = si->fds; fi; fi = fi->next) {
> +		if (fi->fd == fd) {
> +			SWRAP_LOG(SWRAP_LOG_TRACE, "remove stale wrapper for %d", fd);
> +			SWRAP_DLIST_REMOVE(si->fds, fi);
> +			free(fi);
> +			break;
>  		}
> +	}
>  
> -		if (si->fds == NULL) {
> -			SWRAP_DLIST_REMOVE(sockets, si);
> -			if (si->un_addr.sun_path[0] != '\0') {
> -				unlink(si->un_addr.sun_path);
> -			}
> -			free(si);
> +	if (si->fds == NULL) {
> +		SWRAP_DLIST_REMOVE(sockets, si);
> +		if (si->un_addr.sun_path[0] != '\0') {
> +			unlink(si->un_addr.sun_path);
>  		}
> +		free(si);
>  	}
>  }
>  
> -- 
> 2.7.4

Signed-off-by: Michael Adam <obnox at samba.org>

Note that git show -w reveals that this is
mostly an indentation change!


> From d1379c2b032efd9e6b5d8d72d776f062a31978bd Mon Sep 17 00:00:00 2001
> From: Anoop C S <anoopcs at redhat.com>
> Date: Thu, 11 Aug 2016 20:20:17 +0530
> Subject: [PATCH 2/4] swrap: Remove redunant check in swrap_socket
> 
> The very same check is also being made inside swrap_remove_stale().
> So we can get rid of this early if condition.
> 
> Pair-programmed-with: Michael Adam <obnox at samba.org>
> Signed-off-by: Anoop C S <anoopcs at redhat.com>
> ---
>  src/socket_wrapper.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/src/socket_wrapper.c b/src/socket_wrapper.c
> index e944a3b..b11e4a9 100644
> --- a/src/socket_wrapper.c
> +++ b/src/socket_wrapper.c
> @@ -2458,10 +2458,7 @@ static int swrap_socket(int family, int type, int protocol)
>  	}
>  
>  	/* Check if we have a stale fd and remove it */
> -	si = find_socket_info(fd);
> -	if (si != NULL) {
> -		swrap_remove_stale(fd);
> -	}
> +	swrap_remove_stale(fd);
>  
>  	si = (struct socket_info *)calloc(1, sizeof(struct socket_info));
>  	if (si == NULL) {
> -- 
> 2.7.4


Signed-off-by: Michael Adam <obnox at samba.org>


> From ce488b9cc537353681aae47a0b78b4ba384315cf Mon Sep 17 00:00:00 2001
> From: Anoop C S <anoopcs at redhat.com>
> Date: Thu, 11 Aug 2016 20:24:54 +0530
> Subject: [PATCH 3/4] swrap: Delay addition of child socket_info_fd into
>  socket_info list
> 
> In swrap_accept() we used to add new child socket_info_fd[child_fi]
> into newly created child socket_info struture[child_si] without
> considering the fact that we may return early in case of errors from
> subsequent calls to libc_getsockname() and sockaddr_convert_from_un()
> during which we free child_fi and child_si and return. So it is better
> to delay the addition of child_fi into child_si->fds until child_si
> is completely initialized.
> 
> Signed-off-by: Anoop C S <anoopcs at redhat.com>
> ---
>  src/socket_wrapper.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/src/socket_wrapper.c b/src/socket_wrapper.c
> index b11e4a9..00518c1 100644
> --- a/src/socket_wrapper.c
> +++ b/src/socket_wrapper.c
> @@ -2682,8 +2682,6 @@ static int swrap_accept(int s,
>  
>  	child_fi->fd = fd;
>  
> -	SWRAP_DLIST_ADD(child_si->fds, child_fi);
> -
>  	child_si->family = parent_si->family;
>  	child_si->type = parent_si->type;
>  	child_si->protocol = parent_si->protocol;
> @@ -2736,6 +2734,7 @@ static int swrap_accept(int s,
>  	};
>  	memcpy(&child_si->myname.sa.ss, &in_my_addr.sa.ss, in_my_addr.sa_socklen);
>  
> +	SWRAP_DLIST_ADD(child_si->fds, child_fi);
>  	SWRAP_DLIST_ADD(sockets, child_si);
>  
>  	if (addr != NULL) {
> -- 
> 2.7.4


Reviewed-by: Michael Adam <obnox at samba.org>


Cheers - Michael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160811/5a5da6dc/signature.sig>