[PATCH 1/2] - net/socket.c::sys_bind() cleanup.

[PATCH 1/2] - net/socket.c::sys_bind() cleanup.

[Luiz Fernando N. Capitulino]

 Hi,

 net/socket.c::sys_bind() is a bit complex function, the patch
bellow makes it more clear.

 Note that the code does the same thing, it only makes difference
for the programmer.

 Agains't 2.6.10-rc2.


Signed-off-by: Luiz Capitulino <lcapitulino@conectiva.com.br>

 net/socket.c |   29 +++++++++++++++++------------
 1 files changed, 17 insertions(+), 12 deletions(-)


diff -X /home/lcapitulino/kernels/2.6/dontdiff -Nparu a/net/socket.c a~/net/socket.c
--- a/net/socket.c	2004-10-31 18:44:25.000000000 -0200
+++ a~/net/socket.c	2004-10-31 18:52:01.000000000 -0200
@@ -1286,18 +1286,23 @@ asmlinkage long sys_bind(int fd, struct 
 	char address[MAX_SOCK_ADDR];
 	int err;
 
-	if((sock = sockfd_lookup(fd,&err))!=NULL)
-	{
-		if((err=move_addr_to_kernel(umyaddr,addrlen,address))>=0) {
-			err = security_socket_bind(sock, (struct sockaddr *)address, addrlen);
-			if (err) {
-				sockfd_put(sock);
-				return err;
-			}
-			err = sock->ops->bind(sock, (struct sockaddr *)address, addrlen);
-		}
-		sockfd_put(sock);
-	}			
+	sock = sockfd_lookup(fd, &err);
+	if (!sock)
+		goto out;
+
+	err = move_addr_to_kernel(umyaddr, addrlen, address);
+	if (err)
+		goto out_put;
+
+	err = security_socket_bind(sock, (struct sockaddr *)address, addrlen);
+	if (err)
+		goto out_put;
+
+	err = sock->ops->bind(sock, (struct sockaddr *)address, addrlen);
+
+ out_put:
+	sockfd_put(sock);
+ out:
 	return err;
 }
 

Re: [PATCH 1/2] - net/socket.c::sys_bind() cleanup.

[Jesper Juhl]

On Tue, 16 Nov 2004, Luiz Fernando N. Capitulino wrote:

> 
>  Hi,
> 
>  net/socket.c::sys_bind() is a bit complex function, the patch
> bellow makes it more clear.
> 
>  Note that the code does the same thing, it only makes difference
> for the programmer.
> 
Not exactely : 


> -		if((err=move_addr_to_kernel(umyaddr,addrlen,address))>=0) {

> +	err = move_addr_to_kernel(umyaddr, addrlen, address);
> +	if (err)
> +		goto out_put;


The original tests for err >= 0, your replacement tests if err is != 0


--
Jesper Juhl

Re: [PATCH 1/2] - net/socket.c::sys_bind() cleanup.

[James Morris]

On Wed, 17 Nov 2004, Jesper Juhl wrote:

> Not exactely : 
> 
> 
> > -		if((err=move_addr_to_kernel(umyaddr,addrlen,address))>=0) {
> 
> > +	err = move_addr_to_kernel(umyaddr, addrlen, address);
> > +	if (err)
> > +		goto out_put;
> 
> 
> The original tests for err >= 0, your replacement tests if err is != 0

Look at move_addr_to_kernel(), it only returns 0 or -error.

The patch looks good to me.


- James
-- 
James Morris
<jmorris@redhat.com>

Re: [PATCH 1/2] - net/socket.c::sys_bind() cleanup.

[Jesper Juhl]

James Morris wrote:
> On Wed, 17 Nov 2004, Jesper Juhl wrote:
> 
> 
>>Not exactely : 
>>
>>
>>
>>>-		if((err=move_addr_to_kernel(umyaddr,addrlen,address))>=0) {
>>
>>>+	err = move_addr_to_kernel(umyaddr, addrlen, address);
>>>+	if (err)
>>>+		goto out_put;
>>
>>
>>The original tests for err >= 0, your replacement tests if err is != 0
> 
> 
> Look at move_addr_to_kernel(), it only returns 0 or -error.
> 
> The patch looks good to me.
> 
Right, I had not looked at it in detail. I just reacted to the claim 
that "it does exactely the same" but I could see in the posted patch 
that it didn't do exactely the same and there was no explanation of why 
it was ok to have that difference.
After reading move_addr_to_kernel(), I agree that the patch looks fine.

--
Jesper Juhl