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 401176 - Use const for g_slist_length() and g_list_length()
Use const for g_slist_length() and g_list_length()
Status: RESOLVED WONTFIX
Product: glib
Classification: Platform
Component: general
2.24.x
Other All
: Normal minor
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2007-01-26 22:39 UTC by Wolfgang Köbler
Modified: 2017-09-11 21:41 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Bug 401176 — Use const for g_slist_length() and g_list_length() (11.60 KB, patch)
2010-03-31 18:17 UTC, Philip Withnall
rejected Details | Review

Description Wolfgang Köbler 2007-01-26 22:39:26 UTC
guint g_list_length (GList *list);

g_slist_length() does not modify list, so it could use (const GList *list).

This would make it possible to use the function in user-made functions that got a parameter of type const GList * (without getting a warning).

Same applies to g_list_length().
Comment 1 Philip Withnall 2010-03-31 18:17:10 UTC
Created attachment 157625 [details] [review]
Bug 401176 — Use const for g_slist_length() and g_list_length()

Make various GList and GSList functions take const lists. Closes: bgo#401176
Comment 2 Benjamin Otte (Company) 2010-04-03 23:41:23 UTC
I think that patch is wrong - at least in places. It doesn't preserve constness in the way a developer would expect it. g_list_prev (g_list_next(const_list)) is non-const with your patch, and that shouldn't work. To make those functions work properly, you'd need to make the return value depend on the constness of the input (i.e. the returned GList is const if the passed in GList is const, but this is not possible to express. And I don't think it's a good idea to repeat the brokenness of strchr() and related functions that just cast away the constness.

At least it has been the policy in glib to not use const at all if it cannot be expressed well - which is why no GObject subclass is ever used const.
Comment 3 Philip Withnall 2010-06-11 17:22:54 UTC
Aargh, I didn't put myself on the CC list. Sorry.

(In reply to comment #2)
> I think that patch is wrong - at least in places. It doesn't preserve constness
> in the way a developer would expect it. g_list_prev (g_list_next(const_list))
> is non-const with your patch, and that shouldn't work. To make those functions
> work properly, you'd need to make the return value depend on the constness of
> the input (i.e. the returned GList is const if the passed in GList is const,
> but this is not possible to express. And I don't think it's a good idea to
> repeat the brokenness of strchr() and related functions that just cast away the
> constness.

The only way I've come across to represent this would be to have two functions: one which takes a const input, and one which takes a non-const input (both giving the appropriate outputs), but that's just ugly.

Probably best to leave functions like g_list_[next|prev]() then, as you say. That still leaves:
 * g_list_copy()
 * g_list_position()
 * g_list_index()
 * g_list_length()
 * g_list_nth_data()
and their GSList counterparts which can be constified, yes?

> At least it has been the policy in glib to not use const at all if it cannot be
> expressed well - which is why no GObject subclass is ever used const.

That makes sense. Is it documented anywhere?
Comment 4 Benjamin Otte (Company) 2010-06-11 18:35:38 UTC
(In reply to comment #3)
> Probably best to leave functions like g_list_[next|prev]() then, as you say.
> That still leaves:
>  * g_list_copy()
>  * g_list_position()
>  * g_list_index()
>  * g_list_length()
>  * g_list_nth_data()
> and their GSList counterparts which can be constified, yes?
> 
I would not object to that if other maintainers decided it's a good idea, but I lean slightly against it for the do-not-use-const-at-all reason below.

> > At least it has been the policy in glib to not use const at all if it cannot be
> > expressed well - which is why no GObject subclass is ever used const.
> 
> That makes sense. Is it documented anywhere?
>
Unfortunately I've never found a place where things like these are documented as they usually get into common usage without anyone ever writing them down. I wouldn't even know where to look for them.
Comment 5 Owen Taylor 2010-06-11 18:57:30 UTC
This is a very old subject, see, e.g.:

http://mail.gnome.org/archives/gtk-devel-list/2001-May/msg00485.html
Comment 6 Philip Withnall 2010-06-11 23:19:33 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > Probably best to leave functions like g_list_[next|prev]() then, as you say.
> > That still leaves:
> >  * g_list_copy()
> >  * g_list_position()
> >  * g_list_index()
> >  * g_list_length()
> >  * g_list_nth_data()
> > and their GSList counterparts which can be constified, yes?
> > 
> I would not object to that if other maintainers decided it's a good idea, but I
> lean slightly against it for the do-not-use-const-at-all reason below.

I guess the toss-up is between using const to help as much as it can, even though it can never be a perfect solution (for all the reasons above and in the mailing list thread), and the inconsistency that would cause in the API by having some things const and others not for seemingly no apparent reason.
Comment 7 Philip Withnall 2010-06-11 23:23:24 UTC
Just fyi, bug #614684, bug #397125, bug #303543, bug #583036 and bug #547199 are along the same lines, and might want to be taken into consideration.
Comment 8 Philip Withnall 2017-09-11 21:40:43 UTC
(In reply to Benjamin Otte (Company) from comment #2)
> At least it has been the policy in glib to not use const at all if it cannot
> be expressed well - which is why no GObject subclass is ever used const.

For this reason, let’s leave these APIs unmodified then.
Comment 9 Philip Withnall 2017-09-11 21:41:14 UTC
Review of attachment 157625 [details] [review]:

--