netdev - Re: [PATCH net-next 10/10] tipc: refactor accept() code for improved readability

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20121207194226.GB30339@hmsreliant.think-freely.org>
Date:	Fri, 7 Dec 2012 14:42:26 -0500
From:	Neil Horman <nhorman@...driver.com>
To:	Paul Gortmaker <paul.gortmaker@...driver.com>
Cc:	David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
	Jon Maloy <jon.maloy@...csson.com>
Subject: Re: [PATCH net-next 10/10] tipc: refactor accept() code for improved
 readability

On Fri, Dec 07, 2012 at 09:28:18AM -0500, Paul Gortmaker wrote:
> In TIPC's accept() routine, there is a large block of code relating
> to initialization of a new socket, all within an if condition checking
> if the allocation succeeded.
> 
> Here, we simply factor out that init code within the accept() function
> to its own separate function, which improves readability, and makes
> it easier to track which lock_sock() calls are operating on existing
> vs. new sockets.
> 
> Signed-off-by: Paul Gortmaker <paul.gortmaker@...driver.com>
> ---
>  net/tipc/socket.c | 93 +++++++++++++++++++++++++++++--------------------------
>  1 file changed, 49 insertions(+), 44 deletions(-)
> 
> diff --git a/net/tipc/socket.c b/net/tipc/socket.c
> index 38613cf..56661c8 100644
> --- a/net/tipc/socket.c
> +++ b/net/tipc/socket.c
> @@ -1507,6 +1507,53 @@ static int listen(struct socket *sock, int len)
>  	return res;
>  }
>  
> +static void tipc_init_socket(struct sock *sk, struct socket *new_sock,
> +			     struct sk_buff *buf)
> +{
Can you rename this to something more specific to its purpose?  tipc_init_socket
makes me wonder why you're not calling it internally from tipc_create.  This
seems more like a tipc_init_accept_sock, or some such.  Alternatively, since
you're ony using it from your accept call, you might consider not factoring it
out at all, and just reversing the logic in your accept function so that you do:

 res = tipc_create(...)
 if (res)
	goto exit;
 <rest of tipc_init_socket goes here>

That gives you the same level of readability, without the additional potential
call instruction, plus you put the unlikely case inside the if conditional.

Neil

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html