[Qemu-devel] [PATCH] net: Remove dead code from net/socket.c

[Qemu-devel] [PATCH] net: Remove dead code from net/socket.c

[Miguel Di Ciurcio Filho]

When a connection is accepted, the handler net_socket_accept() calls
net_socket_fd_init(). If the socket is UDP/multicast,
net_socket_fd_init() calls net_socket_fd_init_dgram, if
the socket is TCP it calls net_socket_fd_init_dgram_stream.

In both cases the info_str string is written inside
net_socket_fd_init_(stream|dgram), and after that, it is
overwritten on a subsequent snprintf() in net_socket_accept().

net_socket_fd_init_(stream|dgram) always puts into the info_str the fd
number, and this information is overwritten latter. No one has noticed this for
so long that I believe this code is dead.

Signed-off-by: Miguel Di Ciurcio Filho <miguel.filho@gmail.com>
---
 net/socket.c |    7 -------
 1 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/net/socket.c b/net/socket.c
index 1c4e153..b72a2de 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -267,11 +267,6 @@ static NetSocketState *net_socket_fd_init_dgram(VLANState *vlan,
 
     nc = qemu_new_net_client(&net_dgram_socket_info, vlan, NULL, model, name);
 
-    snprintf(nc->info_str, sizeof(nc->info_str),
-	    "socket: fd=%d (%s mcast=%s:%d)",
-	    fd, is_connected ? "cloned" : "",
-	    inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port));
-
     s = DO_UPCAST(NetSocketState, nc, nc);
 
     s->fd = fd;
@@ -307,8 +302,6 @@ static NetSocketState *net_socket_fd_init_stream(VLANState *vlan,
 
     nc = qemu_new_net_client(&net_socket_info, vlan, NULL, model, name);
 
-    snprintf(nc->info_str, sizeof(nc->info_str), "socket: fd=%d", fd);
-
     s = DO_UPCAST(NetSocketState, nc, nc);
 
     s->fd = fd;
-- 
1.7.1

[Qemu-devel] Re: [PATCH] net: Remove dead code from net/socket.c

[Jan Kiszka]

Miguel Di Ciurcio Filho wrote:
> When a connection is accepted, the handler net_socket_accept() calls
> net_socket_fd_init(). If the socket is UDP/multicast,
> net_socket_fd_init() calls net_socket_fd_init_dgram, if
> the socket is TCP it calls net_socket_fd_init_dgram_stream.
> 
> In both cases the info_str string is written inside
> net_socket_fd_init_(stream|dgram), and after that, it is
> overwritten on a subsequent snprintf() in net_socket_accept().
> 

There is non-zero time window between registration and acceptance. And
you have the path taken for fd sockets.

Jan

> net_socket_fd_init_(stream|dgram) always puts into the info_str the fd
> number, and this information is overwritten latter. No one has noticed this for
> so long that I believe this code is dead.
> 
> Signed-off-by: Miguel Di Ciurcio Filho <miguel.filho@gmail.com>
> ---
>  net/socket.c |    7 -------
>  1 files changed, 0 insertions(+), 7 deletions(-)
> 
> diff --git a/net/socket.c b/net/socket.c
> index 1c4e153..b72a2de 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -267,11 +267,6 @@ static NetSocketState *net_socket_fd_init_dgram(VLANState *vlan,
>  
>      nc = qemu_new_net_client(&net_dgram_socket_info, vlan, NULL, model, name);
>  
> -    snprintf(nc->info_str, sizeof(nc->info_str),
> -	    "socket: fd=%d (%s mcast=%s:%d)",
> -	    fd, is_connected ? "cloned" : "",
> -	    inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port));
> -
>      s = DO_UPCAST(NetSocketState, nc, nc);
>  
>      s->fd = fd;
> @@ -307,8 +302,6 @@ static NetSocketState *net_socket_fd_init_stream(VLANState *vlan,
>  
>      nc = qemu_new_net_client(&net_socket_info, vlan, NULL, model, name);
>  
> -    snprintf(nc->info_str, sizeof(nc->info_str), "socket: fd=%d", fd);
> -
>      s = DO_UPCAST(NetSocketState, nc, nc);
>  
>      s->fd = fd;

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

[Qemu-devel] Re: [PATCH] net: Remove dead code from net/socket.c

[Miguel Di Ciurcio Filho]

On Fri, May 14, 2010 at 2:03 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>
>> In both cases the info_str string is written inside
>> net_socket_fd_init_(stream|dgram), and after that, it is
>> overwritten on a subsequent snprintf() in net_socket_accept().
>>
>
> There is non-zero time window between registration and acceptance. And
> you have the path taken for fd sockets.
>

It is non-zero, yes, but what is the point of writing to info_str with
an fd number for less them 5ms, when that value will be overwritten
anyway?

I'm an student, just trying to understand what is going on :-D

Regards,

Miguel

[Qemu-devel] Re: [PATCH] net: Remove dead code from net/socket.c

[Jan Kiszka]

Miguel Di Ciurcio Filho wrote:
> On Fri, May 14, 2010 at 2:03 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>> In both cases the info_str string is written inside
>>> net_socket_fd_init_(stream|dgram), and after that, it is
>>> overwritten on a subsequent snprintf() in net_socket_accept().
>>>
>> There is non-zero time window between registration and acceptance. And
>> you have the path taken for fd sockets.
>>
> 
> It is non-zero, yes, but what is the point of writing to info_str with
> an fd number for less them 5ms, when that value will be overwritten
> anyway?

You are right regarding listen sockets, I forgot that they block, thus
there is no chance to request any information about them before the link
has been established.

But my point about the sockets created by file descriptor remains valid.

> 
> I'm an student, just trying to understand what is going on :-D

Always welcome! People who look from a different angle at this can often
find cruft that others already stopped to realize.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux