|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Review comments addressed:
> g_list_free_full() is glib 2.28 while the configure.ac says we need 0.2.25.11.
> We usually to a g_list_foreach(list, unref), g_list_free() so we don't have to
> bump to very recent glib version.
>
> Also,I strongly prefer if you clear the queue structure, and not keep random
> pointers around then using this boolean. Also, as NULL is a correct empty list,
> you don't have to do any NULL check.
>
>
> Patch: "tpl_text_event_{add,dup}_supersedes":
>
> add_supersedes() writes into a TplTextEvent, this is not allowed in the public
> interface, move this function to text-event-private.h and prepend a _.
>
> > + for (l = old_event->priv->supersedes.head; l != NULL; l = l->next)
> > + g_queue_push_tail (&self->priv->supersedes, g_object_ref (l->data));
>
> Use g_list_next() instead of "l = l->next" for readability, don't worry it's a
> macro, not a function call.
>
> > + for (l = self->priv->supersedes.tail; l != NULL; l = l->prev)
> > + supersedes = g_list_prepend (supersedes, g_object_ref (l->data));
>
> Use g_list_previous().
>
Also, I realised that my annotation (transfer full) was incorrect:
Since I ref the object that is passed in, I think (transfer none) is the
correct annotation (since I create a new ref rather than stealing it off
the caller). Correct me if I'm wrong again.
|