arnaudus a écrit 5728 commentaires

il ne sera jamais audité et aura potentiellement plein de trou.

Mouais, enfin j'ai l'impression que l'actualité a démontré qu'un code très utilisé n'est jamais audité non plus et a potentiellement plein de trous également.

Mouais, c'est difficile de dire que c'est le mal absolu, mais le principe des conventions, en général, c'est que ça simplifie la vie de tout le monde quand elles sont respectées. Si dans ta fonction x c'est l'ordonnée et y c'est l'abscisse, il n'y a pas vraiment de faute de programmation, mais ça pourrit quand même la vie des autres.

c'est comme ça l'avenir du web?

C'est peut-être trollifère, mais pourquoi ne pas accepter de déléguer une tâche assez centrale et cruciale (l'autentification) à un tiers de confiance, plutôt que de laisser chaque site gérer de manière douteuse les procédures de login, changement de password, stockage des infos critiques, etc?

Bah, on ne parle pas de la même chose. Les designs patterns sont des abstractions de la structure d'un programme OO, le bouquin n'a pas la prétention d'expliquer comment on code proprement. Il s'agit de choses qu'on peut traiter en pseudocode par exemple, ça serait la même chose avec un bouquin sur la programmation fonctionnelle ou les algorithmes de tri...

Par ailleurs, la programmation object n'est pas un truc monolithique. La plupart des langages implémentent au moins des pseudo-classes (voire de simples listes) qui peuvent être utilisées pour mimer de la programmation objet, mais certains patterns nécessitent par exemple de l'héritage multiple ou des classes virtuelles, et là, on tombe tout de suite sur une poignée de langages.

Pour revenir au sujet du bouquin, les pires conseils que je n'ai jamais lus sur comment écrire du code propre venait de gens qui appliquaient des recettes qu'ils connaissaient sur leur langage de prédilection sur un autre langage: indenter du C++ comme on le fait en python, mettre une fonction par fichier en perl, terminer les lignes par un point-virgule alors que c'est optionnel dans le langage, etc.

Franchement, j'ai un peu de mal avec la notion de coder proprement sans faire référence au langage. À part des généralités triviales (commenter son code, donner des noms de variables signifiants, nommer les constantes, etc) qui prennent 2 ou 3 pages à énoncer, on rentre forcément dans les bonnes pratiques du langage. Comment parler des constantes sans parler de #define ou des enum? Comment définir un bon découpage des fichiers sans faire référence au langage? Je pense qu'un bouquin qui ne fait pas référence au langage sur sa couverture a de fortes chances d'être totalement inutile : soit trompeur (il porte sur un seul langage, avec très peu de chances que ça corresponde aux besoins du lecteur), soit trivial.

Non, ce genre de typo, ca arrive que le bug soit commenté ou pas. Et puis on ne peut pas non plus se fier au commentaire, parfois le code est bon, et c'est le commentaire qui est faux.

Les bugs arrivent que le code soit commenté ou non. Par contre, si la fonction commençait avec

/* Returns a file descriptor (positive number) or error code (negative number) */

Le bug aurait été évident. Or là, ça n'est pas évident. On a pu déduire que c'était un bug, mais ça aurait aussi pu être un hack dégueu qui fonctionne.

Ah, la programmation par accidents, c'est une bonne méthode de nos jours ?

Je n'arrive pas à savoir si c'était ironique ou non, mais je connais de nombreux cas où les fonctions sont utilisées pour leurs effets de bord...

plot(NULL, xmin=0, xmax=1, ymin=0, ymax=1);

par exemple produit un graphe vide du plus bel effect. Si le type qui a codé le truc ne voyait pas l'intérêt de faire ça, il va tester si le float** que tu lui passes n'est pas NULL et ne va rien faire.

Ça arrive super-souvent ce genre de trucs, et c'est super chiant : un type a décidé à ta place ce que tu devais faire ou non, alors que ça ne lui coutait rien.

Mais les contraintes dans les 2 cas ne sont pas exactement les mêmes.
En l'occurence : aucune pour tes projets persos, là où les autres doivent faire des compromis : délais, qualité, sécurité, performance, nouvelles fonctionnalité, maintenabilité, etc etc.

Bah oui, justement. J'ai l'impression que les écarts aux consignes de base ne sont absolument pas justifiables par ces raisons. Pour les délais, je t'accorde que "int err_code;" prend à peu près une seconde de plus à taper que "int r;", si cette seconde t'empêche de livrer ton programme dans les temps, c'est inquiétant. Commenter le code aussi ça prend du temps, mais s'il avait été commenté, il aurait peut-être été plus clair que "return -r;" était un bug.

J'en ai un peu marre de l'argument "oui mais c'est un pro, il sait ce qu'il fait, il doit bien avoir des raisons que tu es trop con pour comprendre". Pour que ça soit vrai, il faudrait pouvoir éliminer l'argument "flemme de coder proprement".

C'est toujours facile de prendre du code d'un autre et de trouver matière à critiquer.

Et c'est toujours formateur. Il y a une différence fondamentale entre critiquer le code des autres et prétendre que son code est parfait. C'est aussi en crtiquant les autres qu'on apprend beaucoup sur son propre code. Si je postais des bouts de code, je ne m'attendrais pas du tout à avoir des commentaires du style "ton code est parfait". Même pas "Tu codes mieux que Lennart Poettering", ce qui serait ridiculement prétentieux. Systemd fait des trucs dont je n'ai même pas idée de la complexité, la programmation est un métier, je n'ai absolument pas l'intention de prétendre que je pourrais faire mieux.

Mais quand je vois les bouts de code de Lennart qu'on a disséqué ici à titre d'exercice, l'impression que ça me fait, c'est comme de voir un chef trois étoiles qui sort des chiottes sans se laver les mains avant de te servir un plat super bon. Je fais la cuisine moins bien que lui, c'est certain. Mais je sais aussi que quand on fait à bouffer, on se lave les mains en sortant des toilettes. L'un n'empêche pas l'autre.

C'est les droits POSIX,

Oui oui je sais, tout le monde m'est tombé dessus parce qu'ils n'ont pas compris ce que je voulais dire. Mon argument, c'est que je ne vois pas de raison forte de faire un 0700 plutôt qu'un 0770 par exemple ; on pourrait très bien imaginer que pour des raisons de sécurité un autre process lancé de manière indépendante doive accéder à ce fichier, ou n'importe quoi. Bref, on peut imaginer qu'un jour, on puisse vouloir changer les permissions attribuées au fichier, et ce jour là, il faudra que le patch modifie des trucs partout dans le programme, avec de grosses possibilités de bugs ou de modifs non nécessaires, parce que le programmeur initial n'a pas indiqué la sémantique (qu'il connaissait pourtant). Juste en définissant dans un header PERM_ACCESS_PASSWD à 0700, le code n'est pas moins lisible, il n'est pas moins portable, il est juste mieux organisé avec toutes les permissions dans un seul fichier, les unes à la suite des autres : en un coup d'œil, on a l'intégralité de la logique de la gestion de permissions dans systemd.

Moi, c'est comme ça que je code, et pour des appli perso qui n'ont rien à voir avec la complexité et le caractère critique de systemd. Je suis juste surpris que des programmeurs professionnels de haut niveau ne respectent pas les consignes de base qu'on donne aux étudiants, et ont du mal à avoir un regard critique sur le code des autres : dans cette discussion, il y a plein de gens qui se prétendent programmeurs C qui ne voient rien à redire au code cité. Non seulement il y avait un bug (le return -r) dû à mon avis à la structure chaotique des returns, mais il y a plein de trucs moches : les variables qui s'appellent p, r, k, etc., les codes d'erreur mélangés, les chemins en dur, etc. Je veux bien qu'en programmation C, on puisse avoir des paradigmes différents (genre, utiliser la valeur de retour pour signaler les erreurs, ce qui me gène toujours), mais si on suit les commentaires des "pros", en gros, le paradigme du C est de coder comme des porcs et surtout de se serrer les coudes entre développeurs face au petit peuple qui demande comment un tel code peut être maintenu.

C'est un argument fallacieux. On peut critiquer la bouffe de la cantine sans être cuisinier, on peut critiquer le boulot d'un maçon sans être capable de faire mieux. C'est tout à fait légitime.

Merci :-) J'ai l'impression que les non-développeurs C qui se sont fait allumés sur cette page ont trouvé un bug que les méga-pro-développeurs C, qui trouvaient le code très clair, n'avaient pas vu.

Visiblement, toi tu l'es, par contre.

La question c'est, est ce qu'il y a une raison valable pour voulloir changer ca. Si il n'y en a pas, alors il ne faut pas qu'il y ait d'option pour le changer

C'est EXACTEMENT ce que répond Microsoft quand tu demande pourquoi il n'est pas prévu de booter un autre OS. Ça va largement au delà de l'informatique. Pourquoi s'emmerder à mettre des boitiers de dérivation quand on rajoute un truc électrique? On met les dominos dans le platre du mur et puis voila. Pourquoi s'emmerder à utiliser la balayette dans les toilettes? Le suivant va aussi y démouler son cake.

Ce n'est pas à toi de décider s'il y a des raisons valables de faire quelque chose ou non. Surtout pour du code libre : après tout, il est fait pour être réutilisé. Et surtout si la seule raison évidente, c'est la flemme de faire les choses correctement. Franchement, combien de fois chacun a pesté contre une API parce que les concepteurs ont bloqué une possibilité triviale, tout ça parce qu'eux n'en voyaient pas l'intérêt? On peut toujours vouloir appeler une fonction sur un pointeur NULL parce qu'elle a des effets de bord, on peut toujours vouloir passer une valeur négative quelque part parce qu'on a modifié un peu le code ailleurs, et que la fonction qui te bloque le fait juste par simple bêtise, parce que les devs ne voyaient pas l'intérêt de te laisser mettre un pointeur NULL ou une valeur négative. Alors oui, la probabilité que quelqu'un ait un jour besoin d'utiliser un truc dont on ne voit pas l'intérêt immédiat est, quoi, 10%? 1%? Mais comme ça arrive à des milliers d'endroits dans le code, tu es presque sûr que tu pourriras la vie de quelqu'un un jour ou l'autre (avec une très forte probabilité pour que la victime soit toi-même).

/proc c'est vraiment très très spécial. Je crois me souvenir qu'il y avait des raisons techniques pour forcer l'arborescence—en gros, /proc est virtuel, et ouvrir "/proc/self/maps" c'est un peu comme lire une variable qui s'appellerait proc_self_maps. Mais je crois aussi me souvenir que ça avait un peu gueulé au début, à cause justement du fait que la structure du répertoire était imposée.

On peut pinailler sur ce genre de trucs sur le code du noyau de la meme facon

Du peu que j'ai vu du code du noyau, c'est assez faux. Le code du noyau n'est pas forcément lisible, parce qu'il est écrit par des gens qui ne sont pas des programmeurs moyens, mais les fonctions sont hyper courtes, on a rarement plus de deux niveaux de if, il y a des millions de constantes pour tout, beaucoup de fonctions commencent par "int retval;" et dintinguent très clairement les retours d'erreurs et les retours de valeurs attendues, etc.

Je pense qu'on a relevé plein de petits trucs bizarres, pas très propres, pas très orthodoxes. Après, je ne suis pas développeur, et j'ai dit plus haut que le code n'était pas "pourri" : il fait juste un peu "premier jet". Si j'ai un étudiant qui me met les chemins en dur, je lui dis de refaire cette fonction pour qu'il aille lire les chemins dans une variable. S'il a 5 niveaux de if/else, je lui dis de fractionner son code en fonctions plus courtes. Je lui dirais aussi de ne pas appeler ses variables r, p, ou d.

À partir de ça, il serait con de conclure que systemd ne fonctionne pas, qu'il est buggué, ou qu'il n'est pas efficace. Par contre, c'est légitime de se poser des questions sur sa maturité, et sur le nombre de gens qui ont relu le code. Je ne comprends toujours pas le "return -r;" par exemple, c'est probablement un bug ou un truc pas net, simplement dû à la complexité de la fonction. Si elle avait un "int retval = 0;" au début, puis "retval = -r;", et "if (! (retval < 0))" avant le code qui fait vraiment quelque chose, il serait évident à la lecture que la fonction fait un truc idiot. Alors que là, on ne sait pas trop si retourner un code d'erreur positif n'est pas voulu, c'est bizarre quand même, non?

Je ne sais pas, je ne connais pas le kernel par cœur. Le noyau linux fait un open("/etc/passwd"); ?

Non, pardon, je me suis emmélé les pinceaux. Si get_ctty_devnr est négatif, il renvoir l'opposé (donc un truc positif). Mais pourquoi, alors que c'est visiblement un code d'erreur? Ailleurs dans le code de systemd (par exemple dans util.c), on a bien

 k = get_ctty_devnr(pid, &devnr);
 if (k < 0)
 return k;

qui me semble cohérent avec la logique "erreur = on arrête et on renvoie un nombre négatif".

C'est rigolo de tomber la-dessus dans un exemple pris pour discuter de la qualité du code. En tout cas, si c'est valable (si la fonction doit vraiment retourner l'opposé du code d'erreur), là, pour le coup, c'est de l'obfuscation évidente. Autrement c'est un bug.

Bah, par exemple, à la problématique du type qui gère une distribution et qui ne veut pas que les applications décident à sa place de l'arborescence du système?

Bref, ce bout de code m'a l'air correct.

Je n'ai jamais prétendu qu'il était incorrect! C'est juste que je le trouve crade. Rien ne t'empêche de définir une variable de retour au début, et de la retourner une seule fois à la fin.

Ça pinaille là. Ce sont des droits Unix. On peut utiliser les constantes symboliques, mais ça n'apporte pas grand chose, voir c'est contre-productif.

J'aurais dû expliquer, mais ce qui me gène, c'est la duplication de code. Ici tu as 0700, et dans une autre fonction tu as 0700, et dans un autre fichier tu as 0700, etc. Je ne voulais surtout pas dire qu'il fallait remplacer 0700 par une constante user_access_only, mais plutôt par une variable globale int permission_parent_dir = 0700; . En gros, dintinguer le signifiant du signifié, et faciliter la maintenance du code.

C'est un peu moche, mais c'est de la tambouille interne. Comme de toute façon, ils ont décidé que /run devait exister

Ouais, c'est super sympa pour les mainteneurs des distributions qui décideront d'organiser le système autrement. C'est juste de l'égoïsme, "Moi je ne vois pas pourquoi je changerais ça, donc je le met en dur". 1) tu n'es jamais sûr que tu n'auras pas besoin de le changer, et 2) tu n'es jamais sûr que quelqu'un d'autre n'aura pas besoin de le changer.

C'est du C. On a l'habitude d'abuser la sémantique. Si c'est "proprement" documenté :). Pas choqué

Abuser la sémantique est une chose, mais là, ça renvoie 4 trucs différents :
* l'opposé du retour de get_ctty_devnr si ce retour est positif
* -ENOMEM si l'asprint a foiré
* -errno si l'open a foiré
* le file descriptor si open a réussi
Il me semble que la sémantique, c'est que si le retour est positif, alors c'est le file descriptor, s'il est négatif, c'est n'importe quoi. Le traitement de l'erreur va être vachement facile.

Mouais, sans être dev C,

static int wall_tty_block(void) {
 char *p;
 int fd, r;
 dev_t devnr;
 r = get_ctty_devnr(0, &devnr);
 if (r < 0)
 return -r;
 if (asprintf(&p, "/run/systemd/ask-password-block/%u:%u", major(devnr), minor(devnr)) < 0)
 return -ENOMEM;
 mkdir_parents_label(p, 0700);
 mkfifo(p, 0600);
 fd = open(p, O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY);
 free(p);
 if (fd < 0)
 return -errno;
 return fd;
}

me semble un peu douteux.
* 4 return, alors qu'il y a un free(p) qui traine
* D'après ce que je comprends, la sémantique de ce que retourne la fonction dépend du fait que la fonction ait rencontré une erreur ou non. Bof, hein. sqrt(4) renvoie 2, mais sqrt(-4) renvoie -1, parce que -1 est un code d'erreur.
* Des magic values 0700, 0600.
* Des chemins codés en dur

Sans que ça ne soit du code spaghetti (ça n'est pas obfusqué quand même, il ne faut pas déconner), je pense qu'il existe des programmeurs susceptibles de ne pas trouver ça particulièrement propre.

Perso, ça me rappelle juste à quel point C doit être évité pour toute appli hors kernel, embarqué, compilo, ou composant critique du système.

Je pense qu'on pourrait créer très facilement un générateur aléatoire de commentaires de Linux sur les sorties des RC.

"OK, voila la RC $x. J'espère avoir tout intégré, j'ai fait les commits à partir du wifi de l'aéroport, pas très pratique. Rien de bien inhabituel, $pc_driv % des commits dans les drivers, $pc_files pour les systèmes de fichiers, pas mal de changements dans les architectures (notamment $pc_arm % pour divers ARM), le reste un peu partout. J'airemais bien que la prochaine RC soit encore plus petite, mais voila, je sais bien qu'on va m'envoyer tout un tas de trucs le vendredi soir. Retenez-bien ça : je ne veux plus que des corrections de régressions. Celui qui m'envoie une nouvelle fonctionnalité sera suspendu par les testicules dans le hall d'entrée de Microsoft. Pas la peine d'argumenter, la semaine prochaine, je vais manger des marshmallows à la piscine avec mes enfants."

Hello,

Merci pour le petit exemple, je trouve que ça peut faire d'excellents exercices de programmation pour débutants.

Par contre, pour des raisons pédagogiques, je ne comprend pas pourquoi truffer le code de "magic numbers". Non seulement c'est une pratique très critiquable qui rend le code très dur à maintenir, mais en plus c'est illisible : au choix, parmi

Crafty.init(320, 240, game);

ou

Crafty.init(world_width_px, world_height_px, game);

, je trouve que le deuxième est beaucoup plus pédagogique.

En ce qui me concerne, le sens interdit n'était pas indiqué dans OSM. Je viens de corriger.

Testé sur le trajet domicile-travail. En tout cas, je conforme deux choses :

• Le temps de parcours est imprécis. Il estime 24 minutes alors que le temps moyen est aux alentours de 20 minutes, soit quand même 25% d'erreur.
• Il me fait prendre les sens interdits (et d'ailleurs, dans la configuration précise que j'ai testé, le trajet réel est plus long que celui proposé, ce qui rend l'estimation de temps de parcours encore plus imprécise.

Sur le même trajet, Google maps me donne 25 minutes, mais me fait passer par la bonne route.

Le problème, je pense, c'est que les temps de trajet en ville sont difficile à estimer. Mais j'ai déja remarqué que les calculateurs d'itinéraires (GPS compris) ont tendance à les surestimer. Soit ils préfèrent prévoir large (histoire qu'on ne puisse pas leur reprocher des RDV ratés), soit ils partent sur la base de conditions de trafic dense, soit ils n'ont pas encore intégrés le remplacement systématique des feux tricolores par des giratoires, ce qui peut faire gagner beaucoup de temps.

Voila. Maintenant, Google sait où je travaille, quel chemin je prend, quand j'arrive au boulot, et ce que je vais manger à la cantine ce midi.

Je trouve ça hyper contre-intuitif. Tu mets la flèche en bas et il décale complètement à droite, il faut rechercher le carré bleu partout. À mon avis ça ne marche pas.