On 05/13/2012 02:40 PM, Alon Levy wrote:
From: Yonit Halperin<yhalp...@redhat.com>
RHBZ: 808936
---
server/red_worker.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/server/red_worker.c b/server/red_worker.c
index 473d0d6..60f30d3 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -1695,13 +1695,12 @@ static inline void put_red_drawable(RedWorker *worker,
RedDrawable *drawable, ui
{
QXLReleaseInfoExt release_info_ext;
- if (self_bitmap) {
- red_put_image(self_bitmap);
- }
if (--drawable->refs) {
return;
}
-
+ if (self_bitmap) {
+ red_put_image(self_bitmap);
+ }
worker->red_drawable_count--;
release_info_ext.group_id = group_id;
release_info_ext.info = drawable->release_info;
That patch itself looks good, assuming "self_bitmap" must be alive when
"drawable" is alive.
A few comments/questions:
put_red_drawable is called from 3 places, one of which calls it with a
NULL self_bitmap.
Is it possible this call will be the last (and self_bitmap would leak) ?
Also it is not clear to me if self_bitmap is updated together with
drawable->refs.
For example in get_drawable self_bitmap is NULL, till we get an image from
the driver (in red_process_commands -> red_process_drawable ->
red_handle_self_bitmap).
I think it makes sense to get self_bitmap inside RedDrawable (can be in
a different patch).
It would clean the code a bit.
(So much text for such a simple patch)
Regards,
Uri.
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
