Le 08/03/2011 22:29, Colomban Wendling a écrit : > Le 08/03/2011 19:58, Enrico Tröger a écrit : >> On Tue, 08 Mar 2011 11:06:16 +0100, Frank wrote: >> >>> Am 23.02.2011 01:10, schrieb Matthew Brush: >>>> For first thing, maybe we could enforce use/passing of those tools >>>> mentioned and these before adding to release, examples: >>>> http://www.splint.org/ >>>> http://valgrind.org/info/tools.html >>>> (suppression for GTK - >>>> http://people.gnome.org/~johan/gtk.suppression) >>>> http://www.gnu.org/software/indent/ (just for making coding styles >>>> more consistent between plugins) http://check.sourceforge.net/ or >>>> http://cutest.sourceforge.net/ or http://cunit.sourceforge.net/ >>>> Perhaps some or all of these could be automated. > > I don't suggest (yet) to use splint, because I haven't found it useful, > it reported ways too much things (and some was false positive) to be > really usable. However, I probably don't know how to configure it (and > haven't the time to find out yet), so if somebody with experience with > it can provide some hints, maybe we can add this too. > > For Valgrind however, I doubt we can do anything automatically, because > it's a runtime checker, and its output must generally be read with some > care. Writing some sort of guidelines and howto is probably the way to go. > >>> >>> I like that idea. Can someone of you build up a howto on how to use it? >>> I did try valgrind in past and wished for some advice ;) >>> >>> One this is done we can think of automatic tests with some of this >>> tools. >> >> I, and obviously, Colomban as well, though indepdent from each other :D, >> recently played[1,2] with cppcheck. A small tool for static code >> analysis which actually found a few things in the geany-plugins >> repository. >> >> As I'm currently reworking the system to create the nightly builds, we >> could integrate such checks into the nightly builds, maybe run cppcheck >> on the sources after the builds and present the results somewhere on >> nightly.geany.org. > I think it's a good idea. > > I did a few checks, and this is what I suggest: > 1) run cppcheck on `make check` and abort if it detects an error; > 2) enable by default (though in a portable manner) some compiler flags > such as -Werror-implicit-function-declaration [1] [2] After a little more thinking and testing, I suggest:
* -Werror=implicit-function-declaration (see above); * -Werror=pointer-arith, to avoid portability issues with untyped pointer arithmetic; * -Wundef because preprocessor tests should rely on defined variables or explicitly use ifdef; * -Wshadow because shadowing symbols is a Bad Practice (and maybe it is even non-standard, not quite sure however); * -Waggregate-return because returning aggregate values is no good for performances and it's not a Good Practice in C; * -Wwrite-strings because using a string literal as a modifiable value may lead to crashes, and anyway good code should probably assume it's constant. I thing all is perfectly fine but maybe -Wwrite-strings. What -Wwrite-strings does is set all string literals ("foobar") constant (const char*). It is a good practice to assume string literals are constant, but however it is not necessarily wrong to assign it to a non-constant variable (if one actually don't modify the value), so it may have false positive. And actually it probably has: it adds lots of warnings in some plugins, and though I haven't checked, I doubt all are true errors. I think it'd be a good thing if no plugin would emit a warning using this, but maybe it's too soon for this, or maybe we don't want this at all, not sure. So, do you think it is or not a good idea to use it? And finally, I though of -Wstrict-prototypes, -Wmissing-prototypes and -Wmissing-declarations [1], but... * -Wstrict-prototypes even emits a warning in a GTK+ header and one in Scintilla.h; * -Wmissing-prototypes (and -Wmissing-declarations) emits a warning for plugin_init() and friends (though I thought it was fixed already?). ...So we probably don't want them by default. Moreover, the problem reported by -Wmissing-prototypes is not really important for preventing crashes, it's mostly a "beauty" thing. [1] not sure of the difference with -Wmissing-prototypes... > Maybe some other tests might be good, but I think this is a start. > > I'm working on implementing this for Autotools, maybe I can take a look > at Waf too (but Enrico would love to do this :D). > > Thoughts? Cheers, Colomban _______________________________________________ Geany-devel mailing list Geany-devel@uvena.de http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel