All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] qom/object: Comment to use g_slist_free on object_class_get_list result
@ 2020-02-27 19:09 Philippe Mathieu-Daudé
  2020-02-28  9:46 ` Daniel P. Berrangé
  0 siblings, 1 reply; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-02-27 19:09 UTC (permalink / raw)
  To: qemu-devel
  Cc: Daniel P. Berrangé,
	Eduardo Habkost, qemu-trivial, Pan Nengyuan,
	Dr . David Alan Gilbert, Paolo Bonzini,
	Philippe Mathieu-Daudé

Document the list returned by object_class_get_list() must be
released with g_slist_free() to avoid memory leaks.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/qom/object.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/qom/object.h b/include/qom/object.h
index 29546496c1..5517b56508 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -984,6 +984,9 @@ void object_class_foreach(void (*fn)(ObjectClass *klass, void *opaque),
  * @include_abstract: Whether to include abstract classes.
  *
  * Returns: A singly-linked list of the classes in reverse hashtable order.
+ *
+ * The returned list must be released with g_slist_free()
+ * when no longer required.
  */
 GSList *object_class_get_list(const char *implements_type,
                               bool include_abstract);
@@ -995,6 +998,9 @@ GSList *object_class_get_list(const char *implements_type,
  *
  * Returns: A singly-linked list of the classes in alphabetical
  * case-insensitive order.
+ *
+ * The returned list must be released with g_slist_free()
+ * when no longer required.
  */
 GSList *object_class_get_list_sorted(const char *implements_type,
                               bool include_abstract);
-- 
2.21.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] qom/object: Comment to use g_slist_free on object_class_get_list result
  2020-02-27 19:09 [PATCH] qom/object: Comment to use g_slist_free on object_class_get_list result Philippe Mathieu-Daudé
@ 2020-02-28  9:46 ` Daniel P. Berrangé
  2020-02-28 10:06   ` Marc-André Lureau
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel P. Berrangé @ 2020-02-28  9:46 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Eduardo Habkost, qemu-trivial, Pan Nengyuan, qemu-devel,
	Dr . David Alan Gilbert, Paolo Bonzini

On Thu, Feb 27, 2020 at 08:09:42PM +0100, Philippe Mathieu-Daudé wrote:
> Document the list returned by object_class_get_list() must be
> released with g_slist_free() to avoid memory leaks.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  include/qom/object.h | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/include/qom/object.h b/include/qom/object.h
> index 29546496c1..5517b56508 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -984,6 +984,9 @@ void object_class_foreach(void (*fn)(ObjectClass *klass, void *opaque),
>   * @include_abstract: Whether to include abstract classes.
>   *
>   * Returns: A singly-linked list of the classes in reverse hashtable order.
> + *
> + * The returned list must be released with g_slist_free()
> + * when no longer required.

I'd suggest

  "The returned list, but not its elements, must be released with
   g_slist_free() or g_autoptr when no longer required"

>   */
>  GSList *object_class_get_list(const char *implements_type,
>                                bool include_abstract);
> @@ -995,6 +998,9 @@ GSList *object_class_get_list(const char *implements_type,
>   *
>   * Returns: A singly-linked list of the classes in alphabetical
>   * case-insensitive order.
> + *
> + * The returned list must be released with g_slist_free()
> + * when no longer required.
>   */
>  GSList *object_class_get_list_sorted(const char *implements_type,
>                                bool include_abstract);
> -- 
> 2.21.1
> 
> 

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 :|



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] qom/object: Comment to use g_slist_free on object_class_get_list result
  2020-02-28  9:46 ` Daniel P. Berrangé
@ 2020-02-28 10:06   ` Marc-André Lureau
  2020-02-28 10:08     ` Philippe Mathieu-Daudé
  2020-02-28 10:35     ` Daniel P. Berrangé
  0 siblings, 2 replies; 6+ messages in thread
From: Marc-André Lureau @ 2020-02-28 10:06 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: Eduardo Habkost, qemu trival, Pan Nengyuan,
	Dr . David Alan Gilbert, QEMU, Paolo Bonzini,
	Philippe Mathieu-Daudé

Hi

On Fri, Feb 28, 2020 at 10:47 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Thu, Feb 27, 2020 at 08:09:42PM +0100, Philippe Mathieu-Daudé wrote:
> > Document the list returned by object_class_get_list() must be
> > released with g_slist_free() to avoid memory leaks.
> >
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > ---
> >  include/qom/object.h | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/include/qom/object.h b/include/qom/object.h
> > index 29546496c1..5517b56508 100644
> > --- a/include/qom/object.h
> > +++ b/include/qom/object.h
> > @@ -984,6 +984,9 @@ void object_class_foreach(void (*fn)(ObjectClass *klass, void *opaque),
> >   * @include_abstract: Whether to include abstract classes.
> >   *
> >   * Returns: A singly-linked list of the classes in reverse hashtable order.
> > + *
> > + * The returned list must be released with g_slist_free()
> > + * when no longer required.
>
> I'd suggest
>
>   "The returned list, but not its elements, must be released with
>    g_slist_free() or g_autoptr when no longer required"

As gobject-introspection annotations: "Returns: (transfer container)
(element-type ObjectClass): A list of #ObjectClass"

>
> >   */
> >  GSList *object_class_get_list(const char *implements_type,
> >                                bool include_abstract);
> > @@ -995,6 +998,9 @@ GSList *object_class_get_list(const char *implements_type,
> >   *
> >   * Returns: A singly-linked list of the classes in alphabetical
> >   * case-insensitive order.
> > + *
> > + * The returned list must be released with g_slist_free()
> > + * when no longer required.
> >   */
> >  GSList *object_class_get_list_sorted(const char *implements_type,
> >                                bool include_abstract);
> > --
> > 2.21.1
> >
> >
>
> 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 :|
>
>


-- 
Marc-André Lureau


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] qom/object: Comment to use g_slist_free on object_class_get_list result
  2020-02-28 10:06   ` Marc-André Lureau
@ 2020-02-28 10:08     ` Philippe Mathieu-Daudé
  2020-02-28 10:35       ` Marc-André Lureau
  2020-02-28 10:35     ` Daniel P. Berrangé
  1 sibling, 1 reply; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-02-28 10:08 UTC (permalink / raw)
  To: Marc-André Lureau, Daniel P. Berrangé
  Cc: Eduardo Habkost, qemu trival, Pan Nengyuan,
	Dr . David Alan Gilbert, QEMU, Paolo Bonzini

On 2/28/20 11:06 AM, Marc-André Lureau wrote:
> Hi
> 
> On Fri, Feb 28, 2020 at 10:47 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>>
>> On Thu, Feb 27, 2020 at 08:09:42PM +0100, Philippe Mathieu-Daudé wrote:
>>> Document the list returned by object_class_get_list() must be
>>> released with g_slist_free() to avoid memory leaks.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> ---
>>>   include/qom/object.h | 6 ++++++
>>>   1 file changed, 6 insertions(+)
>>>
>>> diff --git a/include/qom/object.h b/include/qom/object.h
>>> index 29546496c1..5517b56508 100644
>>> --- a/include/qom/object.h
>>> +++ b/include/qom/object.h
>>> @@ -984,6 +984,9 @@ void object_class_foreach(void (*fn)(ObjectClass *klass, void *opaque),
>>>    * @include_abstract: Whether to include abstract classes.
>>>    *
>>>    * Returns: A singly-linked list of the classes in reverse hashtable order.
>>> + *
>>> + * The returned list must be released with g_slist_free()
>>> + * when no longer required.
>>
>> I'd suggest
>>
>>    "The returned list, but not its elements, must be released with
>>     g_slist_free() or g_autoptr when no longer required"
> 
> As gobject-introspection annotations: "Returns: (transfer container)
> (element-type ObjectClass): A list of #ObjectClass"

Are you suggesting to replace "Returns: A singly-linked list of the 
classes in reverse hashtable order." by the line you quoted?

> 
>>
>>>    */
>>>   GSList *object_class_get_list(const char *implements_type,
>>>                                 bool include_abstract);
>>> @@ -995,6 +998,9 @@ GSList *object_class_get_list(const char *implements_type,
>>>    *
>>>    * Returns: A singly-linked list of the classes in alphabetical
>>>    * case-insensitive order.
>>> + *
>>> + * The returned list must be released with g_slist_free()
>>> + * when no longer required.
>>>    */
>>>   GSList *object_class_get_list_sorted(const char *implements_type,
>>>                                 bool include_abstract);
>>> --
>>> 2.21.1
>>>
>>>
>>
>> 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 :|
>>
>>
> 
> 



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] qom/object: Comment to use g_slist_free on object_class_get_list result
  2020-02-28 10:06   ` Marc-André Lureau
  2020-02-28 10:08     ` Philippe Mathieu-Daudé
@ 2020-02-28 10:35     ` Daniel P. Berrangé
  1 sibling, 0 replies; 6+ messages in thread
From: Daniel P. Berrangé @ 2020-02-28 10:35 UTC (permalink / raw)
  To: Marc-André Lureau
  Cc: Eduardo Habkost, qemu trival, Pan Nengyuan, QEMU,
	Dr . David Alan Gilbert, Paolo Bonzini,
	Philippe Mathieu-Daudé

On Fri, Feb 28, 2020 at 11:06:38AM +0100, Marc-André Lureau wrote:
> Hi
> 
> On Fri, Feb 28, 2020 at 10:47 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Thu, Feb 27, 2020 at 08:09:42PM +0100, Philippe Mathieu-Daudé wrote:
> > > Document the list returned by object_class_get_list() must be
> > > released with g_slist_free() to avoid memory leaks.
> > >
> > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > > ---
> > >  include/qom/object.h | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > > diff --git a/include/qom/object.h b/include/qom/object.h
> > > index 29546496c1..5517b56508 100644
> > > --- a/include/qom/object.h
> > > +++ b/include/qom/object.h
> > > @@ -984,6 +984,9 @@ void object_class_foreach(void (*fn)(ObjectClass *klass, void *opaque),
> > >   * @include_abstract: Whether to include abstract classes.
> > >   *
> > >   * Returns: A singly-linked list of the classes in reverse hashtable order.
> > > + *
> > > + * The returned list must be released with g_slist_free()
> > > + * when no longer required.
> >
> > I'd suggest
> >
> >   "The returned list, but not its elements, must be released with
> >    g_slist_free() or g_autoptr when no longer required"
> 
> As gobject-introspection annotations: "Returns: (transfer container)
> (element-type ObjectClass): A list of #ObjectClass"

If we were using Gobject introspection and/or Gtk-Doc, then I'd certainly
suggest that syntax, but AFAIK this is not something that fits with our
intented docs tools. There might be sense in having our docs tools parse
these kind of annotations as it is useful to have it in standardized
format. I think we should decide on this before actually adding them to
the docs though.

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 :|



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] qom/object: Comment to use g_slist_free on object_class_get_list result
  2020-02-28 10:08     ` Philippe Mathieu-Daudé
@ 2020-02-28 10:35       ` Marc-André Lureau
  0 siblings, 0 replies; 6+ messages in thread
From: Marc-André Lureau @ 2020-02-28 10:35 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Daniel P. Berrangé,
	Eduardo Habkost, qemu trival, Pan Nengyuan,
	Dr . David Alan Gilbert, QEMU, Paolo Bonzini

Hi

On Fri, Feb 28, 2020 at 11:09 AM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> On 2/28/20 11:06 AM, Marc-André Lureau wrote:
> > Hi
> >
> > On Fri, Feb 28, 2020 at 10:47 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >>
> >> On Thu, Feb 27, 2020 at 08:09:42PM +0100, Philippe Mathieu-Daudé wrote:
> >>> Document the list returned by object_class_get_list() must be
> >>> released with g_slist_free() to avoid memory leaks.
> >>>
> >>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >>> ---
> >>>   include/qom/object.h | 6 ++++++
> >>>   1 file changed, 6 insertions(+)
> >>>
> >>> diff --git a/include/qom/object.h b/include/qom/object.h
> >>> index 29546496c1..5517b56508 100644
> >>> --- a/include/qom/object.h
> >>> +++ b/include/qom/object.h
> >>> @@ -984,6 +984,9 @@ void object_class_foreach(void (*fn)(ObjectClass *klass, void *opaque),
> >>>    * @include_abstract: Whether to include abstract classes.
> >>>    *
> >>>    * Returns: A singly-linked list of the classes in reverse hashtable order.
> >>> + *
> >>> + * The returned list must be released with g_slist_free()
> >>> + * when no longer required.
> >>
> >> I'd suggest
> >>
> >>    "The returned list, but not its elements, must be released with
> >>     g_slist_free() or g_autoptr when no longer required"
> >
> > As gobject-introspection annotations: "Returns: (transfer container)
> > (element-type ObjectClass): A list of #ObjectClass"
>
> Are you suggesting to replace "Returns: A singly-linked list of the
> classes in reverse hashtable order." by the line you quoted?

No, just a remark. I doubt this is compatible with kernel-doc at this
point, and we are not using GI.

fwiw, I don't think "in reverse hashtable order" is really meaningful anyway.

>
> >
> >>
> >>>    */
> >>>   GSList *object_class_get_list(const char *implements_type,
> >>>                                 bool include_abstract);
> >>> @@ -995,6 +998,9 @@ GSList *object_class_get_list(const char *implements_type,
> >>>    *
> >>>    * Returns: A singly-linked list of the classes in alphabetical
> >>>    * case-insensitive order.
> >>> + *
> >>> + * The returned list must be released with g_slist_free()
> >>> + * when no longer required.
> >>>    */
> >>>   GSList *object_class_get_list_sorted(const char *implements_type,
> >>>                                 bool include_abstract);
> >>> --
> >>> 2.21.1
> >>>
> >>>
> >>
> >> 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 :|
> >>
> >>
> >
> >
>


-- 
Marc-André Lureau


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-02-28 10:37 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-27 19:09 [PATCH] qom/object: Comment to use g_slist_free on object_class_get_list result Philippe Mathieu-Daudé
2020-02-28  9:46 ` Daniel P. Berrangé
2020-02-28 10:06   ` Marc-André Lureau
2020-02-28 10:08     ` Philippe Mathieu-Daudé
2020-02-28 10:35       ` Marc-André Lureau
2020-02-28 10:35     ` Daniel P. Berrangé

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.