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 657433 - g_queue_free_full() missing
g_queue_free_full() missing
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: general
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2011-08-26 12:51 UTC by Alban Crequy
Modified: 2011-12-19 07:14 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Added new API to free a Queue including its dynamically-allocated elements. (On similar lines to List and Slist). (3.44 KB, patch)
2011-12-13 11:34 UTC, Ravi Sankar Guntur
reviewed Details | Review
API to free a Queue including its dynamically allocated elements. (4.29 KB, patch)
2011-12-14 14:54 UTC, Ravi Sankar Guntur
accepted-commit_now Details | Review
Added API g_queue_free_full(). (4.28 KB, patch)
2011-12-16 14:55 UTC, Matthias Clasen
committed Details | Review
Patch to only iterate the list once (807 bytes, patch)
2011-12-16 17:28 UTC, Carl-Anton Ingmarsson
none Details | Review

Description Alban Crequy 2011-08-26 12:51:30 UTC
There is no function to free a GQueue, including its dynamically-allocated elements. I would like:

  g_queue_free_full(GQueue *queue, GDestroyNotify free_func);

Without it, I have to iterate twice in the list with:

  g_queue_foreach (queue, (GFunc) free_func, NULL);
  g_queue_free (queue);
Comment 1 Matthias Clasen 2011-08-27 20:04:46 UTC
I guess you could just do

g_list_free_full (queue->head, free_func, NULL);
queue->head = NULL;
g_queue_free (queue);

or

while ((elt = g_queue_pop_head (queue))
  free_func (elt, NULL);
g_queue_free (queue);

to avoid the 'iterate twice'. 

But I guess both of these lack some elegance, and I would accept a patch for free_full, if only to keep the api parallel to list and slist.
Comment 2 Ravi Sankar Guntur 2011-12-13 11:32:33 UTC
Added proposed API and its test case. Please review the patch. 


void  g_queue_free_full  (GQueue  *queue, GDestroyNotify    free_func);
Comment 3 Ravi Sankar Guntur 2011-12-13 11:34:07 UTC
Created attachment 203330 [details] [review]
Added new API to free a Queue including its dynamically-allocated elements. (On similar lines to List and Slist).
Comment 4 Emmanuele Bassi (:ebassi) 2011-12-13 11:42:06 UTC
Review of attachment 203330 [details] [review]:

looks good to me.
Comment 5 Ravi Sankar Guntur 2011-12-14 14:54:32 UTC
Created attachment 203478 [details] [review]
API to free a Queue including its dynamically allocated elements. 

patch amended.

+ added Note to g_queue_free() API. 
+ added g_queue_free_full() API to glib.symbols
Comment 6 Matthias Clasen 2011-12-15 02:09:37 UTC
Review of attachment 203478 [details] [review]:

Looks good, thanks
Comment 7 Matthias Clasen 2011-12-16 14:55:48 UTC
The following fix has been pushed:
1d4009e Added API g_queue_free_full().
Comment 8 Matthias Clasen 2011-12-16 14:55:51 UTC
Created attachment 203678 [details] [review]
Added API g_queue_free_full().

g_queue_free_full(), to free a Queue including its dynamically-allocated elements.
On similar lines to List and Slist.

void  g_queue_free_full  (GQueue  *queue,  GDestroyNotify    free_func);

Test case covering g_queue_free_full() is added.
Added export symbol to glib.symbols.

Closes Bug: https://bugzilla.gnome.org/show_bug.cgi?id=657433

Signed-off-by: Ravi Sankar Guntur <ravi.g@samsung.com>
Comment 9 Carl-Anton Ingmarsson 2011-12-16 16:30:33 UTC
Review of attachment 203678 [details] [review]:

::: glib/gqueue.c
@@ +109,3 @@
+  g_queue_foreach (queue, (GFunc) free_func, NULL);
+  g_queue_free (queue);
+}

Doesn't this iterate the list twice?
Wouldn't it be better to do

g_list_free_full (queue->head, free_func);
g_slice_free (GQueue, queue);
Comment 10 Carl-Anton Ingmarsson 2011-12-16 17:28:34 UTC
Created attachment 203687 [details] [review]
Patch to only iterate the list once
Comment 11 Ravi Sankar Guntur 2011-12-19 07:14:30 UTC
Doesn't g_list_free_full() also iterate twice?
 g_list_foreach()
 g_list_free()