Discussion:
[grisbi-devel] Grisbi compiler warnings and bugs
Ludovic Rousseau
2016-10-06 18:44:08 UTC
Permalink
Bonsoir,

Je commence à corriger les warnings. Juste avec clang et le niveau de
warnings de base j'ai déjà plusieurs warnings inquiétants.

Par exemple j'en ai corrigé 1 qui est aussi un bug :
https://github.com/grisbi/grisbi/commit/f28fd44812a9112c670568849ae491a73f5e7d4e

Mon idée est de corriger tous les warnings et ensuite de configurer
Travis-CI avec -Werror pour qu'un nouveau warning fasse échouer la
compilation automatique.

à+
--
Dr. Ludovic Rousseau
Pierre Biava
2016-10-06 20:37:47 UTC
Permalink
Ludovic Rousseau a écrit le 06/10/2016 à 20:44 :

Bonsoir Ludovic,
Je commence à corriger les warnings. Juste avec clang et le niveau de
warnings de base j'ai déjà plusieurs warnings inquiétants.
https://github.com/grisbi/grisbi/commit/f28fd44812a9112c670568849ae491a73f5e7d4e
Des comme ça il doit y en avoir d'autres. Si on peut systématiquement
les trouver c'est bien. Jusqu'à présent seul le hasard ou si ça
conduisait à un bug effectif nous les faisait corriger. Après est-ce
inquiétant peut-être pas. Ça doit plutôt nous faire réfléchir sur la
pertinence du code ou la
nécessité de la fonction.
Mon idée est de corriger tous les warnings et ensuite de configurer
Travis-CI avec -Werror pour qu'un nouveau warning fasse échouer la
compilation automatique.
Ce serait bien. d'y arriver. Toutefois faire attention au type de
warning. Il est difficile de ne plus en avoir.

En parlant de warning j'ai une question par rapport à gcc. Ça fait
longtemps que gcc ne m'alerte plus sur les variables inutilisées. J'ai
testé plusieurs options mais aucune ne me rend cette erreur. Si tu avais
une idée pour réparer ça ce serait bien de modifier le configure.ac pour
que ça refonctionne.

Bonne soirée.
--
A+

Pierre Biava
Ludovic Rousseau
2016-10-07 06:56:49 UTC
Permalink
Post by Pierre Biava
Bonsoir Ludovic,
Post by Ludovic Rousseau
Je commence à corriger les warnings. Juste avec clang et le niveau de
warnings de base j'ai déjà plusieurs warnings inquiétants.
https://github.com/grisbi/grisbi/commit/f28fd44812a9112c6705
68849ae491a73f5e7d4e
Des comme ça il doit y en avoir d'autres. Si on peut systématiquement les
trouver c'est bien. Jusqu'à présent seul le hasard ou si ça conduisait à un
bug effectif nous les faisait corriger. AprÚs est-ce inquiétant peut-être
pas. Ça doit plutÃŽt nous faire réfléchir sur la pertinence du code ou la
nécessité de la fonction.
Les compilateurs C sont de mieux en mieux pour repérer ce genre de choses.
Ensuite il y a des outils d'analyse statique qui vont encore plus loin. Par
exemple llvm/clang [1] a une option pour faire ça.

Mon idée est de corriger tous les warnings et ensuite de configurer
Post by Pierre Biava
Post by Ludovic Rousseau
Travis-CI avec -Werror pour qu'un nouveau warning fasse échouer la
compilation automatique.
Ce serait bien. d'y arriver. Toutefois faire attention au type de warning.
Il est difficile de ne plus en avoir.
Oui.
Je ne vais pas corriger tous les warnings remontés en activant toutes les
options.

En parlant de warning j'ai une question par rapport à gcc. Ça fait
Post by Pierre Biava
longtemps que gcc ne m'alerte plus sur les variables inutilisées. J'ai
testé plusieurs options mais aucune ne me rend cette erreur. Si tu avais
une idée pour réparer ça ce serait bien de modifier le configure.ac pour
que ça refonctionne.
Je ne pense pas que modifier configure.ac soit une bonne idée. Par contre
tu peux te définir CFLAGS avec les bonnes options pour avoir les warnings
dans tes propres compilations.

Pour les variables inutilisées c'est -Wunused-variable qu'il faut utiliser
(avec gcc). L'option est incluse dans -Wall

$ cat unused.c
#include <stdio.h>

int main(void)
{
int pouet;
return 0;
}

$ CFLAGS="-Wunused-variable" make unused
cc -Wunused-variable unused.c -o unused
unused.c: In function ‘main’:
unused.c:5:6: warning: unused variable ‘pouet’ [-Wunused-variable]
int pouet;
^


Personnellement j'utilise CFLAGS="-Wall -g -O2 -Wextra -pipe
-funsigned-char -fstrict-aliasing -Wchar-subscripts -Wundef -Wshadow
-Wcast-align -Wwrite-strings -Wunused -Wuninitialized -Wpointer-arith
-Wredundant-decls -Winline -Wformat -Wformat-security -Wswitch-enum
-Winit-self -Wmissing-include-dirs -Wempty-body -fdiagnostics-color=auto
-Wmissing-prototypes -Wstrict-prototypes -Wold-style-definition
-Wbad-function-cast -Wnested-externs -Wmissing-declarations"
Et ça génÚre un trÚs gros paquet de warnigngs en compilant grisbi.

Pour l'instant j'ai corrigé les warnings signalés par gcc en ne passant
_aucune_ option. Donc les warnings qui signalent de vrais problÚmes.

à+

[1] http://clang-analyzer.llvm.org/
--
Dr. Ludovic Rousseau
Pierre Biava
2016-10-07 12:42:44 UTC
Permalink
CFLAGS="-Wall -g -O2 -Wextra -pipe -funsigned-char -fstrict-aliasing
-Wchar-subscripts -Wundef -Wshadow -Wcast-align -Wwrite-strings
-Wunused -Wuninitialized -Wpointer-arith -Wredundant-decls -Winline
-Wformat -Wformat-security -Wswitch-enum -Winit-self
-Wmissing-include-dirs -Wempty-body -fdiagnostics-color=auto
-Wmissing-prototypes -Wstrict-prototypes -Wold-style-definition
-Wbad-function-cast -Wnested-externs -Wmissing-declarations"
Effectivement ! il y a de quoi faire.

Bon après-midi
--
A+

Pierre Biava
Rémi Cardona
2016-10-21 06:36:36 UTC
Permalink
Post by Ludovic Rousseau
Bonsoir,
Je commence à corriger les warnings. Juste avec clang et le niveau de
warnings de base j'ai déjà plusieurs warnings inquiétants.
https://github.com/grisbi/grisbi/commit/f28fd44812a9112c670568849ae491a73f5e7d4e
Mon idée est de corriger tous les warnings et ensuite de configurer
Travis-CI avec -Werror pour qu'un nouveau warning fasse échouer la
compilation automatique.
C'est une super idée! Depuis le début, je compilais grisbi avec pas mal
d'options mais le passage à gtk3 a généré tellement de nouveaux warnings
que j'ai laissé tomber (honte sur moi). Idem pour static-analyzer de
clang, il m'a permis de corriger énormément de problèmes alors que le
projet était tout jeune. J'imagine qu'il a encore du s'améliorer depuis
et qu'il trouve moult nouveaux problèmes :)

Concernant les warnings, je suggère de s'inspirer de ce qui est fait
dans la galaxie Xorg:

* une macro XORG_TESTSET_CFLAG[1] qui teste qu'une option de compilateur
marche (du moins, ne fait pas planter la compilation d'un fichier vide)
* une macro XORG_COMPILER_FLAGS[2] qui définit une belle liste de
warnings à activer[3], et de warnings systématiquement transformés en
erreurs[4]
* une macro XORG_STRICT_OPTION[5] qui ajoute l'option
--enable-strict-compilation au ./configure pour facilement activer -Werror

Cette dernière option n'est pas activée par défaut afin de ne pas casser
inutilement la compilation de vieilles versions de paquets Xorg avec de
nouveaux compilateurs. Lorsqu'une personne travaille sur le code de
Xorg, il/elle n'a donc qu'à activer cette option pour avoir le niveau
maximum de sévérité.

Le code des macros m4 de Xorg est assez compliqué, voire velu, mais il
s'explique par le nombre d'OS et d'architectures (donc de compilateurs)
supportés, ainsi que par le nombre de paquets l'utilisant (20+
bibliothèques, 10+ pilotes, 30+ outils, le serveur X, etc).

J'ai pris Xorg parce que je connais plutôt bien ce projet, mais si
d'autres choses existent ailleurs (je pense à autoconf-archive
notamment[6][7]), allons-y!

Rémi

[1]
https://cgit.freedesktop.org/xorg/util/macros/tree/xorg-macros.m4.in#n1540
[2]
https://cgit.freedesktop.org/xorg/util/macros/tree/xorg-macros.m4.in#n1633
[3]
https://cgit.freedesktop.org/xorg/util/macros/tree/xorg-macros.m4.in#n1669
[4]
https://cgit.freedesktop.org/xorg/util/macros/tree/xorg-macros.m4.in#n1702
[5]
https://cgit.freedesktop.org/xorg/util/macros/tree/xorg-macros.m4.in#n1765
[6] https://cgit.freedesktop.org/systemd/systemd/tree/configure.ac#n148
[7]
http://www.gnu.org/software/autoconf-archive/ax_append_compile_flags.html#ax_append_compile_flags
Pierre Biava
2016-10-21 07:13:04 UTC
Permalink
Rémi Cardona a écrit le 21/10/2016 à 08:36 :

Bonjour Rémi,
Post by Rémi Cardona
* une macro XORG_STRICT_OPTION[5] qui ajoute l'option
--enable-strict-compilation au ./configure pour facilement activer -Werror
On ne peut pas activer toute la liste initiale des Warnings de Ludovic
car il y en a quelques uns qui sont intrinsèques à la construction du
code. Au stade actuel avec
CFLAGS="-Wall -Wextra -Wno-deprecated-declarations -Wno-unused-parameter
-Wno-unused-but-set-parameter -Wno-missing-field-initializers"
il n'y a pratiquement plus de warnings dans la compilation de Grisbi.

Ceux qui restent (3 je crois) sont volontaires.

Si on enlève : "-Wno-deprecated-declarations" il reste deux cas :

des gtk_alignement qui disparaîtrons facilement quand je reprendrais
quelques interfaces de dialogue et la gestion des icônes personnalisées
de Grisbi.

Pour ce qui concerne la sérialisation des icônes, aurais-tu une idée
pour la conserver ? Actuellement je penche pour la réintroduction des
icônes dans un répertoire utilisateur.

Bonne journée.
--
A+

Pierre Biava
Ludovic Rousseau
2016-10-26 16:37:20 UTC
Permalink
Bonjour Rémi,
* une macro XORG_STRICT_OPTION[5] qui ajoute l'option
Post by Rémi Cardona
--enable-strict-compilation au ./configure pour facilement activer -Werror
On ne peut pas activer toute la liste initiale des Warnings de Ludovic
car il y en a quelques uns qui sont intrinsÚques à la construction du code.
Au stade actuel avec
CFLAGS="-Wall -Wextra -Wno-deprecated-declarations -Wno-unused-parameter
-Wno-unused-but-set-parameter -Wno-missing-field-initializers"
il n'y a pratiquement plus de warnings dans la compilation de Grisbi.
Avec "-Wall -Wextra -Wno-deprecated-declarations -Wno-unused-parameter" il
n'y a plus _du tout_ de warnings que ce soit avec gcc 4.8.4 ou clang 7.3.0.
C'est la configuration utilisée pour les builds sur Travis-CI et j'ai en
plus activé -"-enable-werror" donc au moindre nouveau warning la
compilation échoue.

à+
--
Dr. Ludovic Rousseau
Ludovic Rousseau
2016-10-26 17:54:39 UTC
Permalink
Post by Ludovic Rousseau
Post by Ludovic Rousseau
Bonsoir,
Je commence à corriger les warnings. Juste avec clang et le niveau de
warnings de base j'ai déjà plusieurs warnings inquiétants.
https://github.com/grisbi/grisbi/commit/f28fd44812a9112c670568849ae491
a73f5e7d4e
Post by Ludovic Rousseau
Mon idée est de corriger tous les warnings et ensuite de configurer
Travis-CI avec -Werror pour qu'un nouveau warning fasse échouer la
compilation automatique.
C'est une super idée! Depuis le début, je compilais grisbi avec pas mal
d'options mais le passage à gtk3 a généré tellement de nouveaux warnings
que j'ai laissé tomber (honte sur moi). Idem pour static-analyzer de
clang, il m'a permis de corriger énormément de problÚmes alors que le
projet était tout jeune. J'imagine qu'il a encore du s'améliorer depuis
et qu'il trouve moult nouveaux problÚmes :)
Concernant les warnings, je suggÚre de s'inspirer de ce qui est fait
* une macro XORG_TESTSET_CFLAG[1] qui teste qu'une option de compilateur
marche (du moins, ne fait pas planter la compilation d'un fichier vide)
* une macro XORG_COMPILER_FLAGS[2] qui définit une belle liste de
warnings à activer[3], et de warnings systématiquement transformés en
erreurs[4]
* une macro XORG_STRICT_OPTION[5] qui ajoute l'option
--enable-strict-compilation au ./configure pour facilement activer -Werror
Cette derniÚre option n'est pas activée par défaut afin de ne pas casser
inutilement la compilation de vieilles versions de paquets Xorg avec de
nouveaux compilateurs. Lorsqu'une personne travaille sur le code de
Xorg, il/elle n'a donc qu'à activer cette option pour avoir le niveau
maximum de sévérité.
Le code des macros m4 de Xorg est assez compliqué, voire velu, mais il
s'explique par le nombre d'OS et d'architectures (donc de compilateurs)
supportés, ainsi que par le nombre de paquets l'utilisant (20+
bibliothÚques, 10+ pilotes, 30+ outils, le serveur X, etc).
J'ai pris Xorg parce que je connais plutÃŽt bien ce projet, mais si
d'autres choses existent ailleurs (je pense à autoconf-archive
notamment[6][7]), allons-y!
Super. Je ne connaissais pas cet aspect de Xorg.

Avec clang 8.0.0 la macro XORG_COMPILER_FLAGS m'a trouvé :
BASE_CFLAGS = -Wall -Wpointer-arith -Wmissing-declarations -Wformat=2
-Wstrict-prototypes -Wmissing-prototypes -Wnested-externs
-Wbad-function-cast -Wold-style-definition -Wdeclaration-after-statement
-Wunused -Wuninitialized -Wshadow -Wmissing-noreturn
-Wmissing-format-attribute -Wredundant-decls -Werror=implicit
-Werror=nonnull -Werror=init-self -Werror=main -Werror=missing-braces
-Werror=sequence-point -Werror=return-type -Werror=trigraphs
-Werror=array-bounds -Werror=write-strings -Werror=address
-Werror=int-to-pointer-cast -Werror=pointer-to-int-cast

Il semble que toutes les options testées par XORG_COMPILER_FLAGS (sauf
-Wlogical-op) soient présentes :
[...]
checking whether __clang__ is declared... yes
checking whether __INTEL_COMPILER is declared... no
checking whether __SUNPRO_C is declared... no
checking if gcc supports -Werror=unknown-warning-option... yes
checking if gcc supports -Werror=unused-command-line-argument... yes
checking if gcc supports -Wall... yes
checking if gcc supports -Wpointer-arith... yes
checking if gcc supports -Wmissing-declarations... yes
checking if gcc supports -Wformat=2... yes
checking if gcc supports -Wstrict-prototypes... yes
checking if gcc supports -Wmissing-prototypes... yes
checking if gcc supports -Wnested-externs... yes
checking if gcc supports -Wbad-function-cast... yes
checking if gcc supports -Wold-style-definition... yes
checking if gcc supports -Wdeclaration-after-statement... yes
checking if gcc supports -Wunused... yes
checking if gcc supports -Wuninitialized... yes
checking if gcc supports -Wshadow... yes
checking if gcc supports -Wmissing-noreturn... yes
checking if gcc supports -Wmissing-format-attribute... yes
checking if gcc supports -Wredundant-decls... yes
checking if gcc supports -Wlogical-op... no
checking if gcc supports -Werror=implicit... yes
checking if gcc supports -Werror=nonnull... yes
checking if gcc supports -Werror=init-self... yes
checking if gcc supports -Werror=main... yes
checking if gcc supports -Werror=missing-braces... yes
checking if gcc supports -Werror=sequence-point... yes
checking if gcc supports -Werror=return-type... yes
checking if gcc supports -Werror=trigraphs... yes
checking if gcc supports -Werror=array-bounds... yes
checking if gcc supports -Werror=write-strings... yes
checking if gcc supports -Werror=address... yes
checking if gcc supports -Werror=int-to-pointer-cast... yes
checking if gcc supports -Werror=pointer-to-int-cast... yes
checking if gcc supports -pedantic... yes
checking if gcc supports -Werror... yes
checking if gcc supports -Werror=attributes... yes
[...]

Je crois qu'il va falloir activer une options une par une. J'ai un peu trop
de warnings si j'utilise tout d'un coup :-)

à+
Post by Ludovic Rousseau
Rémi
[1]
https://cgit.freedesktop.org/xorg/util/macros/tree/xorg-macros.m4.in#n1540
[2]
https://cgit.freedesktop.org/xorg/util/macros/tree/xorg-macros.m4.in#n1633
[3]
https://cgit.freedesktop.org/xorg/util/macros/tree/xorg-macros.m4.in#n1669
[4]
https://cgit.freedesktop.org/xorg/util/macros/tree/xorg-macros.m4.in#n1702
[5]
https://cgit.freedesktop.org/xorg/util/macros/tree/xorg-macros.m4.in#n1765
[6] https://cgit.freedesktop.org/systemd/systemd/tree/configure.ac#n148
[7]
http://www.gnu.org/software/autoconf-archive/ax_append_
compile_flags.html#ax_append_compile_flags
_______________________________________________
devel mailing list
http://listes.grisbi.org/mailman/listinfo/devel
--
Dr. Ludovic Rousseau
Loading...