Re: [Qemu-devel] [PATCH v2] Change net/socket.c to use socket_*() functions

["Daniel P. Berrange" <berrange@redhat.com>]

On Sat, Jun 18, 2016 at 01:24:02PM +0530, Ashijeet Acharya wrote:
> Use socket_*() functions from include/qemu/sockets.h instead of listen()/bind()/  connect()/parse_host_port(). socket_*() fucntions are QAPI based and this patch   performs this api conversion since everything will be using QAPI based sockets in the future. Also add a helper function socket_address_to_string() in              util/qemu-sockets.c which returns the string representation of socket address. Thetask was listed on http://wiki.qemu.org/BiteSizedTasks page.
> 
> Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
> ---
>  include/qemu/sockets.h | 16 ++++++++++++++-
>  net/socket.c           | 55 +++++++++++++++++++++++++-------------------------
>  util/qemu-sockets.c    | 36 +++++++++++++++++++++++++++++++++
>  3 files changed, 78 insertions(+), 29 deletions(-)
> 


> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index 0d6cd1f..771b7db 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -1151,3 +1151,39 @@ void qapi_copy_SocketAddress(SocketAddress **p_dest,
>      qmp_input_visitor_cleanup(qiv);
>      qobject_decref(obj);
>  }
> +
> +char *socket_address_to_string(struct SocketAddress *addr, Error **errp)
> +{
> +    char *buf;
> +    InetSocketAddress *inet;
> +    char host_port[INET6_ADDRSTRLEN + 5 + 4];
> +
> +    switch (addr->type) {
> +    case SOCKET_ADDRESS_KIND_INET:
> +        inet = addr->u.inet.data;
> +        if (strchr(inet->host, ':') == NULL) {
> +            snprintf(host_port, sizeof(host_port), "%s:%s", inet->host,
> +                    inet->port);
> +            buf = g_strdup(host_port);
> +        } else {
> +            snprintf(host_port, sizeof(host_port), "[%s]:%s", inet->host,
> +                    inet->port);
> +            buf = g_strdup(host_port);

I see the patch is already queued, but really this should be changed to
not use a preallocated buffer. It should instead use g_strdup_printf()

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|