Re: [PATCH v14 15/17] net: stream: move to QIO to enable additional parameters

[Laurent Vivier <lvivier@xxxxxxxxxx>]

On 10/21/22 12:35, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@xxxxxxxxxx> writes:
>
> > On 21/10/22 11:09, Laurent Vivier wrote:
> > > Use QIOChannel, QIOChannelSocket and QIONetListener.
> > > This allows net/stream to use all the available parameters provided by
> > > SocketAddress.
> > > Signed-off-by: Laurent Vivier <lvivier@xxxxxxxxxx>
> > > Acked-by: Michael S. Tsirkin <mst@xxxxxxxxxx>
> > > ---
> > >    net/stream.c    | 492 +++++++++++++++++-------------------------------
> > >    qemu-options.hx |   4 +-
> > >    2 files changed, 178 insertions(+), 318 deletions(-)
> > > -static void net_stream_accept(void *opaque)
> > > +static void net_stream_server_listening(QIOTask *task, gpointer opaque)
> > >    {
> > >        NetStreamState *s = opaque;
> > > -    struct sockaddr_storage saddr;
> > > -    socklen_t len;
> > > -    int fd;
> > > -
> > > -    for (;;) {
> > > -        len = sizeof(saddr);
> > > -        fd = qemu_accept(s->listen_fd, (struct sockaddr *)&saddr, &len);
> > > -        if (fd < 0 && errno != EINTR) {
> > > -            return;
> > > -        } else if (fd >= 0) {
> > > -            qemu_set_fd_handler(s->listen_fd, NULL, NULL, NULL);
> > > -            break;
> > > -        }
> > > -    }
> > > +    QIOChannelSocket *listen_sioc = QIO_CHANNEL_SOCKET(s->listen_ioc);
> > > +    SocketAddress *addr;
> > > +    int ret;
> > >    -    s->fd = fd;
> > > -    s->nc.link_down = false;
> > > -    net_stream_connect(s);
> > > -    switch (saddr.ss_family) {
> > > -    case AF_INET: {
> > > -        struct sockaddr_in *saddr_in = (struct sockaddr_in *)&saddr;
> > > -
> > > -        qemu_set_info_str(&s->nc, "connection from %s:%d",
> > > -                          inet_ntoa(saddr_in->sin_addr),
> > > -                          ntohs(saddr_in->sin_port));
> > > -        break;
> > > +    if (listen_sioc->fd < 0) {
> > > +        qemu_set_info_str(&s->nc, "connection error");
> > > +        return;
> > >        }
> > > -    case AF_UNIX: {
> > > -        struct sockaddr_un saddr_un;
> > >    -        len = sizeof(saddr_un);
> > > -        getsockname(s->listen_fd, (struct sockaddr *)&saddr_un, &len);
> > > -        qemu_set_info_str(&s->nc, "connect from %s", saddr_un.sun_path);
> > > -        break;
> > > -    }
> > > -    default:
> > > -        g_assert_not_reached();
> > > +    addr = qio_channel_socket_get_local_address(listen_sioc, NULL);
> > > +    g_assert(addr != NULL);
> > Missing propagating Error* (observed in v12).
> *If* this is really a programming error: what about &error_abort?
assert() informs the compiler that following code will not use addr with a NULL value, I 
don't think &error_abort does that. This could avoid an error report in code static analyzer.
Thanks,
Laurent