On Wed, 2016-06-29 at 11:42 -0400, Frediano Ziglio wrote: > > > > > > Hi Frediano, > > > > On Wed, 2016-06-22 at 16:19 +0100, Frediano Ziglio wrote: > > > > > > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx> > > > --- > > > server/image-encoders.c | 29 ++++++++++++++++++++++++++++- > > > 1 file changed, 28 insertions(+), 1 deletion(-) > > > > > > diff --git a/server/image-encoders.c b/server/image-encoders.c > > > index 0ade8c8..c2d7648 100644 > > > --- a/server/image-encoders.c > > > +++ b/server/image-encoders.c > > > @@ -20,12 +20,15 @@ > > > #endif > > > > > > #include <glib.h> > > > +#include <common/log.h> > > > > > > #include "image-encoders.h" > > > #include "spice-bitmap-utils.h" > > > #include "red-worker.h" // red_drawable_unref > > > #include "pixmap-cache.h" // MAX_CACHE_CLIENTS > > > > > > +#define GLZ_DEBUG 0 > > what about not defining GLZ_DEBUG, that way one could simply enable these > > asserts by compiling with -DGLZ_DEBUG ? > > > > Pavel > > > > Make perfect sense! > > We should document somewhere all that macros to turn some part of > debugging. We can document them & show their usage in some script. I was thinking about having a script for "compile" testing these defines and their combinations (in the past we were breaking them) like: https://paste.fedoraproject.org/386571/14672756 > One related question is in > https://lists.freedesktop.org/archives/spice-devel/2016-May/029519.html > Well I think it is ok to have a configure option, or a define like for these debugs. imo a configure option is easier to use Pavel > Frediano > > > > > > > > > + > > > #define ZLIB_DEFAULT_COMPRESSION_LEVEL 3 > > > > > > #define ENCODER_MESSAGE_SIZE 512 > > > @@ -66,6 +69,23 @@ static void encoder_data_init(EncoderData *data); > > > static void encoder_data_reset(EncoderData *data); > > > static void image_encoders_release_glz(ImageEncoders *enc); > > > > > > +#if GLZ_DEBUG > > > +static void glz_dict_item_check(GlzDictItem *glz_item) > > > +{ > > > + spice_assert(glz_item); > > > + spice_assert(ring_item_is_linked(&glz_item->glz_context_link)); > > > + spice_assert(ring_item_is_linked(&glz_item->retention_link)); > > > + spice_assert(glz_item->enc); > > > + spice_assert(glz_item->red_drawable); > > > + if (ring_item_is_linked(&glz_item->free_link)) { > > > + spice_assert(glz_item->dict_image_context == NULL); > > > + } > > > +} > > > +#else > > > +static inline void glz_dict_item_check(GlzDictItem *glz_item) > > mark parameter as unused > > > > > > +{ > > > +} > > > +#endif > > > > > > static SPICE_GNUC_NORETURN SPICE_GNUC_PRINTF(2, 3) void > > > quic_usr_error(QuicUsrContext *usr, const char *fmt, ...) > > > @@ -371,6 +391,7 @@ static void glz_usr_free_image(GlzEncoderUsrContext > > > *usr, > > > GlzUsrImageContext *im > > > GlzDictItem *glz_item = (GlzDictItem *)image; > > > ImageEncoders *drawable_enc = glz_item->enc; > > > ImageEncoders *this_enc = SPICE_CONTAINEROF(lz_data, ImageEncoders, > > > glz_data); > > > + glz_dict_item_check(glz_item); > > > glz_item->dict_image_context = NULL; > > > if (this_enc == drawable_enc) { > > > glz_context_free(glz_item); > > > @@ -386,6 +407,7 @@ static void glz_usr_free_image(GlzEncoderUsrContext > > > *usr, > > > GlzUsrImageContext *im > > > ring_add_before(&glz_item->free_link, > > > &drawable_enc->glz_dict_items_to_free); > > > pthread_mutex_unlock(&drawable_enc->glz_dict_items_to_free_lock); > > > + glz_dict_item_check(glz_item); > > > } > > > } > > > > > > @@ -487,7 +509,7 @@ static gboolean glz_context_free(GlzDictItem > > > *glz_item) > > > { > > > gboolean ret; > > > > > > - spice_assert(glz_item); > > > + glz_dict_item_check(glz_item); > > > > > > ring_remove(&glz_item->glz_context_link); > > > ring_remove(&glz_item->retention_link); > > > @@ -529,6 +551,7 @@ int > > > image_encoders_free_some_independent_glz_drawables(ImageEncoders *enc, int > > > m > > > while (max_to_free > n > > > && (ring_link = ring_get_head(&enc->glz_orphans)) != NULL) { > > > GlzDictItem *glz_item = SPICE_CONTAINEROF(ring_link, GlzDictItem, > > > retention_link); > > > + glz_dict_item_check(glz_item); > > > if (glz_context_free(glz_item)) { > > > ++n; > > > } > > > @@ -547,6 +570,7 @@ void > > > image_encoders_free_glz_drawables_to_free(ImageEncoders* enc) > > > pthread_mutex_lock(&enc->glz_dict_items_to_free_lock); > > > while ((ring_link = ring_get_head(&enc->glz_dict_items_to_free))) { > > > GlzDictItem * glz_item = SPICE_CONTAINEROF(ring_link, > > > GlzDictItem, > > > free_link); > > > + glz_dict_item_check(glz_item); > > > glz_context_free(glz_item); > > > } > > > pthread_mutex_unlock(&enc->glz_dict_items_to_free_lock); > > > @@ -567,6 +591,7 @@ void image_encoders_free_glz_drawables(ImageEncoders > > > *enc) > > > pthread_rwlock_wrlock(&glz_dict->encode_lock); > > > while ((ring_link = ring_get_head(&enc->glz_dict_items))) { > > > GlzDictItem *glz_item = SPICE_CONTAINEROF(ring_link, GlzDictItem, > > > glz_context_link); > > > + glz_dict_item_check(glz_item); > > > // no need to lock the to_free list, since we assured no other > > > thread > > > is encoding and > > > // thus not other thread access the to_free list of the channel > > > glz_context_free(glz_item); > > > @@ -582,6 +607,7 @@ void glz_retention_destroy(GlzImageRetention > > > *retention) > > > while ((head = ring_get_head(&retention->ring)) != NULL) { > > > GlzDictItem *glz_item = SPICE_CONTAINEROF(head, GlzDictItem, > > > retention_link); > > > > > > + glz_dict_item_check(glz_item); > > > ring_remove(head); > > > ring_add_before(head, &glz_item->enc->glz_orphans); > > > } > > > @@ -1094,6 +1120,7 @@ static GlzDictItem *get_glz_context(ImageEncoders > > > *enc, > > > RedDrawable *red_drawabl > > > > > > ring_add_before(&ret->glz_context_link, &enc->glz_dict_items); > > > ring_add(&glz_retention->ring, &ret->retention_link); > > > + glz_dict_item_check(ret); > > > return ret; > > > } > > > > > _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel