Code et qualité

Il y a des moments dans la vie où j'ai l'impression que le destin se fout ouvertement de ma gueule. Genre, maintenant.

Comme le lectorat assidu n'aura pas manqué de remarquer, je suis dans une période de recherche d'emploi. Un des critères que je ne veux surtout pas lâcher, c'est mon goût, voire mon besoin, pour la qualité. C'est d'ailleurs ce qui me fait penser que je serai super-malheureuse dans beaucoup de postes disponibles en informatique actuellement, où il est plus important de scotcher vaguement ensemble des trucs à la va-vite pour que ça ait l'air de marcher devant le Grand Chef.

Et avec le temps qui passe, je commence à me faire à l'idée de devoir me résigner et accepter de faire de la merde en étant malheureuse. C'est moche la vie, il faut tout négocier au rabais.

Bref, ce n'est pas le sujet, c'est juste le contexte général.

Le sujet, c'est que je suis encore plus ou moins le fork de ma bibliothèque de markdown, même si je sais que ça me fait du mal, il y a peut-être encore des choses intéressantes à en tirer.

Et j'ai vu une histoire de corruption mémoire dans vbufprintf() (bug 78), et mon sang n'a fait qu'un tour : la corruption mémoire, c'est un truc méga giga super grave de la mort qui tue, que j'aurais fait du roaming international sans hésiter pour le corriger si ça arrivait chez moi (et non pas laisser trainer le truc pendant plus de deux semaines, mais ça c'est une autre histoire), et vbufprintf() c'est un nom que j'ai fabriqué moi-même, je m'en rappelle très bien. Donc en gros, ça sentait furieusement le gros gros problème qui est de ma faute.

Le patch proposé pour corriger ça est… heu… “ce qu'il est”, quoi. Un problème dans vbufprintf() ? On supprime vbufprintf() ! Bon j'exagère un peu, mais ça reste un changement qui casse beaucoup de choses.

Interlude purement technique : les fonctions en *buf* gèrent des tampons dynamiques, et bufprintf() et vbufprintf() sont les homologues de sprintf() et vsprintf() pour ces tampons. Ces fonctions sont variadiques, mais comme le tampon est dynamique, il y a parfois besoin de l'agrandir, ce qui conduit à plusieurs appels à vsnprintf(), et la va_list ne serait pas réinitialisée entretemps, ce qui pose de gros problèmes et conduit effectivement à de la corruption mémoire. Le patch proposé supprime vbufprintf() pour l'inclure dans bufprintf(), en réinitialisant la va_list avec va_start() avant chaque appel de vsnprintf().

Mais ça, c'est basique tout ça. C'est trivial. C'est le paragraphe 3 de la section 7.15 du Standard C. Ce qui conduit directement à question qui est au cœur du problème : comment diable est-il possible que j'aie pu faire une telle erreur !? Comment ai-je pu laisser passer deux appels à vsnprintf() sans utiliser un va_copy() (qui est au passage la façon de résoudre le problème de corruption mémoire sans jeter l'eau du bain avec le bébé).

Je reste surprise par la quantité de temps qu'il m'a fallu pour me rendre compte de ce qu'il s'est passé. Depuis le tout premier jour, ma version de vbufprintf() utilise bien va_copy() pour réinitialiser proprement la va_list avant le deuxième vsnprintf(). Ça a juste été cassé par quelqu'un de mieux payé que moi dans un commit dont la description n'a rien à voir avec ce qui est arrivé à vbufprintf(). De toute façon ce commit m'efface de la liste du copyright pour ce fichier, c'est bien que je n'ai rien à voir avec les soucis qui s'y trouvent, non ?

Mais franchement, peu importe le salaire, je détesterais activement traiter du code comme ça. Plutôt nettoyer des toilettes toute la journée que faire ça.

J'ai juste de plus en plus de mal à espérer qu'autre chose m'attend…

Commentaires

1. Le mardi 15 novembre 2011 à 10:11, par Fred :

Finalement, au-delà de la qualité d'un code, il faut aussi souvent regarder son histoire.
Le dernier committer n'a pas fait cet effort qui lui aurait permis de remettre en place l'algo d'origine et le premier a juste voulu marquer son territoire en virant/renommant ton code.
Bref, chacun voit midi à sa porte ;)

2. Le jeudi 24 novembre 2011 à 22:39, par Natacha :

Ce n'est pas une question de voir midi en fait, c'est une question de voir ou non toute l'horloge.

Ce que je reproche au dernier « correctif », c'est de ne pas considérer l'ensemble du système. La bibliothèque de tampon dynamique, c'est justement une bibliothèque. Supprimer une fonction, c'est changer l'API, c'est casser tout ce qui en dépend. Et moi j'ai des tonnes de trucs qui en dépendent, donc c'est hors de question que je rapatrie un tel changement chez moi, à moins que ce soit la seule solution. Et toujours au niveau système, comment est-il possible d'avoir l'information « on ne peut pas réutiliser les va_list » sans avoir immédiatement à côté l'information sur comment le faire quand même, grâce à va_copy() ? Je n'ai pas trouvé de référence parlant de la réutilisation de va_list sans mentionner va_copy(), que ce soit les pages de man, le Standard C, etc.

L'histoire ça aurait juste un indice sur la bonne solution à l'échelle du système, mais même si j'avais fait l'erreur de réutiliser une va_list dans le code original, ça ne change rien à ce que le code devrait être.

Des gens qui n'ont pas daigné poster aussi m'ont dit que mon billet c'est juste « personne n'aime que l'on change son code ». C'est complètement faux. Peu importe que le code soit plus ou moins de moi, le seul impact que ça a c'est que c'est parce que je suis (de loin) à l'origine de ce code que j'en ai pris connaissance ; et peut-être aussi que j'arrive plus facilement que le quidam moyen à voir le système dans son ensemble. Encore une fois ça ne change rien à ce qui aurait dû être fait face à l'erreur rencontrée, quelque soit l'auteur dudit code.

En fait, ce que je dénonce profondément dans ce billet, c'est la mentalité de jeter n'importe quoi sur les symptômes tant que ça les fait disparaître, sans chercher à faire un diagnostic digne de ce nom. Et j'ai la nette impression que c'est extrêmement courant en informatique.

C'est comme donner de l'aspirine (ou peut-être un antalgique plus puissant) à quelqu'un qui a mal au pied à cause d'un caillou dans la chaussure. Ça marche, et le patient n'aura effectivement plus mal au pied après. Mais ce n'est pas ce que j'appelle une solution. Et en suivant cette métaphore, la fin de mon article c'est que je détesterais plus distribuer des antalgiques à ce genre patients que tous les boulots objectivement pénibles que j'arrive à imaginer.

Enfin il manque la vraie conclusion du billet, qui est que cette histoire me fait franchir le pas d'arrêter de suivre l'évolution de ce fork : le code est manifestement trop loin de ce que j'ai pu écrire pour que ce soit d'une quelconque utilité.

3. Le mardi 27 décembre 2011 à 20:31, par mathk :

Sinon pourquoi avoir utilisé du C. Car ce genre de chose peuvent facilement être éviter avec du lisp, ML, scheme, Smalltalk, ... C'est la ou l'on peut aussi conclure que le langage a malgré tous une influence sur la qualité. Je ne prétend pas que le langage vas forcement déterminer la qualité du code mais juste qu'il influence l'auteur.

4. Le mercredi 4 janvier 2012 à 19:02, par Natacha :

J'ai personnellement évité ce genres de choses avec le C ;-)

Plus sérieusement, il y a deux raisons pour lesquelles j'ai utilisé du C, chacune étant à mon avis suffisante à elle seule :

+ j'aime bien le C, et à l'époque c'était mon langage préféré (et il n'a été devancé depuis que par l'Ada, et seulement de peu) ; comme il s'agit d'une activité de loisir, choisir le langage qu'il me le plus agréable d'utiliser est pertinent, même s'il est peu adapté au problème posé ;

+ pour la légèreté, car tous les Unix sont fournis avec une libc, et beaucoup sont même fournis avec un compilateur C, de sorte que programmer en C (sous Unix) n'impose aucun prérequis supplémentaire, contrairement aux langages cités qui imposent tous un interpréteur et/ou un runtime pour fonctionner.

En revanche, il me semble extrêmement douteux sur un plan logique de conclure de cette anecdote, même si elle était généralisée, une influence du langage sur la qualité. On pourrait même argumenter qu'au contraire, un langage plus retors améliore la qualité (pour qui y prête attention) car il oblige a être tout le temps sur ses gardes ; alors qu'un langage qui pardonne plus serait plus propices aux erreurs parce que justement il ne pardonne pas tout.

5. Le mercredi 18 janvier 2012 à 22:02, par mathk :

C'est ici ou mon avis diffère de beaucoup de personnes. Et je te rejoint sur un point un langage qui "pardonne" n'est à mon avis pas un bon langage. Non, ce qui est important c'est la consistance. Et bien sûr peut de langages sont réellement consistants.
Python, Ruby, PHP sont tous loin de cette idées. C'est d'ailleurs assez criant car à chaque nouvelle version on voit arriver des nouvelles constructions du langage, qui a mon avis ne fait que combler des lacunes. Bon, bien évidement, ce n'est pas uniquement cela qui me fait dire qu'ils ne sont pas consistant, mais il me faudrait un peut plus de temps pour le démontrer. Bref, quand a dire pourquoi la consistance est important et bien c'est une question de communication. L'auteur et le lecteur s'affranchisse de la complexité syntaxique avec un langage consistant. On peut alors construire des entité sémantiquement complexe et non complexe au niveau de la syntaxe.
Cela dis le C ne me parait pas forcement un mauvais choix pour certaine application et c'est vrais la réalité des choses font que c'est parfois plus pratique de choisir C. Au final se que je trouve dommage c'est que l'on enseigne pas forcement les bon langage: (Haskell, Scheme, Prolog, Smalltalk). Et je comprend bien pourquoi mais je trouve cela triste.

Poster un commentaire

Nom ou pseudonyme :

Adresse e-mail (facultatif, ne sera pas publié) :

Site web (facultatif) :

Format de mise en forme : format historique Markdown restreint

Enregistrer ces informations dans un cookie

Commentaire (sera publié dans cette page, voir les conseils de mise en forme ci-dessous) :

Message personnel (ne sera pas publié) :

Mise en forme historique : pour mettre en valeur un mot ou un groupe de mot, tapez une étoile de part et d'autre, sans mettre d'espace entre l'étoile et le mot, comme ceci : *mise en valeur*. Pour insérer un lien, mettez le entre crochets, comme ceci : [http://instinctive.eu/].

Mise en forme markdown : voir le guide détaillé, en résumé c'est le Markdown traditionnel, sans HTML ni titres, mais avec les tables PHP-Markdown-Extra.

Attention : les balises HTML entrées dans les commentaires seront affichées telles quelles, et non pas interprétées.