After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 623068 - Add (out) annotations to GtkTreeIter parameters
Add (out) annotations to GtkTreeIter parameters
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: GtkTreeView
2.90.x
Other Linux
: Normal normal
: ---
Assigned To: gtktreeview-bugs
gtktreeview-bugs
Depends on:
Blocks: 622963 623359
 
 
Reported: 2010-06-28 21:00 UTC by Philip Withnall
Modified: 2010-07-07 16:21 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add (out) annotations to GtkTreeIter parameters (14.49 KB, patch)
2010-06-28 21:00 UTC, Philip Withnall
needs-work Details | Review
add out to treeiter (552 bytes, patch)
2010-06-30 21:30 UTC, Ignacio Casal Quinteiro (nacho)
rejected Details | Review

Description Philip Withnall 2010-06-28 21:00:42 UTC
Created attachment 164839 [details] [review]
Add (out) annotations to GtkTreeIter parameters

Patch attached to add (out) annotations to all the relevant GtkTreeIter function parameters I could find.

Queries:
 * Not sure about whether the (inout) parameter of gtk_tree_model_iter_next() should be caller-allocates or callee-allocates
 * Even though I've added caller-allocates to the parameters in the following functions, caller-allocates="0" appears in the GIR file. I suspect it's something to do with them also being virtual methods on GtkTreeModel, but I'm not sure:
   - gtk_tree_model_iter_children()
   - gtk_tree_model_iter_nth_child()
   - gtk_tree_model_iter_parent()
   - gtk_tree_model_get_iter()
   - gtk_tree_model_iter_children()
Comment 1 johnp 2010-06-29 15:09:23 UTC
Comment on attachment 164839 [details] [review]
Add (out) annotations to GtkTreeIter parameters

Nice work.  Just a few comments bellow.



>diff --git a/gtk/gtktreemodel.c b/gtk/gtktreemodel.c
>index 526d168..bc305e0 100644
>--- a/gtk/gtktreemodel.c
>+++ b/gtk/gtktreemodel.c
>@@ -998,7 +998,7 @@ gtk_tree_model_get_column_type (GtkTreeModel *tree_model,
> /**
>  * gtk_tree_model_get_iter:
>  * @tree_model: A #GtkTreeModel.
>- * @iter: The uninitialized #GtkTreeIter.
>+ * @iter: (out caller-allocates): The uninitialized #GtkTreeIter.

You don't need to add caller-allocates.  The scanner will pick this up.

> /**
>  * gtk_tree_model_iter_next:
>  * @tree_model: A #GtkTreeModel.
>- * @iter: The #GtkTreeIter.
>+ * @iter: (inout caller-allocates): The #GtkTreeIter.

There is no such thing as inout caller-allocates.  It may be allocated by the caller or another API.  This is simply
an inout parameter.  Just as a pygobject note, we should test that this does the right thing and doesn't leak memory or worse.

>  *
>  * Sets @iter to point to the node following it at the current level.  If there
>  * is no next @iter, %FALSE is returned and @iter is set to be invalid.
>@@ -1200,7 +1200,7 @@ gtk_tree_model_iter_next (GtkTreeModel  *tree_model,
> /**
>  * gtk_tree_model_iter_children:
>  * @tree_model: A #GtkTreeModel.
>- * @iter: The new #GtkTreeIter to be set to the child.
>+ * @iter: (out caller-allocates): The new #GtkTreeIter to be set to the child.

Again, caller-allocates isn't needed. For consistency we should drop it. 

>  * @parent: (allow-none): The #GtkTreeIter, or %NULL
>  *
>  * Sets @iter to point to the first child of @parent.  If @parent has no
>@@ -1281,7 +1281,7 @@ gtk_tree_model_iter_n_children (GtkTreeModel *tree_model,
> /**
>  * gtk_tree_model_iter_nth_child:
>  * @tree_model: A #GtkTreeModel.
>- * @iter: The #GtkTreeIter to set to the nth child.
>+ * @iter: (out caller-allocates): The #GtkTreeIter to set to the nth child.

drop caller-allocates

>  * @parent: (allow-none): The #GtkTreeIter to get the child from, or %NULL.
>  * @n: Then index of the desired child.
>  *
>@@ -1316,7 +1316,7 @@ gtk_tree_model_iter_nth_child (GtkTreeModel *tree_model,
> /**
>  * gtk_tree_model_iter_parent:
>  * @tree_model: A #GtkTreeModel
>- * @iter: The new #GtkTreeIter to set to the parent.
>+ * @iter: (out caller-allocates): The new #GtkTreeIter to set to the parent.

drop caller-allocates


The rest looks good
Comment 2 Philip Withnall 2010-06-29 15:34:12 UTC
(In reply to comment #1)
> (From update of attachment 164839 [details] [review])
> Nice work.  Just a few comments bellow.
> 
> 
> 
> >diff --git a/gtk/gtktreemodel.c b/gtk/gtktreemodel.c
> >index 526d168..bc305e0 100644
> >--- a/gtk/gtktreemodel.c
> >+++ b/gtk/gtktreemodel.c
> >@@ -998,7 +998,7 @@ gtk_tree_model_get_column_type (GtkTreeModel *tree_model,
> > /**
> >  * gtk_tree_model_get_iter:
> >  * @tree_model: A #GtkTreeModel.
> >- * @iter: The uninitialized #GtkTreeIter.
> >+ * @iter: (out caller-allocates): The uninitialized #GtkTreeIter.
> 
> You don't need to add caller-allocates.  The scanner will pick this up.

I added it in these cases because the scanner apparently didn't pick it up with just (out), although this is probably related to my second query in comment #0.

Do you want an updated patch without these, and then a separate bug filed about the caller-allocates="0" issue, or would it be best to commit with the caller-allocates annotation and fix up the scanner later?

> > /**
> >  * gtk_tree_model_iter_next:
> >  * @tree_model: A #GtkTreeModel.
> >- * @iter: The #GtkTreeIter.
> >+ * @iter: (inout caller-allocates): The #GtkTreeIter.
> 
> There is no such thing as inout caller-allocates.  It may be allocated by the
> caller or another API.  This is simply
> an inout parameter.  Just as a pygobject note, we should test that this does
> the right thing and doesn't leak memory or worse.

Right.
Comment 3 Ignacio Casal Quinteiro (nacho) 2010-06-30 21:30:21 UTC
Created attachment 164999 [details] [review]
add out to treeiter

my 2 cents. I added also caller-allocates. If you want I can change it.
Comment 4 Ignacio Casal Quinteiro (nacho) 2010-06-30 21:32:22 UTC
Comment on attachment 164999 [details] [review]
add out to treeiter

Oh well, didn't see that it was already in the other patch.
Comment 5 Ignacio Casal Quinteiro (nacho) 2010-07-07 16:21:44 UTC
Pushed the patch removing the caller-allocates:
http://git.gnome.org/browse/gtk+/commit/?id=0b51abbfdf5db1d29e28ddc62f74e7e9a40ae010