[Spice-devel,v3,2/2] server/red_worker: release bad drawables

Submitted by Alon Levy on July 22, 2012, 10:04 a.m.

Details

Message ID 1342951460-21744-2-git-send-email-alevy@redhat.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Alon Levy July 22, 2012, 10:04 a.m.
---
 server/red_worker.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Patch hide | download patch | download mbox

diff --git a/server/red_worker.c b/server/red_worker.c
index 5634db5..e239740 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -4843,11 +4843,10 @@  static int red_process_commands(RedWorker *worker, uint32_t max_pipe_size, int *
         case QXL_CMD_DRAW: {
             RedDrawable *red_drawable = red_drawable_new(); // returns with 1 ref
 
-            if (red_get_drawable(&worker->mem_slots, ext_cmd.group_id,
+            if (!red_get_drawable(&worker->mem_slots, ext_cmd.group_id,
                                  red_drawable, ext_cmd.cmd.data, ext_cmd.flags)) {
-                break;
+                red_process_drawable(worker, red_drawable, ext_cmd.group_id);
             }
-            red_process_drawable(worker, red_drawable, ext_cmd.group_id);
             // release the red_drawable
             put_red_drawable(worker, red_drawable, ext_cmd.group_id);
             break;

Comments

On 07/22/2012 01:04 PM, Alon Levy wrote:
> ---
>   server/red_worker.c |    5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/server/red_worker.c b/server/red_worker.c
> index 5634db5..e239740 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -4843,11 +4843,10 @@ static int red_process_commands(RedWorker *worker, uint32_t max_pipe_size, int *
>           case QXL_CMD_DRAW: {
>               RedDrawable *red_drawable = red_drawable_new(); // returns with 1 ref
>
> -            if (red_get_drawable(&worker->mem_slots, ext_cmd.group_id,
> +            if (!red_get_drawable(&worker->mem_slots, ext_cmd.group_id,
>                                    red_drawable, ext_cmd.cmd.data, ext_cmd.flags)) {
> -                break;
> +                red_process_drawable(worker, red_drawable, ext_cmd.group_id);
>               }
> -            red_process_drawable(worker, red_drawable, ext_cmd.group_id);
>               // release the red_drawable
>               put_red_drawable(worker, red_drawable, ext_cmd.group_id);
>               break;

ACK.
Just notice that drawables that result from the previous patch are not 
considered "bad", i.e., red_get_drawable doesn't return an error for them.

Yonit.
On Sun, Jul 22, 2012 at 01:36:42PM +0300, Yonit Halperin wrote:
> On 07/22/2012 01:04 PM, Alon Levy wrote:
> >---
> >  server/red_worker.c |    5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> >
> >diff --git a/server/red_worker.c b/server/red_worker.c
> >index 5634db5..e239740 100644
> >--- a/server/red_worker.c
> >+++ b/server/red_worker.c
> >@@ -4843,11 +4843,10 @@ static int red_process_commands(RedWorker *worker, uint32_t max_pipe_size, int *
> >          case QXL_CMD_DRAW: {
> >              RedDrawable *red_drawable = red_drawable_new(); // returns with 1 ref
> >
> >-            if (red_get_drawable(&worker->mem_slots, ext_cmd.group_id,
> >+            if (!red_get_drawable(&worker->mem_slots, ext_cmd.group_id,
> >                                   red_drawable, ext_cmd.cmd.data, ext_cmd.flags)) {
> >-                break;
> >+                red_process_drawable(worker, red_drawable, ext_cmd.group_id);
> >              }
> >-            red_process_drawable(worker, red_drawable, ext_cmd.group_id);
> >              // release the red_drawable
> >              put_red_drawable(worker, red_drawable, ext_cmd.group_id);
> >              break;
> 
> ACK.
> Just notice that drawables that result from the previous patch are not
> considered "bad", i.e., red_get_drawable doesn't return an error for them.

Right, it's two separate issues, but thanks for noticing. Sending an
additional patch on top of 1/2 to fix it (and other memory leaks for bad
palette).

> 
> Yonit.