Re: const fixes seek commit approval
- From: Havoc Pennington <rhpennin midway uchicago edu>
- To: gtk-devel-list redhat com
- Subject: Re: const fixes seek commit approval
- Date: Mon, 26 Oct 1998 11:47:48 -0600 (CST)
On 26 Oct 1998, Owen Taylor wrote:
>
> By 'no breakage' - are you saying that there are no
> additional warnings at all, or just that it did compile.
>
I changed the const stuff, then made clean and rebuilt over and over with
-Wall -ansi -pedantic, fixing stuff until all warnings that could
conceivably be my fault went away. (there are still a bunch of other
warnings, but I don't think I caused them :-)
As I said there are still two warnings in gtkfontsel.c. I think I fixed
everything "right", i.e. without using casts, and I was unsure what
"right" would be in those two cases.
> The problem with 'const' returns, is that what initially looks
> like a few innocuous warnings often can be solved either
> with casts (and casts are much worse than the absence of
> const, since they remove the ability of the compiler to
> notice any discrepancy in types), or with hundreds of lines
> of modifications to other parts of the code.
>
The patch is largish, true.The changes are trivial though, took a couple
hours and I doubt anything could be broken. The change is almost always
like this:
- char* ltext, rtext;
+ char* rtext;
+ const char* ltext;
The other change is adding const to a function arg, then to the args of
the functions it calls, etc., etc. until you run out of functions.
> char *some_string;
>
> Does _not_ mean "some modifiable string" - it means some string
> with indeterminate modifiability. So I would disagree with the
> statement that returning 'char *' is lying - it is merely
> giving incomplete information.
>
This is true for C. If it's const, you know it *isn't* modifiable, but if
it's not const things are indeterminate. For C++ this is theoretically not
the case, except when interfacing with C libraries. ;-)
> > So you are punishing the people who are doing things the right way,
> > because some people don't want to be pestered with a warning.
>
> 'punishing' is a strong term here - they perhaps don't get every
> possible benefit they could get, but they are not forced to
> add extra casts, or to give up own use of const.
>
> 'punishment' is perhaps more of an apt term for the poor programmer
> who is trying to mix use of an old non-const-correct interface,
> with a interface that returns 'const'. (I've been there,
> it isn't fun.)
>
I see your point; this is even the motivation for the patch. Gtk+ is the
last non-correct interface being used by my program. However it's true
that I don't think return values are causing any of the warnings in my
code.
> That sounds a bit odd - most of the cases, the string is
> not constant in view the widget, so you in fact need a
> (const char *) cast on the return to make it const...
>
C lets you do this implicitly... at least I get no warning.
> > Those things are not a huge deal, but they are IMO a much bigger deal than
> > the desire to avoid warnings for things that do indeed merit a warning.
>
> Things like:
>
> char *somevar = gtk_entry_get_text ();
> g_print (somevar);
>
> ? ;-) 95% of such cases do not merit a warning at all.
>
So put a const in front of the char*, problem solved. ;-)
> But, my inclination is still that constant returns are more trouble
> than they are worth - some support for this view can be found in the
> standard C library, which is const-correct on the arguments, but not
> on the returns.
>
OK, you have convinced me to be at least kind of neutral on the issue, and
I want to go ahead and get the const args stuff in. So I will commit a
const args patch tomorrow or so when I get back to a computer I can
compile stuff on, if there are no objections by then.
One remaining issue, what do you think of this kind of return value:
gtk_label_get(GtkLabel*, char** text);
I changed this to const char**, as you might predict. Is this a return
value or an arg for the purposes of redoing the patch? It looks to me like
it has the disadvantages (and advantages) of each, sadly. Sigh.
Havoc
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]