GNOME Bugzilla – Bug 736620
GQueue: accept NULL sibling for insert_before() and insert_after()
Last modified: 2014-11-10 17:10:51 UTC
For g_queue_insert_before(), it would be nice to allow a NULL sibling to push at the tail of the queue. For g_queue_insert_after(), a NULL sibling could push at the head of the queue. I can write the (simple) patch if you agree that it would be useful.
Any thoughts on this? It would simplify a little bit some code in the undo/redo implementation in GtkSourceView, so it isn't just for a theoretical use case. (the use case in GtkSourceView is that we store a GList pointer to the current history location in the undo/redo GQueue. When we insert a new node it's relative to the location pointer, which can be NULL for the end of the queue).
I agree that this makes sense.
Created attachment 290018 [details] [review] GQueue: accept a NULL sibling for insert_before() and insert_after() It simplifies a little bit some code that inserts data relative to a GList location, that might be NULL for the tail of the queue. A NULL sibling is probably less useful for insert_after(), so it's more for consistency with insert_before().
Created attachment 290019 [details] [review] Simplify code that uses g_queue_insert_before() and insert_after() g_queue_insert_before() and g_queue_insert_after() now accept a NULL sibling.
Review of attachment 290018 [details] [review]: ::: glib/gqueue.c @@ +1008,3 @@ + else + { + queue->head = g_list_insert_before (queue->head, sibling, data); begs the question: why not fix GList in the same way as well?
g_list_insert_before() already supports a NULL sibling, but it runs in O(n) time, while in GQueue it's always O(1).
(In reply to comment #6) > g_list_insert_before() already supports a NULL sibling, but it runs in O(n) > time, while in GQueue it's always O(1). Also: we'd need to update the tail pointer in that case... Can you add a comment about these two points? Otherwise looks good.
Created attachment 290286 [details] [review] GQueue: accept a NULL sibling for insert_before() and insert_after() It simplifies a little bit some code that inserts data relative to a GList location, that might be NULL for the tail of the queue. A NULL sibling is probably less useful for insert_after(), so it's more for consistency with insert_before().
Review of attachment 290286 [details] [review]: Thanks
Review of attachment 290019 [details] [review]: and thanks again
Pushed, thanks for the review.