Hi,
On Tue, Sep 04, 2018 at 06:40:46PM +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_clear_pointer where possible
> for simplification.
>
> Note: this commit only partially touches
> src/vdagent/x11.c as the clipboard-handling
> code here should be removed in the future.
I wouldn't mind to change it all for consistency. But a note in
commit log also works for me.
> Signed-off-by: Jakub Janků <jjanku@xxxxxxxxxx>
> ---
> src/udscs.c | 60 ++++++++++--------------------------
> src/vdagent/x11-randr.c | 54 ++++++++++----------------------
> src/vdagent/x11.c | 15 +++------
> src/vdagentd/console-kit.c | 28 ++++++-----------
> src/vdagentd/systemd-login.c | 19 +++++-------
> src/vdagentd/uinput.c | 9 ++----
> src/vdagentd/vdagentd.c | 55 +++++++++------------------------
> src/vdagentd/virtio-port.c | 42 ++++++++-----------------
> 8 files changed, 86 insertions(+), 196 deletions(-)
>
> diff --git a/src/udscs.c b/src/udscs.c
> index a77da99..31643e9 100644
> --- a/src/udscs.c
> +++ b/src/udscs.c
> @@ -87,10 +87,7 @@ struct udscs_connection *udscs_connect(const char *socketname,
> struct sockaddr_un address;
> struct udscs_connection *conn;
>
> - conn = calloc(1, sizeof(*conn));
> - if (!conn)
> - return NULL;
> -
> + conn = g_new0(struct udscs_connection, 1);
> conn->type_to_string = type_to_string;
> conn->no_types = no_types;
> conn->debug = debug;
> @@ -98,7 +95,7 @@ struct udscs_connection *udscs_connect(const char *socketname,
> conn->fd = socket(PF_UNIX, SOCK_STREAM, 0);
> if (conn->fd == -1) {
> syslog(LOG_ERR, "creating unix domain socket: %m");
> - free(conn);
> + g_free(conn);
> return NULL;
> }
>
> @@ -109,7 +106,7 @@ struct udscs_connection *udscs_connect(const char *socketname,
> if (conn->debug) {
> syslog(LOG_DEBUG, "connect %s: %m", socketname);
> }
> - free(conn);
> + g_free(conn);
> return NULL;
> }
>
> @@ -147,13 +144,12 @@ void udscs_destroy_connection(struct udscs_connection **connp)
> wbuf = conn->write_buf;
> while (wbuf) {
> next_wbuf = wbuf->next;
> - free(wbuf->buf);
> - free(wbuf);
> + g_free(wbuf->buf);
> + g_free(wbuf);
> wbuf = next_wbuf;
> }
>
> - free(conn->data.buf);
> - conn->data.buf = NULL;
> + g_clear_pointer(&conn->data.buf, g_free);
>
> if (conn->next)
> conn->next->prev = conn->prev;
> @@ -171,8 +167,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)
> @@ -194,18 +189,11 @@ int udscs_write(struct udscs_connection *conn, uint32_t type, uint32_t arg1,
> struct udscs_buf *wbuf, *new_wbuf;
> struct udscs_message_header header;
>
> - new_wbuf = malloc(sizeof(*new_wbuf));
> - if (!new_wbuf)
> - return -1;
> -
> + new_wbuf = g_new(struct udscs_buf, 1);
> new_wbuf->pos = 0;
> new_wbuf->size = sizeof(header) + size;
> new_wbuf->next = NULL;
> - new_wbuf->buf = malloc(new_wbuf->size);
> - if (!new_wbuf->buf) {
> - free(new_wbuf);
> - return -1;
> - }
> + new_wbuf->buf = g_malloc(new_wbuf->size);
>
> header.type = type;
> header.arg1 = arg1;
> @@ -271,7 +259,7 @@ static void udscs_read_complete(struct udscs_connection **connp)
> return;
> }
>
> - free(conn->data.buf);
> + g_free(conn->data.buf);
> memset(&conn->data, 0, sizeof(conn->data)); /* data.buf = NULL */
> conn->header_read = 0;
> }
> @@ -312,12 +300,7 @@ static void udscs_do_read(struct udscs_connection **connp)
> }
> conn->data.pos = 0;
> conn->data.size = conn->header.size;
> - conn->data.buf = malloc(conn->data.size);
> - if (!conn->data.buf) {
> - syslog(LOG_ERR, "out of memory, disconnecting %p", conn);
> - udscs_destroy_connection(connp);
> - return;
> - }
> + conn->data.buf = g_malloc(conn->data.size);
> }
> } else {
> conn->data.pos += n;
> @@ -354,8 +337,8 @@ static void udscs_do_write(struct udscs_connection **connp)
> wbuf->pos += n;
> if (wbuf->pos == wbuf->size) {
> conn->write_buf = wbuf->next;
> - free(wbuf->buf);
> - free(wbuf);
> + g_free(wbuf->buf);
> + g_free(wbuf);
> }
> }
>
> @@ -414,10 +397,7 @@ struct udscs_server *udscs_create_server_for_fd(int fd,
> return NULL;
> }
>
> - server = calloc(1, sizeof(*server));
> - if (!server)
> - return NULL;
> -
> + server = g_new0(struct udscs_server, 1);
> server->type_to_string = type_to_string;
> server->no_types = no_types;
> server->debug = debug;
> @@ -487,7 +467,7 @@ void udscs_destroy_server(struct udscs_server *server)
> conn = next_conn;
> }
> close(server->fd);
> - free(server);
> + g_free(server);
> }
>
> int udscs_get_peer_pid(struct udscs_connection *conn)
> @@ -509,13 +489,7 @@ static void udscs_server_accept(struct udscs_server *server) {
> return;
> }
>
> - new_conn = calloc(1, sizeof(*conn));
> - if (!new_conn) {
> - syslog(LOG_ERR, "out of memory, disconnecting new client");
> - close(fd);
> - return;
> - }
> -
> + new_conn = g_new0(struct udscs_connection, 1);
> new_conn->fd = fd;
> new_conn->type_to_string = server->type_to_string;
> new_conn->no_types = server->no_types;
> @@ -528,7 +502,7 @@ static void udscs_server_accept(struct udscs_server *server) {
> if (r != 0) {
> syslog(LOG_ERR, "Could not get peercred, disconnecting new client");
> close(fd);
> - free(new_conn);
> + g_free(new_conn);
> return;
> }
>
> diff --git a/src/vdagent/x11-randr.c b/src/vdagent/x11-randr.c
> index bfa1a02..fa52fba 100644
> --- a/src/vdagent/x11-randr.c
> +++ b/src/vdagent/x11-randr.c
> @@ -81,18 +81,15 @@ 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_clear_pointer(&x11->randr.outputs, g_free);
> }
> if (x11->randr.crtcs != NULL) {
> for (i = 0 ; i < x11->randr.res->ncrtc; ++i) {
> XRRFreeCrtcInfo(x11->randr.crtcs[i]);
> }
> - free(x11->randr.crtcs);
> + g_clear_pointer(&x11->randr.crtcs, g_free);
> }
> - XRRFreeScreenResources(x11->randr.res);
> - x11->randr.res = NULL;
> - x11->randr.outputs = NULL;
> - x11->randr.crtcs = NULL;
> + g_clear_pointer(&x11->randr.res, XRRFreeScreenResources);
> x11->randr.num_monitors = 0;
> }
>
> @@ -105,8 +102,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));
g_new() ?
> for (i = 0 ; i < x11->randr.res->noutput; ++i) {
> x11->randr.outputs[i] = XRRGetOutputInfo(x11->display, x11->randr.res,
> x11->randr.res->outputs[i]);
> @@ -659,11 +656,7 @@ static VDAgentMonitorsConfig *get_current_mon_config(struct vdagent_x11 *x11)
> XRRScreenResources *res = x11->randr.res;
> VDAgentMonitorsConfig *mon_config;
>
> - mon_config = calloc(1, config_size(res->noutput));
> - if (!mon_config) {
> - syslog(LOG_ERR, "out of memory allocating current monitor config");
> - return NULL;
> - }
> + mon_config = g_malloc0(config_size(res->noutput));
>
> for (i = 0 ; i < res->noutput; i++) {
> int j;
> @@ -696,7 +689,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;
> }
>
> @@ -843,12 +836,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));
This can be replaced by g_memdup()
> @@ -900,7 +893,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)
> @@ -919,11 +912,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;
> @@ -931,7 +920,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) {
> @@ -940,17 +929,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;
> @@ -964,9 +949,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];
> @@ -989,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/vdagent/x11.c b/src/vdagent/x11.c
> index 0ea5668..c3c7a65 100644
> --- a/src/vdagent/x11.c
> +++ b/src/vdagent/x11.c
> @@ -210,19 +210,14 @@ struct vdagent_x11 *vdagent_x11_create(struct udscs_connection *vdagentd,
> #endif
> gchar *net_wm_name = NULL;
>
> - x11 = calloc(1, sizeof(*x11));
> - if (!x11) {
> - syslog(LOG_ERR, "out of memory allocating vdagent_x11 struct");
> - return NULL;
> - }
> -
> + x11 = g_new0(struct vdagent_x11, 1);
> x11->vdagentd = vdagentd;
> x11->debug = debug;
>
> x11->display = XOpenDisplay(NULL);
> if (!x11->display) {
> syslog(LOG_ERR, "could not connect to X-server");
> - free(x11);
> + g_free(x11);
> return NULL;
> }
>
> @@ -231,7 +226,7 @@ struct vdagent_x11 *vdagent_x11_create(struct udscs_connection *vdagentd,
> syslog(LOG_ERR, "Error too much screens: %d > %d",
> x11->screen_count, MAX_SCREENS);
> XCloseDisplay(x11->display);
> - free(x11);
> + g_free(x11);
> return NULL;
> }
>
> @@ -345,8 +340,8 @@ void vdagent_x11_destroy(struct vdagent_x11 *x11, int vdagentd_disconnected)
> #endif
>
> XCloseDisplay(x11->display);
> - free(x11->randr.failed_conf);
> - free(x11);
> + g_free(x11->randr.failed_conf);
> + g_free(x11);
> }
>
> int vdagent_x11_get_fd(struct vdagent_x11 *x11)
> diff --git a/src/vdagentd/console-kit.c b/src/vdagentd/console-kit.c
> index 381b97e..c9da0b9 100644
> --- a/src/vdagentd/console-kit.c
> +++ b/src/vdagentd/console-kit.c
> @@ -71,8 +71,7 @@ static void si_dbus_match_remove(struct session_info *info)
> if (info->verbose)
> syslog(LOG_DEBUG, "(console-kit) seat match removed: %s",
> info->match_seat_signals);
> - g_free(info->match_seat_signals);
> - info->match_seat_signals = NULL;
> + g_clear_pointer(&info->match_seat_signals, g_free);
> }
>
> if (info->match_session_signals != NULL) {
> @@ -84,8 +83,7 @@ static void si_dbus_match_remove(struct session_info *info)
> if (info->verbose)
> syslog(LOG_DEBUG, "(console-kit) session match removed: %s",
> info->match_session_signals);
> - g_free(info->match_session_signals);
> - info->match_session_signals = NULL;
> + g_clear_pointer(&info->match_session_signals, g_free);
> }
> }
>
> @@ -117,8 +115,7 @@ static void si_dbus_match_rule_update(struct session_info *info)
> syslog(LOG_WARNING, "Unable to add dbus rule match: %s",
> error.message);
> dbus_error_free(&error);
> - g_free(info->match_seat_signals);
> - info->match_seat_signals = NULL;
> + g_clear_pointer(&info->match_seat_signals, g_free);
> }
> }
>
> @@ -140,8 +137,7 @@ static void si_dbus_match_rule_update(struct session_info *info)
> syslog(LOG_WARNING, "Unable to add dbus rule match: %s",
> error.message);
> dbus_error_free(&error);
> - g_free(info->match_session_signals);
> - info->match_session_signals = NULL;
> + g_clear_pointer(&info->match_session_signals, g_free);
> }
> }
> }
> @@ -162,8 +158,7 @@ si_dbus_read_signals(struct session_info *info)
> gint type;
> gchar *session;
>
> - free(info->active_session);
> - info->active_session = NULL;
> + g_clear_pointer(&info->active_session, g_free);
>
> dbus_message_iter_init(message, &iter);
> type = dbus_message_iter_get_arg_type(&iter);
> @@ -222,10 +217,7 @@ struct session_info *session_info_create(int verbose)
> struct session_info *info;
> DBusError error;
>
> - 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;
> @@ -239,7 +231,7 @@ struct session_info *session_info_create(int verbose)
> dbus_error_free(&error);
> } else
> syslog(LOG_ERR, "Unable to connect to system bus");
> - free(info);
> + g_free(info);
> return NULL;
> }
>
> @@ -265,9 +257,9 @@ void session_info_destroy(struct session_info *info)
>
> si_dbus_match_remove(info);
> dbus_connection_close(info->connection);
> - free(info->seat);
> - free(info->active_session);
> - free(info);
> + g_free(info->seat);
> + g_free(info->active_session);
> + g_free(info);
> }
>
> int session_info_get_fd(struct session_info *info)
> diff --git a/src/vdagentd/systemd-login.c b/src/vdagentd/systemd-login.c
> index 9719c0b..a20cd58 100644
> --- a/src/vdagentd/systemd-login.c
> +++ b/src/vdagentd/systemd-login.c
> @@ -83,8 +83,7 @@ static void si_dbus_match_remove(struct session_info *si)
> si->dbus.match_session_signals,
> &error);
>
> - g_free(si->dbus.match_session_signals);
> - si->dbus.match_session_signals = NULL;
> + g_clear_pointer(&si->dbus.match_session_signals, g_free);
> }
>
> static void si_dbus_match_rule_update(struct session_info *si)
> @@ -113,8 +112,7 @@ static void si_dbus_match_rule_update(struct session_info *si)
> syslog(LOG_WARNING, "Unable to add dbus rule match: %s",
> error.message);
> dbus_error_free(&error);
> - g_free(si->dbus.match_session_signals);
> - si->dbus.match_session_signals = NULL;
> + g_clear_pointer(&si->dbus.match_session_signals, g_free);
> }
> }
>
> @@ -230,17 +228,14 @@ struct session_info *session_info_create(int verbose)
> 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;
>
> 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;
> }
>
> @@ -256,8 +251,8 @@ void session_info_destroy(struct session_info *si)
> si_dbus_match_remove(si);
> dbus_connection_close(si->dbus.system_connection);
> sd_login_monitor_unref(si->mon);
> - free(si->session);
> - free(si);
> + g_free(si->session);
> + g_free(si);
> }
>
> int session_info_get_fd(struct session_info *si)
> @@ -282,7 +277,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_match_rule_update(si);
> return si->session;
> diff --git a/src/vdagentd/uinput.c b/src/vdagentd/uinput.c
> index 6d04322..3172891 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 adfaf39..1ac30ee 100644
> --- a/src/vdagentd/vdagentd.c
> +++ b/src/vdagentd/vdagentd.c
> @@ -124,12 +124,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);
> @@ -145,7 +140,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)
> @@ -198,12 +193,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);
I don't see much point for the if() above. That's just to save
alloc operation but this is not hot path.
Should be fine to replace the if() and memcpy() with
g_free(mon_config);
mon_config = g_memdup(new_monitors, size);
Feel free to do the suggestion above in a separate patch or in
this one as it kinda relate to moving to g_memdup()
> @@ -240,13 +231,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));
Ditto :)
> if (caps->request) {
> @@ -328,7 +314,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)
> @@ -342,7 +328,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,
> @@ -849,13 +835,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);
> @@ -874,12 +854,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,
> @@ -908,12 +887,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));
g_memdup() too
Looks fine otherwise,
Reviewed-by: Victor Toso <victortoso@xxxxxxxxxx>
> agent_data->width = header->arg1;
> agent_data->height = header->arg2;
> @@ -1177,8 +1152,6 @@ int main(int argc, char *argv[])
> if (errno == EADDRINUSE) {
> syslog(LOG_CRIT, "Fatal the server socket %s exists already. Delete it?",
> vdagentd_socket);
> - } else if (errno == ENOMEM) {
> - syslog(LOG_CRIT, "Fatal could not allocate memory for udscs server");
> } else {
> syslog(LOG_CRIT, "Fatal could not create the server socket %s: %m",
> vdagentd_socket);
> diff --git a/src/vdagentd/virtio-port.c b/src/vdagentd/virtio-port.c
> index 3dc6f44..1fac1db 100644
> --- a/src/vdagentd/virtio-port.c
> +++ b/src/vdagentd/virtio-port.c
> @@ -85,9 +85,7 @@ struct vdagent_virtio_port *vdagent_virtio_port_create(const char *portname,
> struct sockaddr_un address;
> int c;
>
> - vport = calloc(1, sizeof(*vport));
> - if (!vport)
> - return 0;
> + vport = g_new0(struct vdagent_virtio_port, 1);
>
> vport->fd = open(portname, O_RDWR);
> if (vport->fd == -1) {
> @@ -118,7 +116,7 @@ error:
> if (vport->fd != -1) {
> close(vport->fd);
> }
> - free(vport);
> + g_free(vport);
> return NULL;
> }
>
> @@ -137,18 +135,17 @@ void vdagent_virtio_port_destroy(struct vdagent_virtio_port **vportp)
> wbuf = vport->write_buf;
> while (wbuf) {
> next_wbuf = wbuf->next;
> - free(wbuf->buf);
> - free(wbuf);
> + g_free(wbuf->buf);
> + g_free(wbuf);
> wbuf = next_wbuf;
> }
>
> for (i = 0; i < VDP_END_PORT; i++) {
> - free(vport->port_data[i].message_data);
> + g_free(vport->port_data[i].message_data);
> }
>
> close(vport->fd);
> - free(vport);
> - *vportp = NULL;
> + g_clear_pointer(vportp, g_free);
> }
>
> int vdagent_virtio_port_fill_fds(struct vdagent_virtio_port *vport,
> @@ -203,19 +200,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->pos = 0;
> new_wbuf->write_pos = 0;
> new_wbuf->size = sizeof(chunk_header) + sizeof(message_header) + data_size;
> new_wbuf->next = NULL;
> - new_wbuf->buf = malloc(new_wbuf->size);
> - if (!new_wbuf->buf) {
> - free(new_wbuf);
> - return -1;
> - }
> + new_wbuf->buf = g_malloc(new_wbuf->size);
>
> chunk_header.port = GUINT32_TO_LE(port_nr);
> chunk_header.size = GUINT32_TO_LE(sizeof(message_header) + data_size);
> @@ -291,7 +281,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]));
> }
>
> @@ -318,12 +308,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");
> - vdagent_virtio_port_destroy(vportp);
> - return;
> - }
> + port->message_data = g_malloc(port->message_header.size);
> }
> }
> pos = read;
> @@ -359,8 +344,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);
> }
> }
> }
> @@ -496,7 +480,7 @@ static void vdagent_virtio_port_do_write(struct vdagent_virtio_port **vportp)
> wbuf->pos += n;
> if (wbuf->pos == wbuf->size) {
> vport->write_buf = wbuf->next;
> - free(wbuf->buf);
> - free(wbuf);
> + g_free(wbuf->buf);
> + g_free(wbuf);
> }
> }
> --
> 2.17.1
>
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel