On 06/08/2017 08:39 AM, Eduardo Habkost wrote: > The only callers of vnc_client_io_error() with non-NULL errp don't use > *errp after the function gets called, so there's no need to use an > Error** argument. Change parameter to Error* and simplify the code. > > Cc: Gerd Hoffmann <kra...@redhat.com> > Cc: Daniel P. Berrange <berra...@redhat.com> > Signed-off-by: Eduardo Habkost <ehabk...@redhat.com> > --- > ui/vnc.h | 2 +- > ui/vnc.c | 13 +++++-------- > 2 files changed, 6 insertions(+), 9 deletions(-)
>
> -ssize_t vnc_client_io_error(VncState *vs, ssize_t ret, Error **errp)
> +ssize_t vnc_client_io_error(VncState *vs, ssize_t ret, Error *err)
> {
This is unusual.
> if (ret <= 0) {
> if (ret == 0) {
> @@ -1188,14 +1188,11 @@ ssize_t vnc_client_io_error(VncState *vs, ssize_t
> ret, Error **errp)
> vnc_disconnect_start(vs);
> } else if (ret != QIO_CHANNEL_ERR_BLOCK) {
> VNC_DEBUG("Closing down client sock: ret %zd (%s)\n",
> - ret, errp ? error_get_pretty(*errp) : "Unknown");
> + ret, err ? error_get_pretty(err) : "Unknown");
> vnc_disconnect_start(vs);
> }
>
> - if (errp) {
> - error_free(*errp);
> - *errp = NULL;
> - }
> + error_free(err);
Worse, it's action at a distance. You are freeing memory here that was
allocated in the callers.
> return 0;
> }
> return ret;
> @@ -1231,7 +1228,7 @@ ssize_t vnc_client_write_buf(VncState *vs, const
> uint8_t *data, size_t datalen)
> ret = qio_channel_write(
> vs->ioc, (const char *)data, datalen, &err);
> VNC_DEBUG("Wrote wire %p %zd -> %ld\n", data, datalen, ret);
> - return vnc_client_io_error(vs, ret, &err);
> + return vnc_client_io_error(vs, ret, err);
> }
It looks like the only reason that err gets passed is for the sake of
VNC_DEBUG messages, but I'm not sure I like this patch.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
