[Spice-devel] [RFC spice-vdagent 16/18] move to GLib memory functions

Victor Toso victortoso at redhat.com
Tue Aug 28 06:16:09 UTC 2018


Hi,

This one I would also take, together with Patch 0001, outside of
this series in order to reduce the patches a bit and focus more
on the Glib integration.

The memory functions in glib are not bounded to GMainLoop so it
should be fine.

On Tue, Aug 14, 2018 at 08:53:50PM +0200, Jakub Janků wrote:
> Some older parts of the code currently use
> memory functions defined in stdlib.h
> and usually handle allocation errors.
> 
> On the other hand, newer parts of the code
> and GLib/GTK+ functions themselves commonly use
> wrappers provided by GLib that terminate
> the program when there isn't enough memory.
> 
> So it doesn't make much sense to check for
> ENOMEM somewhere, while the program can be
> terminated at any time elsewhere.
> 
> Unify the code and use GLib wrappers only.
> 
> malloc --> g_malloc, g_new
> calloc --> g_malloc0, g_new0
> free   --> g_free
> 
> Use g_try_malloc only when allocating
> message buffers, where the agent could
> theoretically continue functioning even
> after the allocation fails.
> (e.g. when trying to send large
>       clipboard data buffer)

No, I don't think we should. For the clipboard example, the best
would be to either have a clipboard-size-limit (pretty sure there
is one) or we should use some limit in the spice-protocol itself.

The use case of g-try-malloc is for systems with small amount of
resources, such as embedded systems (IMHO).

> Use g_clear_pointer where possible
> for simplification.

Yep. If possible, S-o-B too.

Small comment below.

> ---
>  src/udscs.c                  | 23 ++++-----------
>  src/vdagent/x11-randr.c      | 43 ++++++++++-----------------
>  src/vdagentd/console-kit.c   | 13 ++++----
>  src/vdagentd/systemd-login.c | 13 ++++----
>  src/vdagentd/uinput.c        |  9 ++----
>  src/vdagentd/vdagentd.c      | 57 +++++++++---------------------------
>  src/vdagentd/virtio-port.c   | 37 ++++++++---------------
>  7 files changed, 60 insertions(+), 135 deletions(-)
> 
> diff --git a/src/udscs.c b/src/udscs.c
> index 0b80317..681e80f 100644
> --- a/src/udscs.c
> +++ b/src/udscs.c
> @@ -101,9 +101,7 @@ struct udscs_connection *udscs_connect(const char *socketname,
>      if (io_stream == NULL)
>          return NULL;
>  
> -    conn = calloc(1, sizeof(*conn));
> -    if (!conn)
> -        return NULL;
> +    conn = g_new0(struct udscs_connection, 1);
>      conn->debug = debug;
>      conn->conn = vdagent_connection_new(io_stream,
>                                          FALSE,
> @@ -141,8 +139,7 @@ void udscs_destroy_connection(struct udscs_connection **connp)
>      if (conn->debug)
>          syslog(LOG_DEBUG, "%p disconnected", conn);
>  
> -    free(conn);
> -    *connp = NULL;
> +    g_clear_pointer(connp, g_free);
>  }
>  
>  void udscs_set_user_data(struct udscs_connection *conn, void *data)
> @@ -166,7 +163,7 @@ int udscs_write(struct udscs_connection *conn, uint32_t type, uint32_t arg1,
>      struct udscs_message_header header;
>  
>      buff_size = sizeof(header) + size;
> -    buff = malloc(buff_size);
> +    buff = g_try_malloc(buff_size);
>      if (buff == NULL)
>          return -1;
>  
> @@ -212,10 +209,7 @@ struct udscs_server *udscs_server_new(
>  {
>      struct udscs_server *server;
>  
> -    server = calloc(1, sizeof(*server));
> -    if (!server)
> -        return NULL;
> -
> +    server = g_new0(struct udscs_server, 1);
>      server->debug = debug;
>      server->connect_callback = connect_callback;
>      server->read_callback = read_callback;
> @@ -282,7 +276,7 @@ void udscs_destroy_server(struct udscs_server *server)
>          conn = next_conn;
>      }
>      g_object_unref(server->service);
> -    free(server);
> +    g_free(server);
>  }
>  
>  int udscs_get_peer_pid(struct udscs_connection *conn)
> @@ -299,12 +293,7 @@ static gboolean udscs_server_accept_cb(GSocketService    *service,
>      struct udscs_server *server = user_data;
>      struct udscs_connection *new_conn, *conn;
>  
> -    new_conn = calloc(1, sizeof(*conn));
> -    if (!new_conn) {
> -        syslog(LOG_ERR, "out of memory, disconnecting new client");
> -        return TRUE;
> -    }
> -
> +    new_conn = g_new0(struct udscs_connection, 1);
>      new_conn->debug = server->debug;
>      new_conn->read_callback = server->read_callback;
>      new_conn->disconnect_callback = server->disconnect_callback;
> diff --git a/src/vdagent/x11-randr.c b/src/vdagent/x11-randr.c
> index 803cf73..29ce6fc 100644
> --- a/src/vdagent/x11-randr.c
> +++ b/src/vdagent/x11-randr.c
> @@ -81,13 +81,13 @@ static void free_randr_resources(struct vdagent_x11 *x11)
>          for (i = 0 ; i < x11->randr.res->noutput; ++i) {
>              XRRFreeOutputInfo(x11->randr.outputs[i]);
>          }
> -        free(x11->randr.outputs);
> +        g_free(x11->randr.outputs);
>      }
>      if (x11->randr.crtcs != NULL) {
>          for (i = 0 ; i < x11->randr.res->ncrtc; ++i) {
>              XRRFreeCrtcInfo(x11->randr.crtcs[i]);
>          }
> -        free(x11->randr.crtcs);
> +        g_free(x11->randr.crtcs);
>      }
>      XRRFreeScreenResources(x11->randr.res);
>      x11->randr.res = NULL;
> @@ -105,8 +105,8 @@ static void update_randr_res(struct vdagent_x11 *x11, int poll)
>          x11->randr.res = XRRGetScreenResources(x11->display, x11->root_window[0]);
>      else
>          x11->randr.res = XRRGetScreenResourcesCurrent(x11->display, x11->root_window[0]);
> -    x11->randr.outputs = malloc(x11->randr.res->noutput * sizeof(*x11->randr.outputs));
> -    x11->randr.crtcs = malloc(x11->randr.res->ncrtc * sizeof(*x11->randr.crtcs));
> +    x11->randr.outputs = g_malloc(x11->randr.res->noutput * sizeof(*x11->randr.outputs));
> +    x11->randr.crtcs = g_malloc(x11->randr.res->ncrtc * sizeof(*x11->randr.crtcs));
>      for (i = 0 ; i < x11->randr.res->noutput; ++i) {
>          x11->randr.outputs[i] = XRRGetOutputInfo(x11->display, x11->randr.res,
>                                                   x11->randr.res->outputs[i]);
> @@ -696,7 +696,7 @@ static VDAgentMonitorsConfig *get_current_mon_config(struct vdagent_x11 *x11)
>  
>  error:
>      syslog(LOG_ERR, "error: inconsistent or stale data from X");
> -    free(mon_config);
> +    g_free(mon_config);
>      return NULL;
>  }
>  
> @@ -844,12 +844,12 @@ void vdagent_x11_set_monitor_config(struct vdagent_x11 *x11,
>              if (!fallback) {
>                  syslog(LOG_WARNING, "Restoring previous config");
>                  vdagent_x11_set_monitor_config(x11, curr, 1);
> -                free(curr);
> +                g_free(curr);
>                  /* Remember this config failed, if the client is maximized or
>                     fullscreen it will keep sending the failing config. */
> -                free(x11->randr.failed_conf);
> +                g_free(x11->randr.failed_conf);
>                  x11->randr.failed_conf =
> -                    malloc(config_size(mon_config->num_of_monitors));
> +                    g_malloc(config_size(mon_config->num_of_monitors));
>                  if (x11->randr.failed_conf)
>                      memcpy(x11->randr.failed_conf, mon_config,
>                             config_size(mon_config->num_of_monitors));
> @@ -895,7 +895,7 @@ exit:
>  
>      /* Flush output buffers and consume any pending events */
>      vdagent_x11_do_read(x11);
> -    free(curr);
> +    g_free(curr);
>  }
>  
>  void vdagent_x11_send_daemon_guest_xorg_res(struct vdagent_x11 *x11, int update)
> @@ -914,11 +914,7 @@ void vdagent_x11_send_daemon_guest_xorg_res(struct vdagent_x11 *x11, int update)
>              goto no_info;
>  
>          screen_count = curr->num_of_monitors;
> -        res = malloc(screen_count * sizeof(*res));
> -        if (!res) {
> -            free(curr);
> -            goto no_mem;
> -        }
> +        res = g_malloc(screen_count * sizeof(*res));
>  
>          for (i = 0; i < screen_count; i++) {
>              res[i].width  = curr->monitors[i].width;
> @@ -926,7 +922,7 @@ void vdagent_x11_send_daemon_guest_xorg_res(struct vdagent_x11 *x11, int update)
>              res[i].x = curr->monitors[i].x;
>              res[i].y = curr->monitors[i].y;
>          }
> -        free(curr);
> +        g_free(curr);
>          width  = x11->width[0];
>          height = x11->height[0];
>      } else if (x11->has_xinerama) {
> @@ -935,17 +931,13 @@ void vdagent_x11_send_daemon_guest_xorg_res(struct vdagent_x11 *x11, int update)
>          screen_info = XineramaQueryScreens(x11->display, &screen_count);
>          if (!screen_info)
>              goto no_info;
> -        res = malloc(screen_count * sizeof(*res));
> -        if (!res) {
> -            XFree(screen_info);
> -            goto no_mem;
> -        }
> +        res = g_malloc(screen_count * sizeof(*res));
>          for (i = 0; i < screen_count; i++) {
>              if (screen_info[i].screen_number >= screen_count) {
>                  syslog(LOG_ERR, "Invalid screen number in xinerama screen info (%d >= %d)",
>                         screen_info[i].screen_number, screen_count);
>                  XFree(screen_info);
> -                free(res);
> +                g_free(res);
>                  return;
>              }
>              res[screen_info[i].screen_number].width = screen_info[i].width;
> @@ -959,9 +951,7 @@ void vdagent_x11_send_daemon_guest_xorg_res(struct vdagent_x11 *x11, int update)
>      } else {
>  no_info:
>          screen_count = x11->screen_count;
> -        res = malloc(screen_count * sizeof(*res));
> -        if (!res)
> -            goto no_mem;
> +        res = g_malloc(screen_count * sizeof(*res));
>          for (i = 0; i < screen_count; i++) {
>              res[i].width  = x11->width[i];
>              res[i].height = x11->height[i];
> @@ -982,8 +972,5 @@ no_info:
>  
>      udscs_write(x11->vdagentd, VDAGENTD_GUEST_XORG_RESOLUTION, width, height,
>                  (uint8_t *)res, screen_count * sizeof(*res));
> -    free(res);
> -    return;
> -no_mem:
> -    syslog(LOG_ERR, "out of memory while trying to send resolutions, not sending resolutions.");
> +    g_free(res);
>  }
> diff --git a/src/vdagentd/console-kit.c b/src/vdagentd/console-kit.c
> index 390491e..8094fa9 100644
> --- a/src/vdagentd/console-kit.c
> +++ b/src/vdagentd/console-kit.c
> @@ -162,10 +162,7 @@ struct session_info *session_info_create(int verbose, ActiveSessionChangeCb cb)
>      struct session_info *info;
>      GError *err = NULL;
>  
> -    info = calloc(1, sizeof(*info));
> -    if (!info)
> -        return NULL;
> -
> +    info = g_new0(struct session_info, 1);
>      info->verbose = verbose;
>      info->session_is_locked = FALSE;
>      info->session_idle_hint = FALSE;
> @@ -174,7 +171,7 @@ struct session_info *session_info_create(int verbose, ActiveSessionChangeCb cb)
>      if (err) {
>          syslog(LOG_ERR, "%s: %s", __func__, err->message);
>          g_error_free(err);
> -        free(info);
> +        g_free(info);
>          return NULL;
>      }
>  
> @@ -194,9 +191,9 @@ void session_info_destroy(struct session_info *info)
>  
>      si_dbus_signals_unsubscribe(info);
>      g_object_unref(info->dbus);
> -    free(info->seat);
> -    free(info->active_session);
> -    free(info);
> +    g_free(info->seat);
> +    g_free(info->active_session);
> +    g_free(info);
>  }
>  
>  /* Invoke a method on a remote object through DBus and wait for reply.
> diff --git a/src/vdagentd/systemd-login.c b/src/vdagentd/systemd-login.c
> index 0940230..7a9a13f 100644
> --- a/src/vdagentd/systemd-login.c
> +++ b/src/vdagentd/systemd-login.c
> @@ -109,10 +109,7 @@ struct session_info *session_info_create(int verbose, ActiveSessionChangeCb cb)
>      struct session_info *si;
>      int r;
>  
> -    si = calloc(1, sizeof(*si));
> -    if (!si)
> -        return NULL;
> -
> +    si = g_new0(struct session_info, 1);
>      si->verbose = verbose;
>      si->session_is_locked = FALSE;
>      si->session_change_cb = cb;
> @@ -120,7 +117,7 @@ struct session_info *session_info_create(int verbose, ActiveSessionChangeCb cb)
>      r = sd_login_monitor_new("session", &si->mon);
>      if (r < 0) {
>          syslog(LOG_ERR, "Error creating login monitor: %s", strerror(-r));
> -        free(si);
> +        g_free(si);
>          return NULL;
>      }
>  
> @@ -139,8 +136,8 @@ void session_info_destroy(struct session_info *si)
>      g_source_remove(si->io_watch_id);
>      g_io_channel_unref(si->io_channel);
>      sd_login_monitor_unref(si->mon);
> -    free(si->session);
> -    free(si);
> +    g_free(si->session);
> +    g_free(si);
>  }
>  
>  const char *session_info_get_active_session(struct session_info *si)
> @@ -160,7 +157,7 @@ const char *session_info_get_active_session(struct session_info *si)
>          syslog(LOG_INFO, "Active session: %s", si->session);
>  
>      sd_login_monitor_flush(si->mon);
> -    free(old_session);
> +    g_free(old_session);
>  
>      si_dbus_proxy_update(si);
>      return si->session;
> diff --git a/src/vdagentd/uinput.c b/src/vdagentd/uinput.c
> index e2966c4..880a679 100644
> --- a/src/vdagentd/uinput.c
> +++ b/src/vdagentd/uinput.c
> @@ -31,6 +31,7 @@
>  #include <linux/input.h>
>  #include <linux/uinput.h>
>  #include <spice/vd_agent.h>
> +#include <glib.h>
>  #include "uinput.h"
>  
>  struct vdagentd_uinput {
> @@ -52,10 +53,7 @@ struct vdagentd_uinput *vdagentd_uinput_create(const char *devname,
>  {
>      struct vdagentd_uinput *uinput;
>  
> -    uinput = calloc(1, sizeof(*uinput));
> -    if (!uinput)
> -        return NULL;
> -
> +    uinput = g_new0(struct vdagentd_uinput, 1);
>      uinput->devname = devname;
>      uinput->fd      = -1; /* Gets opened by vdagentd_uinput_update_size() */
>      uinput->debug   = debug;
> @@ -76,8 +74,7 @@ void vdagentd_uinput_destroy(struct vdagentd_uinput **uinputp)
>  
>      if (uinput->fd != -1)
>          close(uinput->fd);
> -    free(uinput);
> -    *uinputp = NULL;
> +    g_clear_pointer(uinputp, g_free);
>  }
>  
>  void vdagentd_uinput_update_size(struct vdagentd_uinput **uinputp,
> diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c
> index 6d117a8..b8f617c 100644
> --- a/src/vdagentd/vdagentd.c
> +++ b/src/vdagentd/vdagentd.c
> @@ -123,12 +123,7 @@ static void send_capabilities(struct vdagent_virtio_port *vport,
>      uint32_t size;
>  
>      size = sizeof(*caps) + VD_AGENT_CAPS_BYTES;
> -    caps = calloc(1, size);
> -    if (!caps) {
> -        syslog(LOG_ERR, "out of memory allocating capabilities array (write)");
> -        return;
> -    }
> -
> +    caps = g_malloc0(size);
>      caps->request = request;
>      VD_AGENT_SET_CAPABILITY(caps->caps, VD_AGENT_CAP_MOUSE_STATE);
>      VD_AGENT_SET_CAPABILITY(caps->caps, VD_AGENT_CAP_MONITORS_CONFIG);
> @@ -144,7 +139,7 @@ static void send_capabilities(struct vdagent_virtio_port *vport,
>      vdagent_virtio_port_write(vport, VDP_CLIENT_PORT,
>                                VD_AGENT_ANNOUNCE_CAPABILITIES, 0,
>                                (uint8_t *)caps, size);
> -    free(caps);
> +    g_free(caps);
>  }
>  
>  static void do_client_disconnect(void)
> @@ -197,12 +192,8 @@ static void do_client_monitors(struct vdagent_virtio_port *vport, int port_nr,
>  
>      if (!mon_config ||
>              mon_config->num_of_monitors != new_monitors->num_of_monitors) {
> -        free(mon_config);
> -        mon_config = malloc(size);
> -        if (!mon_config) {
> -            syslog(LOG_ERR, "out of memory allocating monitors config");
> -            return;
> -        }
> +        g_free(mon_config);
> +        mon_config = g_malloc(size);
>      }
>      memcpy(mon_config, new_monitors, size);
>  
> @@ -239,13 +230,8 @@ static void do_client_capabilities(struct vdagent_virtio_port *vport,
>  
>      if (capabilities_size != new_size) {
>          capabilities_size = new_size;
> -        free(capabilities);
> -        capabilities = malloc(capabilities_size * sizeof(uint32_t));
> -        if (!capabilities) {
> -            syslog(LOG_ERR, "oom allocating capabilities array (read)");
> -            capabilities_size = 0;
> -            return;
> -        }
> +        g_free(capabilities);
> +        capabilities = g_malloc(capabilities_size * sizeof(uint32_t));
>      }
>      memcpy(capabilities, caps->caps, capabilities_size * sizeof(uint32_t));
>      if (caps->request) {
> @@ -327,7 +313,7 @@ static void send_file_xfer_status(struct vdagent_virtio_port *vport,
>          data_size = 0;
>      }
>  
> -    status = malloc(sizeof(*status) + data_size);
> +    status = g_malloc(sizeof(*status) + data_size);
>      status->id = GUINT32_TO_LE(id);
>      status->result = GUINT32_TO_LE(xfer_status);
>      if (data)
> @@ -341,7 +327,7 @@ static void send_file_xfer_status(struct vdagent_virtio_port *vport,
>                                    VD_AGENT_FILE_XFER_STATUS, 0,
>                                    (uint8_t *)status, sizeof(*status) + data_size);
>  
> -    free(status);
> +    g_free(status);
>  }
>  
>  static void do_client_file_xfer(struct vdagent_virtio_port *vport,
> @@ -875,13 +861,7 @@ static gboolean remove_active_xfers(gpointer key, gpointer value, gpointer conn)
>  static void agent_connect(struct udscs_connection *conn)
>  {
>      struct agent_data *agent_data;
> -
> -    agent_data = calloc(1, sizeof(*agent_data));
> -    if (!agent_data) {
> -        syslog(LOG_ERR, "Out of memory allocating agent data, disconnecting");
> -        udscs_destroy_connection(&conn);
> -        return;
> -    }
> +    agent_data = g_new0(struct agent_data, 1);
>  
>      if (session_info) {
>          uint32_t pid = udscs_get_peer_pid(conn);
> @@ -900,12 +880,11 @@ static void agent_disconnect(struct udscs_connection *conn)
>  
>      g_hash_table_foreach_remove(active_xfers, remove_active_xfers, conn);
>  
> -    free(agent_data->session);
> -    agent_data->session = NULL;
> +    g_clear_pointer(&agent_data->session, g_free);
>      update_active_session_connection(NULL);
>  
> -    free(agent_data->screen_info);
> -    free(agent_data);
> +    g_free(agent_data->screen_info);
> +    g_free(agent_data);
>  }
>  
>  static void agent_read_complete(struct udscs_connection **connp,
> @@ -934,12 +913,8 @@ static void agent_read_complete(struct udscs_connection **connp,
>              return;
>          }
>  
> -        free(agent_data->screen_info);
> -        res = malloc(n * sizeof(*res));
> -        if (!res) {
> -            syslog(LOG_ERR, "out of memory allocating screen info");
> -            n = 0;
> -        }
> +        g_free(agent_data->screen_info);
> +        res = g_malloc(n * sizeof(*res));
>          memcpy(res, data, n * sizeof(*res));
>          agent_data->width  = header->arg1;
>          agent_data->height = header->arg2;
> @@ -1112,10 +1087,6 @@ int main(int argc, char *argv[])
>      /* Setup communication with vdagent process(es) */
>      server = udscs_server_new(agent_connect, agent_read_complete,
>                                agent_disconnect, debug);
> -    if (server == NULL) {
> -        syslog(LOG_CRIT, "Fatal could not allocate memory for udscs server");
> -        return 1;
> -    }
>  #ifdef WITH_SYSTEMD_SOCKET_ACTIVATION
>      int n_fds;
>      /* try to retrieve pre-configured sockets from systemd */
> diff --git a/src/vdagentd/virtio-port.c b/src/vdagentd/virtio-port.c
> index 9731086..723d9ad 100644
> --- a/src/vdagentd/virtio-port.c
> +++ b/src/vdagentd/virtio-port.c
> @@ -116,10 +116,7 @@ struct vdagent_virtio_port *vdagent_virtio_port_create(const char *portname,
>              return NULL;
>      }
>  
> -    vport = calloc(1, sizeof(*vport));
> -    if (!vport)
> -        return 0;
> -
> +    vport = g_new0(struct vdagent_virtio_port, 1);
>      /* When calling vdagent_connection_new(),
>       * @wait_on_opening MUST be set to TRUE:
>       *
> @@ -166,17 +163,16 @@ static void virtio_port_destroy(struct vdagent_virtio_port **vportp,
>          vport->disconnect_callback(vport, by_user);
>  
>      if (vport->write_buf) {
> -        free(vport->write_buf->buf);
> -        free(vport->write_buf);
> +        g_free(vport->write_buf->buf);
> +        g_free(vport->write_buf);
>      }
>  
>      for (i = 0; i < VDP_END_PORT; i++) {
> -        free(vport->port_data[i].message_data);
> +        g_free(vport->port_data[i].message_data);
>      }
>  
>      vdagent_connection_destroy(vport->conn);
> -    free(vport);
> -    *vportp = NULL;
> +    g_clear_pointer(vportp, g_free);
>  }
>  
>  void vdagent_virtio_port_destroy(struct vdagent_virtio_port **vportp)
> @@ -195,15 +191,12 @@ int vdagent_virtio_port_write_start(
>      VDIChunkHeader chunk_header;
>      VDAgentMessage message_header;
>  
> -    new_wbuf = malloc(sizeof(*new_wbuf));
> -    if (!new_wbuf)
> -        return -1;
> -
> +    new_wbuf = g_new(struct vdagent_virtio_port_buf, 1);
>      new_wbuf->write_pos = 0;
>      new_wbuf->size = sizeof(chunk_header) + sizeof(message_header) + data_size;
> -    new_wbuf->buf = malloc(new_wbuf->size);
> +    new_wbuf->buf =g_try_malloc(new_wbuf->size);
>      if (!new_wbuf->buf) {
> -        free(new_wbuf);
> +        g_free(new_wbuf);
>          return -1;

Functions like this one, we can remove the return value after
using g_malloc() as that would abort() in case of failure
(ENOMEM). In that case, a patch for that function only would be
easier to review/revert if necessary.

Reviewed-by: Victor Toso <victortoso at redhat.com>

Cheers,
Victor

>      }
>  
> @@ -249,7 +242,7 @@ int vdagent_virtio_port_write_append(struct vdagent_virtio_port *vport,
>  
>      if (wbuf->write_pos == wbuf->size) {
>          vdagent_connection_write(vport->conn, wbuf->buf, wbuf->size);
> -        g_clear_pointer(&vport->write_buf, free);
> +        g_clear_pointer(&vport->write_buf, g_free);
>      }
>      return 0;
>  }
> @@ -282,7 +275,7 @@ void vdagent_virtio_port_reset(struct vdagent_virtio_port *vport, int port)
>          syslog(LOG_ERR, "vdagent_virtio_port_reset port out of range");
>          return;
>      }
> -    free(vport->port_data[port].message_data);
> +    g_free(vport->port_data[port].message_data);
>      memset(&vport->port_data[port], 0, sizeof(vport->port_data[0]));
>  }
>  
> @@ -311,12 +304,7 @@ static void vdagent_virtio_port_do_chunk(struct vdagent_virtio_port **vportp,
>              port->message_header.size = GUINT32_FROM_LE(port->message_header.size);
>  
>              if (port->message_header.size) {
> -                port->message_data = malloc(port->message_header.size);
> -                if (!port->message_data) {
> -                    syslog(LOG_ERR, "out of memory, disconnecting virtio");
> -                    virtio_port_destroy(vportp, FALSE);
> -                    return;
> -                }
> +                port->message_data = g_malloc(port->message_header.size);
>              }
>          }
>          pos = read;
> @@ -352,8 +340,7 @@ static void vdagent_virtio_port_do_chunk(struct vdagent_virtio_port **vportp,
>              }
>              port->message_header_read = 0;
>              port->message_data_pos = 0;
> -            free(port->message_data);
> -            port->message_data = NULL;
> +            g_clear_pointer(&port->message_data, g_free);
>          }
>      }
>  }
> -- 
> 2.17.1
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180828/97d5ce58/attachment-0001.sig>


More information about the Spice-devel mailing list