Patchwork spice: drop incorrect vm_change_state_handler() opaque

login
register
mail settings
Submitter Stefan Hajnoczi
Date Dec. 19, 2012, 1:07 p.m.
Message ID <1355922436-8378-1-git-send-email-stefanha@redhat.com>
Download mbox | patch
Permalink /patch/207345/
State New
Headers show

Comments

Stefan Hajnoczi - Dec. 19, 2012, 1:07 p.m.
The spice_server pointer is a global variable and
vm_change_state_handler() therefore does not use its opaque parameter.

The vm change state handler is added with a pointer to the spice_server
pointer.  This is useless and we probably would not want 2 levels of
pointers.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 ui/spice-core.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
Uri Lublin - Dec. 19, 2012, 3:39 p.m.
On 12/19/2012 03:07 PM, Stefan Hajnoczi wrote:
> The spice_server pointer is a global variable and

... is a static variable in ui/spice-core.c and

> vm_change_state_handler() therefore does not use its opaque parameter.
>
> The vm change state handler is added with a pointer to the spice_server
> pointer.  This is useless and we probably would not want 2 levels of
> pointers.

Reviewed-by: Uri Lublin <uril@redhat.com>

>
> Signed-off-by: Stefan Hajnoczi<stefanha@redhat.com>
> ---
>   ui/spice-core.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/ui/spice-core.c b/ui/spice-core.c
> index ac46deb..c128c0b 100644
> --- a/ui/spice-core.c
> +++ b/ui/spice-core.c
> @@ -709,7 +709,7 @@ void qemu_spice_init(void)
>       qemu_spice_input_init();
>       qemu_spice_audio_init();
>
> -    qemu_add_vm_change_state_handler(vm_change_state_handler,&spice_server);
> +    qemu_add_vm_change_state_handler(vm_change_state_handler, NULL);
>
>       g_free(x509_key_file);
>       g_free(x509_cert_file);
> @@ -736,8 +736,7 @@ int qemu_spice_add_interface(SpiceBaseInstance *sin)
>            */
>           spice_server = spice_server_new();
>           spice_server_init(spice_server,&core_interface);
> -        qemu_add_vm_change_state_handler(vm_change_state_handler,
> -&spice_server);
> +        qemu_add_vm_change_state_handler(vm_change_state_handler, NULL);
>       }
>
>       return spice_server_add_interface(spice_server, sin);
Stefan Hajnoczi - Jan. 2, 2013, 3:56 p.m.
On Wed, Dec 19, 2012 at 02:07:16PM +0100, Stefan Hajnoczi wrote:
> The spice_server pointer is a global variable and
> vm_change_state_handler() therefore does not use its opaque parameter.
> 
> The vm change state handler is added with a pointer to the spice_server
> pointer.  This is useless and we probably would not want 2 levels of
> pointers.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  ui/spice-core.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/ui/spice-core.c b/ui/spice-core.c
> index ac46deb..c128c0b 100644
> --- a/ui/spice-core.c
> +++ b/ui/spice-core.c
> @@ -709,7 +709,7 @@ void qemu_spice_init(void)
>      qemu_spice_input_init();
>      qemu_spice_audio_init();
>  
> -    qemu_add_vm_change_state_handler(vm_change_state_handler, &spice_server);
> +    qemu_add_vm_change_state_handler(vm_change_state_handler, NULL);
>  
>      g_free(x509_key_file);
>      g_free(x509_cert_file);
> @@ -736,8 +736,7 @@ int qemu_spice_add_interface(SpiceBaseInstance *sin)
>           */
>          spice_server = spice_server_new();
>          spice_server_init(spice_server, &core_interface);
> -        qemu_add_vm_change_state_handler(vm_change_state_handler,
> -                                         &spice_server);
> +        qemu_add_vm_change_state_handler(vm_change_state_handler, NULL);
>      }
>  
>      return spice_server_add_interface(spice_server, sin);
> -- 
> 1.8.0.2

Gerd, would you like to take this through the spice queue or should I
put it in trivial-patches?

Stefan
Gerd Hoffmann - Jan. 3, 2013, 8:20 a.m.
On 01/02/13 16:56, Stefan Hajnoczi wrote:
> On Wed, Dec 19, 2012 at 02:07:16PM +0100, Stefan Hajnoczi wrote:
>> The spice_server pointer is a global variable and
>> vm_change_state_handler() therefore does not use its opaque parameter.
>>
>> The vm change state handler is added with a pointer to the spice_server
>> pointer.  This is useless and we probably would not want 2 levels of
>> pointers.

> Gerd, would you like to take this through the spice queue or should I
> put it in trivial-patches?

Taking through trivial is fine with me, especially as the spice patch
queue is empty atm ;)

thanks,
  Gerd
Stefan Hajnoczi - Jan. 3, 2013, 10:57 a.m.
On Thu, Jan 3, 2013 at 9:20 AM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> On 01/02/13 16:56, Stefan Hajnoczi wrote:
>> On Wed, Dec 19, 2012 at 02:07:16PM +0100, Stefan Hajnoczi wrote:
>>> The spice_server pointer is a global variable and
>>> vm_change_state_handler() therefore does not use its opaque parameter.
>>>
>>> The vm change state handler is added with a pointer to the spice_server
>>> pointer.  This is useless and we probably would not want 2 levels of
>>> pointers.
>
>> Gerd, would you like to take this through the spice queue or should I
>> put it in trivial-patches?
>
> Taking through trivial is fine with me, especially as the spice patch
> queue is empty atm ;)

Okay, I'll take it.  Thanks!

Stefan

Patch

diff --git a/ui/spice-core.c b/ui/spice-core.c
index ac46deb..c128c0b 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -709,7 +709,7 @@  void qemu_spice_init(void)
     qemu_spice_input_init();
     qemu_spice_audio_init();
 
-    qemu_add_vm_change_state_handler(vm_change_state_handler, &spice_server);
+    qemu_add_vm_change_state_handler(vm_change_state_handler, NULL);
 
     g_free(x509_key_file);
     g_free(x509_cert_file);
@@ -736,8 +736,7 @@  int qemu_spice_add_interface(SpiceBaseInstance *sin)
          */
         spice_server = spice_server_new();
         spice_server_init(spice_server, &core_interface);
-        qemu_add_vm_change_state_handler(vm_change_state_handler,
-                                         &spice_server);
+        qemu_add_vm_change_state_handler(vm_change_state_handler, NULL);
     }
 
     return spice_server_add_interface(spice_server, sin);