[Spice-devel] [spice-server PATCH 4/8] main-channel: getpeername/getsockname return early if no sockfd

[Frediano Ziglio]

> 
> I'm not sure that needed as it seems getpeername/getsockname
> accept int fd and return -1 for fd=-1
> 
> Signed-off-by: Uri Lublin <uril at redhat.com>
> ---
>  server/main-channel.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/server/main-channel.c b/server/main-channel.c
> index bf84694..9ff4dcd 100644
> --- a/server/main-channel.c
> +++ b/server/main-channel.c
> @@ -281,12 +281,24 @@ MainChannelClient *main_channel_link(MainChannel
> *channel, RedClient *client,
>  
>  int main_channel_getsockname(MainChannel *main_chan, struct sockaddr *sa,
>  socklen_t *salen)
>  {
> -    return main_chan ?
> getsockname(red_channel_get_first_socket(&main_chan->base), sa, salen) : -1;
> +    int socketfd;
> +
> +    if (!main_chan || ((socketfd =
> red_channel_get_first_socket(&main_chan->base)) < 0)) {
> +        return -1;
> +    }
> +
> +    return getsockname(socketfd, sa, salen);
>  }
>  
>  int main_channel_getpeername(MainChannel *main_chan, struct sockaddr *sa,
>  socklen_t *salen)
>  {
> -    return main_chan ?
> getpeername(red_channel_get_first_socket(&main_chan->base), sa, salen) : -1;
> +    int socketfd;
> +
> +    if (!main_chan || ((socketfd =
> red_channel_get_first_socket(&main_chan->base)) < 0)) {
> +        return -1;
> +    }
> +
> +    return getpeername(socketfd, sa, salen);
>  }
>  
>  // TODO: ? shouldn't it disonnect all clients? or shutdown all
>  main_channels?

Mumble... I don't know why but it does not look that good.

These functions assume that there are only one client.
They are used only by spice_server_get_sock_info and spice_server_get_peer_info
which are no more used by recent Qemu so mainly they are dead code.
Perhaps spice_server_get_sock_info and spice_server_get_peer_info should
check for the first client and call getpeername/getsockname by themself?
Or just adding a main_channel_get_socket instead of 2 obsolete main_channel_*
functions?

OT: red_channel_get_first_socket is used only for obsolete or wrong code.

OT: main_channel_close is causing a dandling file descriptor.

Frediano