Re: [Qemu-block] [Qemu-devel] [PATCH 2/8] io: introduce a network socket listener API

[Daniel P. Berrange]

On Fri, Aug 11, 2017 at 10:15:04AM +0100, Daniel P. Berrange wrote:
> On Thu, Aug 10, 2017 at 01:12:25PM -0500, Eric Blake wrote:
> > On 08/10/2017 11:04 AM, Daniel P. Berrange wrote:
> > > The existing QIOChannelSocket class provides the ability to
> > > listen on a single socket at a time. This patch introduces
> > > a QIONetListener class that provides a higher level API
> > > concept around listening for network services, allowing
> > > for listening on multiple sockets.
> > > 
> > > Signed-off-by: Daniel P. Berrange <address@hidden>
> > > ---
> > 
> > > +++ b/include/io/net-listener.h
> > > @@ -0,0 +1,174 @@
> > > +/*
> > > + * QEMU I/O network listener
> > > + *
> > > + * Copyright (c) 2016 Red Hat, Inc.
> > 
> > Want to add 2017?
> > 
> > At least it's covered by MAINTAINERS :)
> > 
> > 
> > > +/**
> > > + * qio_net_listener_is_disconnected:
> > > + * @listener: the network listener object
> > > + *
> > > + * Determine if the listener is connected to any socket
> > > + * channels
> > > + *
> > > + * Returns: TRUE if connected, FALSE otherwise
> > > + */
> > > +gboolean qio_net_listener_is_disconnected(QIONetListener *listener);
> > > +
> > 
> > Must it return gboolean, or is bool sufficient?
> 
> bool is fine.
> 
> > 
> > TRUE if connected for a function named 'is_disconnected' sounds
> > backwards.  Avoid the double negative, name it:
> > 
> > qio_net_listener_is_connected(), returning true if connected
> 
> The docs are wrong, as you noticed below
> 
> > 
> > > +++ b/io/net-listener.c
> > > @@ -0,0 +1,315 @@
> > > +/*
> > > + * QEMU network listener
> > > + *
> > > + * Copyright (c) 2016 Red Hat, Inc.
> > 
> > More 2017.  Probably for the whole series :)
> > 
> > 
> > > +static gboolean qio_net_listener_channel_func(QIOChannel *ioc,
> > > +                                              GIOCondition condition,
> > > +                                              gpointer opaque)
> > > +{
> > 
> > Again, can we use bool instead of gboolean?
> 
> Yes

Opps, no, this one must be gboolean. The others can be bool though.


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 :|