diff mbox series

[v2,2/2] ui/clipboard: Don't use g_autoptr just to free a variable

Message ID 20211207204038.664133-3-jsnow@redhat.com
State New
Headers show
Series Misc: build fixes for Fedora 35, Ubuntu et al | expand

Commit Message

John Snow Dec. 7, 2021, 8:40 p.m. UTC
Clang doesn't recognize that the variable is being "used" and will emit
a warning:

  ../ui/clipboard.c:47:34: error: variable 'old' set but not used [-Werror,-Wunused-but-set-variable]
      g_autoptr(QemuClipboardInfo) old = NULL;
                                 ^
  1 error generated.

OK, fine. Just do things the old way.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 ui/clipboard.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Philippe Mathieu-Daudé Dec. 8, 2021, 8:40 a.m. UTC | #1
On 12/7/21 21:40, John Snow wrote:
> Clang doesn't recognize that the variable is being "used" and will emit
> a warning:
> 
>   ../ui/clipboard.c:47:34: error: variable 'old' set but not used [-Werror,-Wunused-but-set-variable]
>       g_autoptr(QemuClipboardInfo) old = NULL;
>                                  ^
>   1 error generated.
> 
> OK, fine. Just do things the old way.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  ui/clipboard.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Daniel P. Berrangé Dec. 8, 2021, 9:11 a.m. UTC | #2
On Tue, Dec 07, 2021 at 03:40:38PM -0500, John Snow wrote:
> Clang doesn't recognize that the variable is being "used" and will emit
> a warning:
> 
>   ../ui/clipboard.c:47:34: error: variable 'old' set but not used [-Werror,-Wunused-but-set-variable]
>       g_autoptr(QemuClipboardInfo) old = NULL;
>                                  ^
>   1 error generated.
> 
> OK, fine. Just do things the old way.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  ui/clipboard.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/ui/clipboard.c b/ui/clipboard.c
> index d7b008d62a..9ab65efefb 100644
> --- a/ui/clipboard.c
> +++ b/ui/clipboard.c
> @@ -44,12 +44,11 @@ void qemu_clipboard_peer_release(QemuClipboardPeer *peer,
>  
>  void qemu_clipboard_update(QemuClipboardInfo *info)
>  {
> -    g_autoptr(QemuClipboardInfo) old = NULL;
>      assert(info->selection < QEMU_CLIPBOARD_SELECTION__COUNT);
>  
>      notifier_list_notify(&clipboard_notifiers, info);
>  
> -    old = cbinfo[info->selection];
> +    g_free(cbinfo[info->selection]);

This is a ref counted data type - it can't use g_free:

  https://lists.gnu.org/archive/html/qemu-devel/2021-11/msg04890.html

>      cbinfo[info->selection] = qemu_clipboard_info_ref(info);
>  }

Regards,
Daniel
Philippe Mathieu-Daudé Dec. 8, 2021, 1:49 p.m. UTC | #3
On 12/8/21 10:11, Daniel P. Berrangé wrote:
> On Tue, Dec 07, 2021 at 03:40:38PM -0500, John Snow wrote:
>> Clang doesn't recognize that the variable is being "used" and will emit
>> a warning:
>>
>>   ../ui/clipboard.c:47:34: error: variable 'old' set but not used [-Werror,-Wunused-but-set-variable]
>>       g_autoptr(QemuClipboardInfo) old = NULL;
>>                                  ^
>>   1 error generated.
>>
>> OK, fine. Just do things the old way.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>  ui/clipboard.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/ui/clipboard.c b/ui/clipboard.c
>> index d7b008d62a..9ab65efefb 100644
>> --- a/ui/clipboard.c
>> +++ b/ui/clipboard.c
>> @@ -44,12 +44,11 @@ void qemu_clipboard_peer_release(QemuClipboardPeer *peer,
>>  
>>  void qemu_clipboard_update(QemuClipboardInfo *info)
>>  {
>> -    g_autoptr(QemuClipboardInfo) old = NULL;
>>      assert(info->selection < QEMU_CLIPBOARD_SELECTION__COUNT);
>>  
>>      notifier_list_notify(&clipboard_notifiers, info);
>>  
>> -    old = cbinfo[info->selection];
>> +    g_free(cbinfo[info->selection]);
> 
> This is a ref counted data type - it can't use g_free:
> 
>   https://lists.gnu.org/archive/html/qemu-devel/2021-11/msg04890.html

Doh, I vaguely remembered having already reviewed this then only
found John's v1 and finally mis-reviewed it :/ Thanks for noticing.
John Snow Dec. 8, 2021, 3:22 p.m. UTC | #4
On Wed, Dec 8, 2021, 4:11 AM Daniel P. Berrangé <berrange@redhat.com> wrote:

> On Tue, Dec 07, 2021 at 03:40:38PM -0500, John Snow wrote:
> > Clang doesn't recognize that the variable is being "used" and will emit
> > a warning:
> >
> >   ../ui/clipboard.c:47:34: error: variable 'old' set but not used
> [-Werror,-Wunused-but-set-variable]
> >       g_autoptr(QemuClipboardInfo) old = NULL;
> >                                  ^
> >   1 error generated.
> >
> > OK, fine. Just do things the old way.
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
> >  ui/clipboard.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/ui/clipboard.c b/ui/clipboard.c
> > index d7b008d62a..9ab65efefb 100644
> > --- a/ui/clipboard.c
> > +++ b/ui/clipboard.c
> > @@ -44,12 +44,11 @@ void qemu_clipboard_peer_release(QemuClipboardPeer
> *peer,
> >
> >  void qemu_clipboard_update(QemuClipboardInfo *info)
> >  {
> > -    g_autoptr(QemuClipboardInfo) old = NULL;
> >      assert(info->selection < QEMU_CLIPBOARD_SELECTION__COUNT);
> >
> >      notifier_list_notify(&clipboard_notifiers, info);
> >
> > -    old = cbinfo[info->selection];
> > +    g_free(cbinfo[info->selection]);
>
> This is a ref counted data type - it can't use g_free:
>
>   https://lists.gnu.org/archive/html/qemu-devel/2021-11/msg04890.html
>
> >      cbinfo[info->selection] = qemu_clipboard_info_ref(info);
> >  }
>
> Regards,
> Daniel
>

Whoops, sorry. I saw free being used elsewhere and assumed it was safe.
That's what I get for assuming things...
diff mbox series

Patch

diff --git a/ui/clipboard.c b/ui/clipboard.c
index d7b008d62a..9ab65efefb 100644
--- a/ui/clipboard.c
+++ b/ui/clipboard.c
@@ -44,12 +44,11 @@  void qemu_clipboard_peer_release(QemuClipboardPeer *peer,
 
 void qemu_clipboard_update(QemuClipboardInfo *info)
 {
-    g_autoptr(QemuClipboardInfo) old = NULL;
     assert(info->selection < QEMU_CLIPBOARD_SELECTION__COUNT);
 
     notifier_list_notify(&clipboard_notifiers, info);
 
-    old = cbinfo[info->selection];
+    g_free(cbinfo[info->selection]);
     cbinfo[info->selection] = qemu_clipboard_info_ref(info);
 }