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 795975 - Fix g_array_insert_vals() with an index off the end of the array
Fix g_array_insert_vals() with an index off the end of the array
Status: RESOLVED OBSOLETE
Product: glib
Classification: Platform
Component: garray
2.56.x
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
: 414301 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2018-05-09 14:33 UTC by Philip Withnall
Modified: 2018-05-24 20:31 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
garray: Fix (nullable) annotation on GArray.[prepend|insert]_vals() (1.94 KB, patch)
2018-05-09 14:42 UTC, Philip Withnall
none Details | Review
garray: Allow over-allocation in g_array_insert_vals() (2.61 KB, patch)
2018-05-09 14:42 UTC, Philip Withnall
none Details | Review
tests: Expand GArray test coverage to cover all construction forms (18.36 KB, patch)
2018-05-09 14:42 UTC, Philip Withnall
none Details | Review

Description Philip Withnall 2018-05-09 14:33:39 UTC
Currently, calling g_array_insert_vals() with an index which is off the end of the array results in a crash in memmove(). Given that GArray supports clearing newly created elements when setting the array larger than its current size using g_array_set_size(), it seems appropriate to allow this with g_array_insert_vals().

Patch series coming up which fixes this and adds lots of test coverage for various bits of GArray (from looking at the missing branch coverage on our existing tests: https://gnome.pages.gitlab.gnome.org/glib/coverage/glib/garray.c.gcov.html).

If we decide we want to disallow g_array_insert_vals() from being called with out-of-bounds indexes, that’s also fine; I can rearrange the patches to add a g_return_val_if_fail() on the index instead (as per bug #414301).
Comment 1 Philip Withnall 2018-05-09 14:38:51 UTC
*** Bug 414301 has been marked as a duplicate of this bug. ***
Comment 2 Philip Withnall 2018-05-09 14:42:07 UTC
Created attachment 371845 [details] [review]
garray: Fix (nullable) annotation on GArray.[prepend|insert]_vals()

They do both accept NULL value arrays, but only if the number of
elements in the value array is zero. Fix the annotations and mention
this in the documentation.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Comment 3 Philip Withnall 2018-05-09 14:42:13 UTC
Created attachment 371846 [details] [review]
garray: Allow over-allocation in g_array_insert_vals()

Previously, g_array_insert_vals() would crash if called with an index
off the end of the array. This is inconsistent with the behaviour of
other methods (like g_array_set_size()), which will expand the array as
necessary.

Modify g_array_insert_vals() to expand the array as necessary. New array
elements will be cleared to zero if the GArray was constructed with
(clear_ == TRUE).

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Comment 4 Philip Withnall 2018-05-09 14:42:20 UTC
Created attachment 371847 [details] [review]
tests: Expand GArray test coverage to cover all construction forms

Previously, there was very little coverage of GArray behaviour with
either of the zero_terminated or clear_ arguments to g_array_new() set
to TRUE.

Parameterise the tests and exhaustively expand the coverage to cover all
possible GArray configurations.

This was made possible by the online code coverage report for GLib which
we now have:
https://gnome.pages.gitlab.gnome.org/glib/coverage/glib/garray.c.gcov.html.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Comment 5 GNOME Infrastructure Team 2018-05-24 20:31:34 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/1374.