[Geany-devel] [patch] "Constify" some arguments and fields

Colomban Wendling lists.ban at xxxxx
Thu Apr 22 13:35:17 UTC 2010


Nick Treleaven a écrit :
> On Wed, 21 Apr 2010 22:51:33 +0200
> Colomban Wendling <lists.ban at herbesfolles.org> wrote:
> 
>> Hi,
>>
>> I'm used to compile my programs with quite strict compiler options [1],
>> and it made me see that the plugin API had some non-const arguments or
>> field where it probably should have const ones - where string literals
>> are most likely expected.
>> I think of keybindings_set_item(), plugin_signal_connect(), the
>> PluginInfo structure, etc. I think it is quite important since it makes
>> the plugin API cleaner (IMHO) and allows to write more clean plugins (no
>> more useless implicit promotions that may even be invalid).
> 
> I suppose this is due to source "foo" strings being const. 
Exactly.

>> Attached a patch with the suggested changes.
> 
> Applied, thanks.
No problem :)

>> On quite the same topic, would a patch that fixes a lot of small
>> problems like that in Geany's core be appreciated? It isn't hard to do
>> but I think it would clean the code a bit.
> 
> Yes, but only if it's clear that const is really meant, not just to
> silence compiler warnings.
Of course. Casting away should only be done where the cast is completely
wanted or at least carefully understood.
I'll then do it unless somebody else go faster than me.

> I don't like casting away const (e.g. keybindings_set_item) but for
> the API it's OK when 'safe'.
I'd wanted to make the internal pointer constant at the beginning, but I
saw that it is assigned to non-const strings if group->plugin is set, so
I searched a little the code before doing the cast. And it is because I
don't think such a cast is a normal situation that I added a small comment.

>> [1] -Wall -W -O2 -Wunused -Wunreachable-code -Wformat=2 -Wundef -Wshadow
>> -Wpointer-arith -Wbad-function-cast -Wwrite-strings -Wconversion
>> -Waggregate-return -Wstrict-prototypes -Wmissing-prototypes
>> -Wmissing-declarations -Wmissing-noreturn -Wmissing-format-attribute
>> -Wredundant-decls -Wnested-externs -pedantic
> 
> BTW, which one causes the const literal string warnings?
-Wwrite-strings. It makes string literals explicitly constant so such an
implicit cast is found.

> (You might want to add -ansi or -std=foo for portability.)
Yay true, even though most of the things that wouldn't be valid in ANSI
C already gets a warning with the options above (rather that simply
failing, I think e.g. of C++-style comments) with the current GCC default.

Ah, as we talk about compiler options, another think I like to add is
-Werror-implicit-function-declaration (that, as its name suggests, take
an implicit function declaration as an error), since such a situation is
most likely a bug in the program; and passing invalid arguments to
functions leads to strange bugs too anyway.

Regards,
Colomban



More information about the Devel mailing list