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 795376 - Add g_ptr_array_steal()
Add g_ptr_array_steal()
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: garray
2.56.x
Other Linux
: Normal enhancement
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2018-04-19 13:33 UTC by Philip Withnall
Modified: 2018-05-09 12:54 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
garray: Factor out implementation of g_ptr_array_remove_index*() (3.55 KB, patch)
2018-04-19 14:22 UTC, Philip Withnall
committed Details | Review
garray: Document that return value of g_ptr_array_remove() may be junk (2.42 KB, patch)
2018-04-19 14:22 UTC, Philip Withnall
committed Details | Review
garray: Add g_ptr_array_steal_index*() functions (6.70 KB, patch)
2018-04-19 14:22 UTC, Philip Withnall
committed Details | Review

Description Philip Withnall 2018-04-19 13:33:24 UTC
At the moment, if you create a GPtrArray with a free function, it’s really quite hard to remove an entry from the array without its free function being called.

If the free function ignores NULLs, you can do g_steal_pointer (&my_array->pdata[i]), but if the free function doesn’t, you’re a bit stuck.

It would be useful to add

gpointer g_ptr_array_steal (GPtrArray *array, guint index);
Comment 1 Philip Withnall 2018-04-19 14:22:23 UTC
Created attachment 371132 [details] [review]
garray: Factor out implementation of g_ptr_array_remove_index*()

They were almost identically the same. This introduces no functional
changes, but will help with upcoming additions to GPtrArray.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Comment 2 Philip Withnall 2018-04-19 14:22:29 UTC
Created attachment 371133 [details] [review]
garray: Document that return value of g_ptr_array_remove() may be junk

If using g_ptr_array_remove*() with a non-NULL GDestroyNotify function,
the value returned will probably be freed memory (depending on what the
GDestroyNotify) function actually does. Warn about that in the
documentation. We can’t just unconditionally return NULL in these cases,
though, since the user might have set the GDestroyNotify to a nifty
function which doesn’t actually free the element; so returning it might
still be valid and useful.

Also add missing (nullable) annotations to that documentation.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Comment 3 Philip Withnall 2018-04-19 14:22:35 UTC
Created attachment 371134 [details] [review]
garray: Add g_ptr_array_steal_index*() functions

These make it easy to steal elements from pointer arrays without having
the array’s GDestroyNotify called on them, similarly to what’s possible
with g_hash_table_steal().

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Comment 4 Emmanuele Bassi (:ebassi) 2018-05-08 11:27:35 UTC
I do wonder if we want to have the function under the `g_steal_` namespace instead of using the `GPtrArray` one.
Comment 5 Philip Withnall 2018-05-08 11:39:34 UTC
(In reply to Emmanuele Bassi (:ebassi) from comment #4)
> I do wonder if we want to have the function under the `g_steal_` namespace
> instead of using the `GPtrArray` one.

I think it makes more sense to have under the GPtrArray namespace. It’s definitely a method of the GPtrArray class (I realise it’s not a class in the GObject sense, but it is still one in the general OOP sense).
Comment 6 Emmanuele Bassi (:ebassi) 2018-05-09 12:07:36 UTC
Fair enough; then the whole series is ACK-by: me.
Comment 7 Philip Withnall 2018-05-09 12:54:13 UTC
Trivial rebase and pushed to master, ta.

Attachment 371132 [details] pushed as 2c9a84e - garray: Factor out implementation of g_ptr_array_remove_index*()
Attachment 371133 [details] pushed as e6200ea - garray: Document that return value of g_ptr_array_remove() may be junk
Attachment 371134 [details] pushed as d0a48f2 - garray: Add g_ptr_array_steal_index*() functions