[Spice-devel] [PATCH spice-server 01/11] Start using GLib memory allocation
Jonathon Jongsma
jjongsma at redhat.com
Mon Sep 18 21:06:59 UTC 2017
On Mon, 2017-09-11 at 16:15 +0100, Frediano Ziglio wrote:
> Start reducing the usage of spice_new*/spice_malloc allocations.
> They were designed in a similar way to GLib ones.
> Now that we use GLib make sense to remove them.
> However the versions we support for GLib can use different memory
> allocators so we have to match g_free with GLib allocations
> and spice_* ones (which uses always malloc allocator) with free().
> This patch remove some easy ones.
>
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
> server/common-graphics-channel.c | 4 ++--
> server/dcc-send.c | 4 ++--
> server/display-channel.c | 6 +++---
> server/jpeg-encoder.c | 8 ++++----
> server/lz4-encoder.c | 4 ++--
> server/main-channel-client.c | 14 +++++++-------
> server/memslot.c | 8 ++++----
> server/red-channel-capabilities.c | 14 +++++++-------
> server/red-qxl.c | 4 ++--
> server/red-worker.c | 8 ++++----
> server/reds.c | 2 +-
> server/smartcard-channel-client.c | 4 ++--
> server/stat-file.c | 7 +++----
> server/stream.c | 12 ++++++------
> server/zlib-encoder.c | 6 +++---
> 15 files changed, 52 insertions(+), 53 deletions(-)
>
> diff --git a/server/common-graphics-channel.c b/server/common-
> graphics-channel.c
> index a664f05d4..0cbc2762c 100644
> --- a/server/common-graphics-channel.c
> +++ b/server/common-graphics-channel.c
> @@ -56,7 +56,7 @@ static uint8_t
> *common_alloc_recv_buf(RedChannelClient *rcc, uint16_t type, uint
>
> /* SPICE_MSGC_MIGRATE_DATA is the only client message whose size
> is dynamic */
> if (type == SPICE_MSGC_MIGRATE_DATA) {
> - return spice_malloc(size);
> + return g_malloc(size);
> }
>
> if (size > CHANNEL_RECEIVE_BUF_SIZE) {
> @@ -70,7 +70,7 @@ static void
> common_release_recv_buf(RedChannelClient *rcc, uint16_t type, uint32
> uint8_t* msg)
> {
> if (type == SPICE_MSGC_MIGRATE_DATA) {
> - free(msg);
> + g_free(msg);
> }
> }
>
> diff --git a/server/dcc-send.c b/server/dcc-send.c
> index 18e226f0d..8692cff2c 100644
> --- a/server/dcc-send.c
> +++ b/server/dcc-send.c
> @@ -2280,7 +2280,7 @@ static void
> marshall_monitors_config(RedChannelClient *rcc, SpiceMarshaller *bas
> {
> int heads_size = sizeof(SpiceHead) * monitors_config->count;
> int i;
> - SpiceMsgDisplayMonitorsConfig *msg = spice_malloc0(sizeof(*msg)
> + heads_size);
> + SpiceMsgDisplayMonitorsConfig *msg = g_malloc0(sizeof(*msg) +
> heads_size);
> int count = 0; // ignore monitors_config->count, it may contain
> zero width monitors, remove them now
>
> red_channel_client_init_send_data(rcc,
> SPICE_MSG_DISPLAY_MONITORS_CONFIG);
> @@ -2299,7 +2299,7 @@ static void
> marshall_monitors_config(RedChannelClient *rcc, SpiceMarshaller *bas
> msg->count = count;
> msg->max_allowed = monitors_config->max_allowed;
> spice_marshall_msg_display_monitors_config(base_marshaller,
> msg);
> - free(msg);
> + g_free(msg);
> }
>
> static void marshall_stream_activate_report(RedChannelClient *rcc,
> diff --git a/server/display-channel.c b/server/display-channel.c
> index f7e36dbb4..fbdca265f 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -2014,7 +2014,7 @@ static void region_to_qxlrects(QRegion *region,
> QXLRect *qxl_rects, uint32_t num
> SpiceRect *rects;
> int i;
>
> - rects = spice_new0(SpiceRect, num_rects);
> + rects = g_new0(SpiceRect, num_rects);
> region_ret_rects(region, rects, num_rects);
> for (i = 0; i < num_rects; i++) {
> qxl_rects[i].top = rects[i].top;
> @@ -2022,7 +2022,7 @@ static void region_to_qxlrects(QRegion *region,
> QXLRect *qxl_rects, uint32_t num
> qxl_rects[i].bottom = rects[i].bottom;
> qxl_rects[i].right = rects[i].right;
> }
> - free(rects);
> + g_free(rects);
> }
>
> void display_channel_update(DisplayChannel *display,
> @@ -2040,7 +2040,7 @@ void display_channel_update(DisplayChannel
> *display,
> surface = &display->priv->surfaces[surface_id];
> if (*qxl_dirty_rects == NULL) {
> *num_dirty_rects = pixman_region32_n_rects(&surface-
> >draw_dirty_region);
> - *qxl_dirty_rects = spice_new0(QXLRect, *num_dirty_rects);
> + *qxl_dirty_rects = g_new0(QXLRect, *num_dirty_rects);
> }
>
> region_to_qxlrects(&surface->draw_dirty_region,
> *qxl_dirty_rects, *num_dirty_rects);
> diff --git a/server/jpeg-encoder.c b/server/jpeg-encoder.c
> index 5c5edf143..a6eaf86f4 100644
> --- a/server/jpeg-encoder.c
> +++ b/server/jpeg-encoder.c
> @@ -84,7 +84,7 @@ JpegEncoderContext*
> jpeg_encoder_create(JpegEncoderUsrContext *usr)
> return NULL;
> }
>
> - enc = spice_new0(JpegEncoder, 1);
> + enc = g_new0(JpegEncoder, 1);
>
> enc->usr = usr;
>
> @@ -103,7 +103,7 @@ JpegEncoderContext*
> jpeg_encoder_create(JpegEncoderUsrContext *usr)
> void jpeg_encoder_destroy(JpegEncoderContext* encoder)
> {
> jpeg_destroy_compress(&((JpegEncoder*)encoder)->cinfo);
> - free(encoder);
> + g_free(encoder);
> }
>
> static void convert_RGB16_to_RGB24(void *line, int width, uint8_t
> **out_line)
> @@ -185,7 +185,7 @@ static void do_jpeg_encode(JpegEncoder *jpeg,
> uint8_t *lines, unsigned int num_l
> stride = jpeg->cur_image.stride;
>
> if (jpeg->cur_image.type != JPEG_IMAGE_TYPE_RGB24) {
> - RGB24_line = (uint8_t *)spice_malloc(width*3);
> + RGB24_line = g_new(uint8_t, width*3);
> }
>
> lines_end = lines + (stride * num_lines);
> @@ -198,7 +198,7 @@ static void do_jpeg_encode(JpegEncoder *jpeg,
> uint8_t *lines, unsigned int num_l
> }
>
> if (jpeg->cur_image.type != JPEG_IMAGE_TYPE_RGB24) {
> - free(RGB24_line);
> + g_free(RGB24_line);
> }
> }
>
> diff --git a/server/lz4-encoder.c b/server/lz4-encoder.c
> index 104834903..fe736d272 100644
> --- a/server/lz4-encoder.c
> +++ b/server/lz4-encoder.c
> @@ -34,7 +34,7 @@ Lz4EncoderContext*
> lz4_encoder_create(Lz4EncoderUsrContext *usr)
> return NULL;
> }
>
> - enc = spice_new0(Lz4Encoder, 1);
> + enc = g_new0(Lz4Encoder, 1);
> enc->usr = usr;
>
> return (Lz4EncoderContext*)enc;
> @@ -42,7 +42,7 @@ Lz4EncoderContext*
> lz4_encoder_create(Lz4EncoderUsrContext *usr)
>
> void lz4_encoder_destroy(Lz4EncoderContext* encoder)
> {
> - free(encoder);
> + g_free(encoder);
> }
>
> int lz4_encode(Lz4EncoderContext *lz4, int height, int stride,
> uint8_t *io_ptr,
> diff --git a/server/main-channel-client.c b/server/main-channel-
> client.c
> index 4abab6c6b..b7b60eddb 100644
> --- a/server/main-channel-client.c
> +++ b/server/main-channel-client.c
> @@ -238,17 +238,17 @@ static bool
> main_channel_client_push_ping(MainChannelClient *mcc, int size);
> static void main_notify_item_free(RedPipeItem *base)
> {
> RedNotifyPipeItem *data = SPICE_UPCAST(RedNotifyPipeItem, base);
> - free(data->msg);
> - free(data);
> + g_free(data->msg);
> + g_free(data);
> }
>
> static RedPipeItem *main_notify_item_new(const char *msg, int num)
> {
> - RedNotifyPipeItem *item =
> spice_malloc(sizeof(RedNotifyPipeItem));
> + RedNotifyPipeItem *item = g_new(RedNotifyPipeItem, 1);
>
> red_pipe_item_init_full(&item->base,
> RED_PIPE_ITEM_TYPE_MAIN_NOTIFY,
> main_notify_item_free);
> - item->msg = spice_strdup(msg);
> + item->msg = g_strdup(msg);
> return &item->base;
> }
>
> @@ -314,14 +314,14 @@ static void
> main_agent_data_item_free(RedPipeItem *base)
> {
> RedAgentDataPipeItem *item = SPICE_UPCAST(RedAgentDataPipeItem,
> base);
> item->free_data(item->data, item->opaque);
> - free(item);
> + g_free(item);
> }
>
> static RedPipeItem *main_agent_data_item_new(uint8_t* data, size_t
> len,
> spice_marshaller_item_f
> ree_func free_data,
> void *opaque)
> {
> - RedAgentDataPipeItem *item =
> spice_malloc(sizeof(RedAgentDataPipeItem));
> + RedAgentDataPipeItem *item = g_new(RedAgentDataPipeItem, 1);
>
> red_pipe_item_init_full(&item->base,
> RED_PIPE_ITEM_TYPE_MAIN_AGENT_DATA,
> main_agent_data_item_free);
> @@ -722,7 +722,7 @@ static void
> main_channel_marshall_channels(RedChannelClient *rcc,
> red_channel_client_init_send_data(rcc,
> SPICE_MSG_MAIN_CHANNELS_LIST);
> channels_info =
> reds_msg_channels_new(red_channel_get_server(channel));
> spice_marshall_msg_main_channels_list(m, channels_info);
> - free(channels_info);
> + g_free(channels_info);
I'd personally prefer to add a new reds_msg_channels_free() function in
reds.c as a partner for reds_msg_channels_new(). This way the
allocation and free implementation is maintained in the same spot so we
don't have so that it's obvious if there's a mismatch.
> }
>
> static void main_channel_marshall_ping(RedChannelClient *rcc,
> diff --git a/server/memslot.c b/server/memslot.c
> index fdcd02325..7074b4328 100644
> --- a/server/memslot.c
> +++ b/server/memslot.c
> @@ -152,10 +152,10 @@ void memslot_info_init(RedMemSlotInfo *info,
> info->mem_slot_bits = id_bits;
> info->internal_groupslot_id = internal_groupslot_id;
>
> - info->mem_slots = spice_new(MemSlot *, num_groups);
> + info->mem_slots = g_new(MemSlot *, num_groups);
>
> for (i = 0; i < num_groups; ++i) {
> - info->mem_slots[i] = spice_new0(MemSlot, num_slots);
> + info->mem_slots[i] = g_new0(MemSlot, num_slots);
> }
>
> /* TODO: use QXLPHYSICAL_BITS */
> @@ -171,9 +171,9 @@ void memslot_info_destroy(RedMemSlotInfo *info)
> uint32_t i;
>
> for (i = 0; i < info->num_memslots_groups; ++i) {
> - free(info->mem_slots[i]);
> + g_free(info->mem_slots[i]);
> }
> - free(info->mem_slots);
> + g_free(info->mem_slots);
> }
>
> void memslot_info_add_slot(RedMemSlotInfo *info, uint32_t
> slot_group_id, uint32_t slot_id,
> diff --git a/server/red-channel-capabilities.c b/server/red-channel-
> capabilities.c
> index 5a3984ab7..2113c6e9a 100644
> --- a/server/red-channel-capabilities.c
> +++ b/server/red-channel-capabilities.c
> @@ -31,24 +31,24 @@ void
> red_channel_capabilities_init(RedChannelCapabilities *dest,
> {
> *dest = *caps;
> if (caps->common_caps) {
> - dest->common_caps = spice_memdup(caps->common_caps,
> - caps->num_common_caps *
> sizeof(uint32_t));
> + dest->common_caps = g_memdup(caps->common_caps,
> + caps->num_common_caps *
> sizeof(uint32_t));
> }
> if (caps->num_caps) {
> - dest->caps = spice_memdup(caps->caps, caps->num_caps *
> sizeof(uint32_t));
> + dest->caps = g_memdup(caps->caps, caps->num_caps *
> sizeof(uint32_t));
> }
> }
>
> void red_channel_capabilities_reset(RedChannelCapabilities *caps)
> {
> - free(caps->common_caps);
> - free(caps->caps);
> + g_free(caps->common_caps);
> + g_free(caps->caps);
> memset(caps, 0, sizeof(*caps));
> }
>
> static RedChannelCapabilities *red_channel_capabilities_dup(const
> RedChannelCapabilities *caps)
> {
> - RedChannelCapabilities *res = spice_new(RedChannelCapabilities,
> 1);
> + RedChannelCapabilities *res = g_new(RedChannelCapabilities, 1);
> red_channel_capabilities_init(res, caps);
> return res;
> }
> @@ -56,7 +56,7 @@ static RedChannelCapabilities
> *red_channel_capabilities_dup(const RedChannelCapa
> static void red_channel_capabilities_free(RedChannelCapabilities
> *caps)
> {
> red_channel_capabilities_reset(caps);
> - free(caps);
> + g_free(caps);
> }
>
> SPICE_CONSTRUCTOR_FUNC(red_channel_capabilities_construct)
> diff --git a/server/red-qxl.c b/server/red-qxl.c
> index 4ef293060..0df93a82a 100644
> --- a/server/red-qxl.c
> +++ b/server/red-qxl.c
> @@ -934,7 +934,7 @@ void red_qxl_init(RedsState *reds, QXLInstance
> *qxl)
>
> spice_return_if_fail(qxl != NULL);
>
> - qxl_state = spice_new0(QXLState, 1);
> + qxl_state = g_new0(QXLState, 1);
> qxl_state->reds = reds;
> qxl_state->qxl = qxl;
> pthread_mutex_init(&qxl_state->scanout_mutex, NULL);
> @@ -994,7 +994,7 @@ void red_qxl_destroy(QXLInstance *qxl)
> /* this must be done after calling red_worker_free */
> qxl->st = NULL;
> pthread_mutex_destroy(&qxl_state->scanout_mutex);
> - free(qxl_state);
> + g_free(qxl_state);
> }
>
> Dispatcher *red_qxl_get_dispatcher(QXLInstance *qxl)
> diff --git a/server/red-worker.c b/server/red-worker.c
> index 58d9e0add..dabcbec37 100644
> --- a/server/red-worker.c
> +++ b/server/red-worker.c
> @@ -431,7 +431,7 @@ static void handle_dev_update_async(void *opaque,
> void *payload)
>
> red_qxl_update_area_complete(worker->qxl, msg->surface_id,
> qxl_dirty_rects, num_dirty_rects);
> - free(qxl_dirty_rects);
> + g_free(qxl_dirty_rects);
> red_qxl_async_complete(worker->qxl, msg->base.cmd);
> }
>
> @@ -448,7 +448,7 @@ static void handle_dev_update(void *opaque, void
> *payload)
> msg->surface_id, msg->qxl_area, msg-
> >clear_dirty_region,
> &qxl_dirty_rects, &msg->num_dirty_rects);
> if (msg->qxl_dirty_rects == NULL) {
> - free(qxl_dirty_rects);
> + g_free(qxl_dirty_rects);
> }
> }
>
> @@ -1294,7 +1294,7 @@ RedWorker* red_worker_new(QXLInstance *qxl,
>
> red_qxl_get_init_info(qxl, &init_info);
>
> - worker = spice_new0(RedWorker, 1);
> + worker = g_new0(RedWorker, 1);
> worker->core = event_loop_core;
> worker->core.main_context = g_main_context_new();
>
> @@ -1434,5 +1434,5 @@ void red_worker_free(RedWorker *worker)
> red_record_unref(worker->record);
> }
> memslot_info_destroy(&worker->mem_slots);
> - free(worker);
> + g_free(worker);
> }
> diff --git a/server/reds.c b/server/reds.c
> index 24ec2bdde..01e8c5499 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -1004,7 +1004,7 @@ SpiceMsgChannels
> *reds_msg_channels_new(RedsState *reds)
>
> spice_assert(reds != NULL);
>
> - channels_info = (SpiceMsgChannels
> *)spice_malloc(sizeof(SpiceMsgChannels)
> + channels_info = (SpiceMsgChannels
> *)g_malloc(sizeof(SpiceMsgChannels)
> + g_list_length(reds->channels) *
> sizeof(SpiceChannelId));
>
> reds_fill_channels(reds, channels_info);
> diff --git a/server/smartcard-channel-client.c b/server/smartcard-
> channel-client.c
> index 7a2ef0dd8..07d451f47 100644
> --- a/server/smartcard-channel-client.c
> +++ b/server/smartcard-channel-client.c
> @@ -135,7 +135,7 @@
> smartcard_channel_client_alloc_msg_rcv_buf(RedChannelClient *rcc,
> * differenc channels */
> if (!scc->priv->smartcard) {
> scc->priv->msg_in_write_buf = FALSE;
> - return spice_malloc(size);
> + return g_malloc(size);
> } else {
> RedCharDeviceSmartcard *smartcard;
>
> @@ -168,7 +168,7 @@
> smartcard_channel_client_release_msg_rcv_buf(RedChannelClient *rcc,
>
> if (!scc->priv->msg_in_write_buf) {
> spice_assert(!scc->priv->write_buf);
> - free(msg);
> + g_free(msg);
> } else {
> if (scc->priv->write_buf) { /* msg hasn't been pushed to the
> guest */
> spice_assert(scc->priv->write_buf->buf == msg);
> diff --git a/server/stat-file.c b/server/stat-file.c
> index 96c3bc18e..2797fd739 100644
> --- a/server/stat-file.c
> +++ b/server/stat-file.c
> @@ -29,7 +29,6 @@
> #include <sys/mman.h>
> #include <spice/stats.h>
> #include <common/log.h>
> -#include <common/mem.h>
>
> #include "stat-file.h"
>
> @@ -47,7 +46,7 @@ RedStatFile *stat_file_new(unsigned int max_nodes)
> {
> int fd;
> size_t shm_size = STAT_SHM_SIZE(max_nodes);
> - RedStatFile *stat_file = spice_new0(RedStatFile, 1);
> + RedStatFile *stat_file = g_new0(RedStatFile, 1);
>
> stat_file->max_nodes = max_nodes;
> stat_file->shm_name = g_strdup_printf(SPICE_STAT_SHM_NAME,
> getpid());
> @@ -78,7 +77,7 @@ RedStatFile *stat_file_new(unsigned int max_nodes)
> return stat_file;
>
> cleanup:
> - free(stat_file);
> + g_free(stat_file);
> return NULL;
> }
>
> @@ -96,7 +95,7 @@ void stat_file_free(RedStatFile *stat_file)
> #endif
>
> pthread_mutex_destroy(&stat_file->lock);
> - free(stat_file);
> + g_free(stat_file);
> }
>
> const char *stat_file_get_shm_name(RedStatFile *stat_file)
> diff --git a/server/stream.c b/server/stream.c
> index 0148d1eda..ddc5232e1 100644
> --- a/server/stream.c
> +++ b/server/stream.c
> @@ -70,12 +70,12 @@ static void
> stream_create_destroy_item_release(RedPipeItem *base)
> StreamCreateDestroyItem *item =
> SPICE_UPCAST(StreamCreateDestroyItem, base);
> DisplayChannel *display = DCC_TO_DC(item->agent->dcc);
> stream_agent_unref(display, item->agent);
> - free(item);
> + g_free(item);
> }
>
> static RedPipeItem *stream_create_destroy_item_new(StreamAgent
> *agent, gint type)
> {
> - StreamCreateDestroyItem *item =
> spice_new0(StreamCreateDestroyItem, 1);
> + StreamCreateDestroyItem *item = g_new0(StreamCreateDestroyItem,
> 1);
>
> red_pipe_item_init_full(&item->base, type,
> stream_create_destroy_item_release);
> @@ -172,12 +172,12 @@ static void
> red_stream_clip_item_free(RedPipeItem *base)
>
> stream_agent_unref(display, item->stream_agent);
> free(item->rects);
> - free(item);
> + g_free(item);
> }
>
> RedStreamClipItem *red_stream_clip_item_new(StreamAgent *agent)
> {
> - RedStreamClipItem *item = spice_new(RedStreamClipItem, 1);
> + RedStreamClipItem *item = g_new(RedStreamClipItem, 1);
> red_pipe_item_init_full(&item->base,
> RED_PIPE_ITEM_TYPE_STREAM_CLIP,
> red_stream_clip_item_free);
>
> @@ -780,7 +780,7 @@ static void red_upgrade_item_free(RedPipeItem
> *base)
>
> drawable_unref(item->drawable);
> free(item->rects);
> - free(item);
> + g_free(item);
> }
>
> /*
> @@ -821,7 +821,7 @@ static void
> dcc_detach_stream_gracefully(DisplayChannelClient *dcc,
> spice_debug("stream %d: upgrade by drawable. box ==>",
> stream_id);
> rect_debug(&stream->current->red_drawable->bbox);
> rcc = RED_CHANNEL_CLIENT(dcc);
> - upgrade_item = spice_new(RedUpgradeItem, 1);
> + upgrade_item = g_new(RedUpgradeItem, 1);
> red_pipe_item_init_full(&upgrade_item->base,
> RED_PIPE_ITEM_TYPE_UPGRADE,
> red_upgrade_item_free);
> upgrade_item->drawable = stream->current;
> diff --git a/server/zlib-encoder.c b/server/zlib-encoder.c
> index 1579ed784..7b98abbb3 100644
> --- a/server/zlib-encoder.c
> +++ b/server/zlib-encoder.c
> @@ -40,7 +40,7 @@ ZlibEncoder*
> zlib_encoder_create(ZlibEncoderUsrContext *usr, int level)
> return NULL;
> }
>
> - enc = spice_new0(ZlibEncoder, 1);
> + enc = g_new0(ZlibEncoder, 1);
>
> enc->usr = usr;
>
> @@ -52,7 +52,7 @@ ZlibEncoder*
> zlib_encoder_create(ZlibEncoderUsrContext *usr, int level)
> enc->last_level = level;
> if (z_ret != Z_OK) {
> spice_printerr("zlib error");
> - free(enc);
> + g_free(enc);
> return NULL;
> }
>
> @@ -62,7 +62,7 @@ ZlibEncoder*
> zlib_encoder_create(ZlibEncoderUsrContext *usr, int level)
> void zlib_encoder_destroy(ZlibEncoder *encoder)
> {
> deflateEnd(&encoder->strm);
> - free(encoder);
> + g_free(encoder);
> }
>
> /* returns the total size of the encoded data */
Aside from the minor suggestion above, looks fine
Aked-by: Jonathon Jongsma <jjongsma at redhat.com>
More information about the Spice-devel
mailing list