Re: [PATCH] io/channel-socket: Remove unused qio_channel_socket_dgram_async

[Daniel P . Berrangé]

On Thu, Sep 19, 2024 at 12:36:51PM +0000, Dr. David Alan Gilbert wrote:
> * Daniel P. Berrangé (berrange@redhat.com) wrote:
> > On Thu, Sep 19, 2024 at 01:00:34AM +0100, dave@treblig.org wrote:
> > > From: "Dr. David Alan Gilbert" <dave@treblig.org>
> > > 
> > > qio_channel_socket_dgram_async has been unused since it was originally
> > > added in 2015.
> > > 
> > > Remove it.
> > 
> > This was knowingly added as unused, since the QIO channel APIs
> > were intended to be providing an general purpose interface that
> > was anticipating future development. On the one hand it hasn't
> > been used in 9 years, but on the other hand removing it makes
> > the API inconsistent since we provide sync+async variants of
> > every API.
> 
> Yeh that's fair enough; there's a big variation in approaches
> to deadcode, some people are absolute on it, some people would
> rather keep the consistency.
> See the kernel thread at:
>    https://lore.kernel.org/lkml/ZugliLgw5VFb9yau@gallifrey/

I think most QEMU deadcode can be removed, as much of the internal
code has just grown organically with usage. A few areas of QEMU
were designed as formal internal infra APIs, effectively as a
"library" for QEMU. Mostly in the latter, we might find places
where it is better to keep the deadcode, assuming we still like
the design of the APIs in question. I guess this is where things
like "deadcode" annotations for linters come in handy.

So I'll pass on this particular patch.

> 
> Dave
> 
> > > 
> > > Signed-off-by: Dr. David Alan Gilbert <dave@treblig.org>
> > > ---
> > >  include/io/channel-socket.h | 29 --------------------
> > >  io/channel-socket.c         | 54 -------------------------------------
> > >  io/trace-events             |  1 -
> > >  3 files changed, 84 deletions(-)
> > > 
> > > diff --git a/include/io/channel-socket.h b/include/io/channel-socket.h
> > > index ab15577d38..6c858cc6b5 100644
> > > --- a/include/io/channel-socket.h
> > > +++ b/include/io/channel-socket.h
> > > @@ -182,35 +182,6 @@ int qio_channel_socket_dgram_sync(QIOChannelSocket 
> > > *ioc,
> > >                                    SocketAddress *remoteAddr,
> > >                                    Error **errp);
> > >  
> > > -/**
> > > - * qio_channel_socket_dgram_async:
> > > - * @ioc: the socket channel object
> > > - * @localAddr: the address to local bind address
> > > - * @remoteAddr: the address to remote peer address
> > > - * @callback: the function to invoke on completion
> > > - * @opaque: user data to pass to @callback
> > > - * @destroy: the function to free @opaque
> > > - * @context: the context to run the async task. If %NULL, the default
> > > - *           context will be used.
> > > - *
> > > - * Attempt to initialize a datagram socket bound to
> > > - * @localAddr and communicating with peer @remoteAddr.
> > > - * This method will run in the background so the caller
> > > - * will regain execution control immediately. The function
> > > - * @callback will be invoked on completion or failure.
> > > - * The @localAddr and @remoteAddr parameters will be copied,
> > > - * so may be freed as soon as this function returns without
> > > - * waiting for completion.
> > > - */
> > > -void qio_channel_socket_dgram_async(QIOChannelSocket *ioc,
> > > -                                    SocketAddress *localAddr,
> > > -                                    SocketAddress *remoteAddr,
> > > -                                    QIOTaskFunc callback,
> > > -                                    gpointer opaque,
> > > -                                    GDestroyNotify destroy,
> > > -                                    GMainContext *context);
> > > -
> > > -
> > >  /**
> > >   * qio_channel_socket_get_local_address:
> > >   * @ioc: the socket channel object
> > > diff --git a/io/channel-socket.c b/io/channel-socket.c
> > > index 608bcf066e..2282e7a549 100644
> > > --- a/io/channel-socket.c
> > > +++ b/io/channel-socket.c
> > > @@ -319,60 +319,6 @@ int qio_channel_socket_dgram_sync(QIOChannelSocket 
> > > *ioc,
> > >  }
> > >  
> > >  
> > > -struct QIOChannelSocketDGramWorkerData {
> > > -    SocketAddress *localAddr;
> > > -    SocketAddress *remoteAddr;
> > > -};
> > > -
> > > -
> > > -static void qio_channel_socket_dgram_worker_free(gpointer opaque)
> > > -{
> > > -    struct QIOChannelSocketDGramWorkerData *data = opaque;
> > > -    qapi_free_SocketAddress(data->localAddr);
> > > -    qapi_free_SocketAddress(data->remoteAddr);
> > > -    g_free(data);
> > > -}
> > > -
> > > -static void qio_channel_socket_dgram_worker(QIOTask *task,
> > > -                                            gpointer opaque)
> > > -{
> > > -    QIOChannelSocket *ioc = 
> > > QIO_CHANNEL_SOCKET(qio_task_get_source(task));
> > > -    struct QIOChannelSocketDGramWorkerData *data = opaque;
> > > -    Error *err = NULL;
> > > -
> > > -    /* socket_dgram() blocks in DNS lookups, so we must use a thread */
> > > -    qio_channel_socket_dgram_sync(ioc, data->localAddr,
> > > -                                  data->remoteAddr, &err);
> > > -
> > > -    qio_task_set_error(task, err);
> > > -}
> > > -
> > > -
> > > -void qio_channel_socket_dgram_async(QIOChannelSocket *ioc,
> > > -                                    SocketAddress *localAddr,
> > > -                                    SocketAddress *remoteAddr,
> > > -                                    QIOTaskFunc callback,
> > > -                                    gpointer opaque,
> > > -                                    GDestroyNotify destroy,
> > > -                                    GMainContext *context)
> > > -{
> > > -    QIOTask *task = qio_task_new(
> > > -        OBJECT(ioc), callback, opaque, destroy);
> > > -    struct QIOChannelSocketDGramWorkerData *data = g_new0(
> > > -        struct QIOChannelSocketDGramWorkerData, 1);
> > > -
> > > -    data->localAddr = QAPI_CLONE(SocketAddress, localAddr);
> > > -    data->remoteAddr = QAPI_CLONE(SocketAddress, remoteAddr);
> > > -
> > > -    trace_qio_channel_socket_dgram_async(ioc, localAddr, remoteAddr);
> > > -    qio_task_run_in_thread(task,
> > > -                           qio_channel_socket_dgram_worker,
> > > -                           data,
> > > -                           qio_channel_socket_dgram_worker_free,
> > > -                           context);
> > > -}
> > > -
> > > -
> > >  QIOChannelSocket *
> > >  qio_channel_socket_accept(QIOChannelSocket *ioc,
> > >                            Error **errp)
> > > diff --git a/io/trace-events b/io/trace-events
> > > index d4c0f84a9a..5d0d4358db 100644
> > > --- a/io/trace-events
> > > +++ b/io/trace-events
> > > @@ -25,7 +25,6 @@ qio_channel_socket_listen_async(void *ioc, void *addr, 
> > > int num) "Socket listen a
> > >  qio_channel_socket_listen_fail(void *ioc) "Socket listen fail ioc=%p"
> > >  qio_channel_socket_listen_complete(void *ioc, int fd) "Socket listen 
> > > complete ioc=%p fd=%d"
> > >  qio_channel_socket_dgram_sync(void *ioc, void *localAddr, void 
> > > *remoteAddr) "Socket dgram sync ioc=%p localAddr=%p remoteAddr=%p"
> > > -qio_channel_socket_dgram_async(void *ioc, void *localAddr, void 
> > > *remoteAddr) "Socket dgram async ioc=%p localAddr=%p remoteAddr=%p"
> > >  qio_channel_socket_dgram_fail(void *ioc) "Socket dgram fail ioc=%p"
> > >  qio_channel_socket_dgram_complete(void *ioc, int fd) "Socket dgram 
> > > complete ioc=%p fd=%d"
> > >  qio_channel_socket_accept(void *ioc) "Socket accept start ioc=%p"
> > > -- 
> > > 2.46.0
> > > 
> > 
> > With regards,
> > Daniel
> > -- 
> > |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange 
> > :|
> > |: https://libvirt.org         -o-            https://fstop138.berrange.com 
> > :|
> > |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange 
> > :|
> > 
> -- 
>  -----Open up your eyes, open up your mind, open up your code -------   
> / Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
> \        dave @ treblig.org |                               | In Hex /
>  \ _________________________|_____ http://www.treblig.org   |_______/
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|