[Spice-devel] [RFC spice-vdagent 16/18] move to GLib memory functions
Jakub Janků
jjanku at redhat.com
Tue Aug 14 18:53:50 UTC 2018
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)
Use g_clear_pointer where possible
for simplification.
---
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;
}
@@ -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
More information about the Spice-devel
mailing list