1. Patchew
  2. QEMU
  3. [PATCH 0/6] tests/docker: Update Fedora containers
  4. View patch

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

John Snow posted 6 patches 4 years, 3 months ago
Maintainers: Wainer dos Santos Moschetta <wainersm@redhat.com>, Willian Rampazzo <willianr@redhat.com>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>, "Alex Bennée" <alex.bennee@linaro.org>, Thomas Huth <thuth@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>
[PATCH 2/6] ui/clipboard: Don't use g_autoptr just to free a variable
Posted by John Snow 4 years, 3 months ago 
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, 2 insertions(+), 1 deletion(-)

diff --git a/ui/clipboard.c b/ui/clipboard.c
index d7b008d62a..d53576b0f6 100644
--- a/ui/clipboard.c
+++ b/ui/clipboard.c
@@ -44,13 +44,14 @@ void qemu_clipboard_peer_release(QemuClipboardPeer *peer,
 
 void qemu_clipboard_update(QemuClipboardInfo *info)
 {
-    g_autoptr(QemuClipboardInfo) old = NULL;
+    QemuClipboardInfo *old = NULL;
     assert(info->selection < QEMU_CLIPBOARD_SELECTION__COUNT);
 
     notifier_list_notify(&clipboard_notifiers, info);
 
     old = cbinfo[info->selection];
     cbinfo[info->selection] = qemu_clipboard_info_ref(info);
+    g_free(old);
 }
 
 QemuClipboardInfo *qemu_clipboard_info(QemuClipboardSelection selection)
-- 
2.31.1
Re: [PATCH 2/6] ui/clipboard: Don't use g_autoptr just to free a variable
Posted by Marc-André Lureau 4 years, 3 months ago 
Hi

On Wed, Nov 3, 2021 at 6:50 PM John Snow <jsnow@redhat.com> 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, 2 insertions(+), 1 deletion(-)
>
> diff --git a/ui/clipboard.c b/ui/clipboard.c
> index d7b008d62a..d53576b0f6 100644
> --- a/ui/clipboard.c
> +++ b/ui/clipboard.c
> @@ -44,13 +44,14 @@ void qemu_clipboard_peer_release(QemuClipboardPeer
> *peer,
>
>  void qemu_clipboard_update(QemuClipboardInfo *info)
>  {
> -    g_autoptr(QemuClipboardInfo) old = NULL;
> +    QemuClipboardInfo *old = NULL;
>      assert(info->selection < QEMU_CLIPBOARD_SELECTION__COUNT);
>
>      notifier_list_notify(&clipboard_notifiers, info);
>
>      old = cbinfo[info->selection];
>      cbinfo[info->selection] = qemu_clipboard_info_ref(info);
> +    g_free(old);
>

qemu_clipboard_info_unref(cbinfo[info->selection]), don't need "old" either
then

thanks

 }
>
>  QemuClipboardInfo *qemu_clipboard_info(QemuClipboardSelection selection)
> --
> 2.31.1
>
>
>

-- 
Marc-André Lureau
Re: [PATCH 2/6] ui/clipboard: Don't use g_autoptr just to free a variable
Posted by Daniel P. Berrangé 4 years, 3 months ago 
On Wed, Nov 03, 2021 at 10:48:40AM -0400, 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, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/ui/clipboard.c b/ui/clipboard.c
> index d7b008d62a..d53576b0f6 100644
> --- a/ui/clipboard.c
> +++ b/ui/clipboard.c
> @@ -44,13 +44,14 @@ void qemu_clipboard_peer_release(QemuClipboardPeer *peer,
>  
>  void qemu_clipboard_update(QemuClipboardInfo *info)
>  {
> -    g_autoptr(QemuClipboardInfo) old = NULL;
> +    QemuClipboardInfo *old = NULL;
>      assert(info->selection < QEMU_CLIPBOARD_SELECTION__COUNT);
>  
>      notifier_list_notify(&clipboard_notifiers, info);
>  
>      old = cbinfo[info->selection];
>      cbinfo[info->selection] = qemu_clipboard_info_ref(info);
> +    g_free(old);
>  }

Surely the right answer here is to get rid of the variable
entirely as it isn't adding value

   g_free(cbinfo[info->selection]);


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [PATCH 2/6] ui/clipboard: Don't use g_autoptr just to free a variable
Posted by John Snow 4 years, 3 months ago 
On Wed, Nov 3, 2021 at 10:59 AM Daniel P. Berrangé <berrange@redhat.com>
wrote:

> On Wed, Nov 03, 2021 at 10:48:40AM -0400, 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, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/ui/clipboard.c b/ui/clipboard.c
> > index d7b008d62a..d53576b0f6 100644
> > --- a/ui/clipboard.c
> > +++ b/ui/clipboard.c
> > @@ -44,13 +44,14 @@ void qemu_clipboard_peer_release(QemuClipboardPeer
> *peer,
> >
> >  void qemu_clipboard_update(QemuClipboardInfo *info)
> >  {
> > -    g_autoptr(QemuClipboardInfo) old = NULL;
> > +    QemuClipboardInfo *old = NULL;
> >      assert(info->selection < QEMU_CLIPBOARD_SELECTION__COUNT);
> >
> >      notifier_list_notify(&clipboard_notifiers, info);
> >
> >      old = cbinfo[info->selection];
> >      cbinfo[info->selection] = qemu_clipboard_info_ref(info);
> > +    g_free(old);
> >  }
>
> Surely the right answer here is to get rid of the variable
> entirely as it isn't adding value
>
>    g_free(cbinfo[info->selection]);
>

Alrighty, I'll clean it up.

Respin pending comments on 1/6 and 6/6.

--js

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.