> On 20 Sep 2017, at 11:53, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote: > >>> >>> On 20 Sep 2017, at 09:50, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote: >>> >>> Reduce 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(). >>> >>> The final target is to remove spice_new*/spice_malloc* >>> functions/helper from spice-common. >> >> I am a bit ambivalent about this kind of source-level replacement. Why not >> simply #define spice_malloc to glib_malloc? >> > > Does not work as you need to change free according where needed. But you need to change free calls in your patch too, right? > Unless you add a spice_free too (and change required free calls). Well, if we change things, I’d rather change them in that direction, yes. > But the spice_XXX functions were designed like GLib so would cause some > confusion having 2 sets of functions potentially using different pools. This is a problem you already need to address in your patch, if I’m not mistaken. > >> The benefit of doing it that way (in addition to requiring less source code >> changes and making following rebases or merge much easier) is that it leaves >> the option to instrument spice allocations specifically when the need >> arises. >> > > There are many tools to instruments memory allocations and is not hard > to write one on your own. For instance knowing that objects file takes > precedence over libraries you can write a module defining malloc, or use > --wrap linker option or LD_PRELOAD. That works if you want to instrument all malloc calls. If you want to do something specific to spice, you can’t do that. > >>> >>> This series finish changing all allocations from spice_XXX >>> to Glib memory functions (tests excluded). >>> >>> You will notice some patches have changes only in allocation part >>> without any free. This means code has leaks, not a regression. >>> >>> If you check the code there is still a spice_malloc_n call in >>> display-channel.c. This is required as the memory is freed by >>> spice-common with a normal free. >>> >>> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx> >>> >>> Frediano Ziglio (23): >>> lz4-encoder: Use GLib memory functions >>> mjpeg: Use GLib memory functions >>> reds-stream: Use GLib memory functions >>> smartcard: Use GLib memory functions >>> stream: Use GLib memory functions >>> spicevmc: Use GLib memory functions >>> gstreamer-encoder: Use GLib memory functions >>> char-device: Use GLib memory functions >>> event-loop: Use GLib memory functions >>> dispatcher: Use GLib memory functions >>> Use GLib memory functions for SpiceChannelEventInfo >>> reds: Use GLib memory functions for link message >>> reds: Use GLib memory functions for ChannelSecurityOptions >>> dcc: Use GLib memory functions >>> display-channel: Use GLib memory functions >>> replay-qxl: Use GLib memory functions >>> worker: Use GLib memory functions >>> tree: Use GLib memory functions >>> inputs-channel: Use GLib memory functions >>> image-cache: Use GLib memory functions >>> pixmap-cache: Use GLib memory functions >>> parse-qxl: Use GLib memory functions >>> red-pipe-item: Use GLib memory functions >>> >>> server/cache-item.tmpl.c | 6 ++--- >>> server/char-device.c | 12 +++++----- >>> server/cursor-channel.c | 2 +- >>> server/dcc.c | 40 ++++++++++++++++---------------- >>> server/dispatcher.c | 4 ++-- >>> server/display-channel.c | 6 ++--- >>> server/event-loop.c | 8 +++---- >>> server/gstreamer-encoder.c | 16 ++++++------- >>> server/image-cache.c | 4 ++-- >>> server/inputs-channel.c | 10 ++++---- >>> server/lz4-encoder.c | 8 +++---- >>> server/main-channel-client.c | 14 ++++++------ >>> server/mjpeg-encoder.c | 28 +++++++++++------------ >>> server/pixmap-cache.c | 6 ++--- >>> server/red-channel-client.c | 8 +++---- >>> server/red-channel.c | 2 +- >>> server/red-parse-qxl.c | 46 >>> ++++++++++++++++++------------------- >>> server/red-pipe-item.c | 2 +- >>> server/red-replay-qxl.c | 48 >>> +++++++++++++++++++-------------------- >>> server/red-worker.c | 8 +++---- >>> server/reds-stream.c | 39 ++++++++++++------------------- >>> server/reds.c | 14 ++++++------ >>> server/smartcard-channel-client.c | 2 +- >>> server/smartcard.c | 14 ++++++------ >>> server/spicevmc.c | 24 ++++++++++---------- >>> server/stream.c | 8 +++---- >>> server/tree.c | 8 +++---- >>> 27 files changed, 189 insertions(+), 198 deletions(-) _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel