Re: Outstanding patches, #58609

[Owen Taylor <otaylor redhat com>]

"Matthias Clasen" <matthiasc poet de> writes:

> ----- Original Message -----
> From: "Owen Taylor" <otaylor redhat com>
> To: "Matthias Clasen" <matthiasc poet de>
> Cc: <gtk-devel-list gnome org>
> Sent: Tuesday, August 07, 2001 11:43 PM
> Subject: Re: Outstanding patches, #58609
> 
> 
> >
> > This has two problems:
> >
> >  - g_object_class_list_properties() returns allocated memory.
> >
> >  - g_object_class_list_properties() lists all the properties
> >    for the class and its parents, not just the ones you
> >    want.
> >
> >    What you need to do is in create_prop_editor() when type == 0:
> >    create all the pages in the notebook, call
> >    g_object_class_list_properties(), sort the results by name,
> >    and then iterate through them adding them to the appropriate
> >    page based on param_spec->owner_type.
> >
> >    For type != 0, you want to just find the properties
> >    where param_spec->owner_type == type.
> >
> 
> Here is a new patch, hopefully addressing the issues. Less efficient than
> what you propose for type == 0, since we
> repeatedly allocate and free the params, but that should not be a problem, I
> guess. There is also a simple typo fix thrown in.

Yeah, this inefficiency should be fine. Looks OK to commit
with one simple comment:

> @@ -670,12 +655,14 @@
>    GtkWidget *sw;
>    GtkWidget *vbox;
>    GtkWidget *table;
> +  GObjectClass *class;
> +  GParamSpec **specs;
> +  gint n_specs;
>    int i;
> -  gint n_specs = 0;
> -  GParamSpec **specs = NULL;
> 
> -  get_param_specs (type, &specs, &n_specs);
> -
> +  class = g_type_class_peek (type);

why don't you add a g_assert (class != NULL); here. We know it
will be loaded here because we have an object of a
derived type, but in general, you shouldn't assume
that g_type_class_peek() will return the class.

Regards,
                                        Owen