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 614684 - Make various parts of GObject const-correct
Make various parts of GObject const-correct
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gobject
unspecified
Other All
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2010-04-02 18:12 UTC by Philip Withnall
Modified: 2015-03-04 08:55 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Make various parts of GObject const-correct (12.95 KB, patch)
2010-04-02 18:13 UTC, Philip Withnall
rejected Details | Review
gobject: Mark a helper variable as const (856 bytes, patch)
2015-03-04 08:55 UTC, Philip Withnall
committed Details | Review

Description Philip Withnall 2010-04-02 18:12:56 UTC
Attached is a patch to make various parts of GObject const-correct. It obeys
the rule of thumb that reference counted types shouldn't use const, but there
are a few places where non-reference counted parameters were missing a const.

I don't know if this is acceptable re. API stability, but I thought I might
as well submit it anyway.
Comment 1 Philip Withnall 2010-04-02 18:13:00 UTC
Created attachment 157769 [details] [review]
Make various parts of GObject const-correct

Closes: bgo
Comment 2 Allison Karlitskaya (desrt) 2015-03-03 15:29:48 UTC
Review of attachment 157769 [details] [review]:

I'm not really interested in this patch -- I think it has the potential to do more harm than good.

Please feel free to commit the one fix about 'iindent' that I flagged below, but I think the rest of these are not very nice...

Is there some reason that you wanted to do this?  Trying to please a static analyzer or something?  Maybe with a bit more information, this patch could make more sense...

::: gobject/gobject-query.c
@@ +126,3 @@
   gboolean gen_tree = 0;
   gint i;
+  const gchar *iindent = "";

nice

::: gobject/gobject.h
@@ +405,3 @@
+gpointer    g_object_newv		      (GType             object_type,
+					       guint	         n_parameters,
+					       const GParameter *parameters);

I think this might be an overpromise... maybe one day we will want to modify these under certain conditions (eg: to steal the values out of them).
Comment 3 Philip Withnall 2015-03-04 08:55:31 UTC
(In reply to Ryan Lortie (desrt) from comment #2)
> I'm not really interested in this patch -- I think it has the potential to
> do more harm than good.

Agreed.

> Please feel free to commit the one fix about 'iindent' that I flagged below,
> but I think the rest of these are not very nice...

Done.

> Is there some reason that you wanted to do this?  Trying to please a static
> analyzer or something?  Maybe with a bit more information, this patch could
> make more sense...

This was before I got into static analysis, and was just a random cleanup I thought was appropriate at the time. Given I understand GLib conventions for opaque object-like structs better now, almost all of these changes are inappropriate. I don’t think there is a way to make this patch make more sense.


The following fix has been pushed:
6d030ea gobject: Mark a helper variable as const
Comment 4 Philip Withnall 2015-03-04 08:55:36 UTC
Created attachment 298504 [details] [review]
gobject: Mark a helper variable as const

It’s only used for argv values, which are not modified here.