No review, just an observation.
Mao Zhongyi <maozy.f...@cn.fujitsu.com> writes:
> Currently, net_socket_mcast_create(), net_socket_fd_init_dgram() and
> net_socket_fd_init() use the function such as fprintf(), perror() to
> report an error message.
>
> Now, convert these functions to Error.
>
> CC: jasow...@redhat.com, arm...@redhat.com
> Signed-off-by: Mao Zhongyi <maozy.f...@cn.fujitsu.com>
> ---
> net/socket.c | 66
> +++++++++++++++++++++++++++++++++++++-----------------------
> 1 file changed, 41 insertions(+), 25 deletions(-)
>
> diff --git a/net/socket.c b/net/socket.c
> index b0decbe..559e09a 100644
> --- a/net/socket.c
> +++ b/net/socket.c
[...]
> @@ -433,25 +437,27 @@ static NetSocketState
> *net_socket_fd_init_stream(NetClientState *peer,
>
> static NetSocketState *net_socket_fd_init(NetClientState *peer,
> const char *model, const char
> *name,
> - int fd, int is_connected)
> + int fd, int is_connected,
> + Error **errp)
> {
> int so_type = -1, optlen=sizeof(so_type);
>
> if(getsockopt(fd, SOL_SOCKET, SO_TYPE, (char *)&so_type,
> (socklen_t *)&optlen)< 0) {
> - fprintf(stderr, "qemu: error: getsockopt(SO_TYPE) for fd=%d
> failed\n",
> + error_setg(errp, "qemu: error: getsockopt(SO_TYPE) for fd=%d failed",
> fd);
> closesocket(fd);
> return NULL;
> }
> switch(so_type) {
> case SOCK_DGRAM:
> - return net_socket_fd_init_dgram(peer, model, name, fd, is_connected);
> + return net_socket_fd_init_dgram(peer, model, name, fd, is_connected,
> errp);
> case SOCK_STREAM:
> return net_socket_fd_init_stream(peer, model, name, fd,
> is_connected);
> default:
> /* who knows ... this could be a eg. a pty, do warn and continue as
> stream */
> - fprintf(stderr, "qemu: warning: socket type=%d for fd=%d is not
> SOCK_DGRAM or SOCK_STREAM\n", so_type, fd);
> + error_setg(errp, "qemu: warning: socket type=%d for fd=%d is not
> SOCK_DGRAM"
> + " or SOCK_STREAM", so_type, fd);
Not this patches problem: this case is odd, and the comment is bogus.
If @fd really was a pty, getsockopt() would fail with ENOTSOCK, wouldn't
it? If @fd is a socket, but neither SOCK_DGRAM nor SOCK_STREAM (say
SOCK_RAW), why is it safe to continue as if it was SOCK_STREAM? Jason?
> return net_socket_fd_init_stream(peer, model, name, fd,
> is_connected);
> }
> return NULL;
Not reachable. Ugly, but not your patch's concern.
[...]
