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 143472 - [api] gst_tag_list_foreach() takes a non-const GstTagList*
[api] gst_tag_list_foreach() takes a non-const GstTagList*
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other All
: Normal minor
: 0.9.6
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks: 319388
Reported: 2004-06-01 04:06 UTC by Jeffrey Yasskin
Modified: 2005-11-19 15:53 UTC
See Also:
GNOME target: ---
GNOME version: ---

add_const_to_tag_list_foreach.diff (1.24 KB, patch)
2004-06-01 04:12 UTC, Jeffrey Yasskin
none Details | Review
add_gst_structure_field_foreach_const.diff (5.07 KB, patch)
2004-06-01 04:23 UTC, Jeffrey Yasskin
none Details | Review

Description Jeffrey Yasskin 2004-06-01 04:06:33 UTC
try to call gst_tag_list_foreach() passing a const GstTagList*
Comment 1 Jeffrey Yasskin 2004-06-01 04:12:06 UTC
Created attachment 28222 [details] [review]

I think this patch is ok to apply to the stable branch because it won't break
any programs that currently work.

It's theoretically unsafe because it casts the GstTagList* argument to a
non-const GstStructure* to pass it to gst_structure_field_foreach(). But it's
actually ok because gst_structure_field_foreach() doesn't modify the structure.
Comment 2 Jeffrey Yasskin 2004-06-01 04:23:33 UTC
Created attachment 28223 [details] [review]

This patch includes the last one.

It adds a gst_structure_field_foreach_const() function that takes a const
GstStructure* and calls a GstStructureForeachConstFunc. I added a new function
rather than change the old one because that would break some previously-working
programs. On the other hand, it's probably incorrect to modify the structure
inside a foreach...

Anyway, this patch is only tangentially related to the previous one, so if you
want, I can make a new bug for it.
Comment 3 Benjamin Otte (Company) 2004-06-06 13:48:29 UTC
I think we a greed to not duplicate functions because of constness issues but
just use const and add in the docs a warning that you must not do some specific
things if your structure/tag list really is a const. There are other examples
(like gst_caps_get_structure) where this is already practiced.

The only thing I'm not sure about is if the tag list in GstTagListForEachFunc
really should be a const...

I'd like to get David's input on how to handle these constness issues before
changing anything though.
Comment 4 David Schleef 2004-06-06 20:18:39 UTC
See the comment on gst_caps_get_structure().  The goal is to avoid making the
application do casts from a const pointer to a non-const pointer.

I don't mind having separate gst_blah() and gst_blah_const() functions, and with
a little creative programming, these can be aliased to the same code.  (In fact,
I consider such aliasing a requisite for doing it correctly.)

However, during a stable series, it's not wise to be doing such changes.  Just
because we _can_ add symbols or make backward-compatible changes doesn't mean we
_should_, since that means that applications need to depend on 0.8.2, instead of
just 0.8.0.  Arguably, they have to do this already, but I'd like to not make
the problem worse.
Comment 5 Andy Wingo 2005-07-15 09:35:22 UTC
We should solve this like it is with structures right now:

gboolean gst_structure_foreach (const GstStructure      *structure,
				GstStructureForeachFunc  func,
                                gpointer user_data);
gboolean gst_structure_map_in_place (GstStructure *structure,
                                     GstStructureMapFunc func,
                                     gpointer user_data);

The MapFunc takes a non-const GValue*, and the ForeachFunc takes a const GValue.
The current tag_list_for_each should then be renamed to map_in_place.
Milestoning to 0.9.x.
Comment 6 Andy Wingo 2005-11-19 15:48:21 UTC
Well it turns out my statement was rather silly. The foreach function just
passes the names of the tags, not their values. The tag list api is very unlike
other parts of gstreamer it seems. So I am just going to change the prototype to
have const, because the foreach function doesn't even have a pointer to the
value, and entries shouldn't be added or removed in the foreach.