Journal [PHP] Apache Check, première release

Posté par  (site Web personnel) . Licence CC By‑SA.
32
2
août
2021

Introduction

Les vacances sont un moment adéquat pour se lancer dans de petits projets qu’on reporte sans cesse, procrastinateur que nous sommes. Cette fois-ci, j’ai eu l’élan nécessaire pour me lancer dans l’écriture d’un petit script PHP nommé Apache Check.

Apache Check est donc un script PHP qui vérifie la configuration d'Apache HTTP Server sur un server LAMP (Linux-Apache-Mysql-PHP).

Ce script analyse les fichiers de configuration d'Apache, ses processus et ses consommations mémoire. Il analyse également les consommations mémoire de PHP-FPM et MySql afin d’estimer la consommation mémoire totale dans différentes situations. Il fait quelques vérifications des valeurs des paramètres d'Apache, et indique les éventuels problèmes critiques, des alertes et des recommandations. Vous utiliserez (ou pas, vous êtes libres) ce script à vos risques et périls, vous êtes seul responsable des conséquences des modifications que vous apportez à votre système.

Ce script est sous licence Apache 2.0, donc compatible GPL. Pourquoi choisir cette licence Apache 2.0 ? En fait, ce script s’inspire un peu d’un vieux script non-maintenu « apachebuddy » et d’un moins vieux mais plus vraiment actif « apache2buddy » qui est sous cette licence. Ce dernier script, écrit en Perl, n’a pas évolué depuis longtemps et son mainteneur refuse quasi-systématiquement toutes les demandes d’amélioration. Par exemple il ne prend en compte que le MPM prefork (sans mpm-itk), donc ne fonctionne pas sur les configurations utilisant les MPM worker et event, pourtant de plus en plus utilisés. Les distros ne sont pas non plus toutes à la fête, seules certaines versions sont supportées. Ainsi un Apache sur une Ubuntu toute fraîche ne pourra pas être analysé. J'ai donc voulu apporter un peu de neuf.

Pour les curieux qui jetteront un coup d’œil au code, soyez assis avant d’ouvrir le fichier : je code avec des partis pris (moi aussi je suis libre !). Par exemple je n’utilise pas de regex, je déteste ça, je trouve ça illisible, même si je comprends fort bien que ce qui est illisible pour l’un est peut-être parfaitement lisible par d’autres, chacun voit midi à sa porte. Autre exemple, je limite au maximum les dépendances du genre ps, awk, pmap, grep … ce qui oblige à aller taper en direct dans le pseudo-système de fichiers /proc pour y trouver les infos qu’il faut. L’avantage, c’est que c’est sans doute plus fiable que d’utiliser des programmes externes dont les formats de sortie peuvent évoluer dans le temps, ou dont les options diffèrent d’une distro à l’autre.

Utilisation

Commencez par télécharger le script ici, décompressez le (par exemple avec unzip apachecheck_v1.0.zip), et copiez le script PHP sur votre serveur.

Si PHP CLI (Interpréteur en ligne de commande) n'est pas installé, installez-le (par exemple apt-get install php-cli pour les distributions basées sur Debian).

En tant qu'utilisateur root, exécutez le script : php apachecheck.php

Puisqu'il doit scruter les processus et les fichiers de description présents dans le répertoire /proc, il doit être exécuté avec les droits root sur le serveur, pas d’autre choix.

Vous devriez alors avoir un résultat qui ressemble à ça :

Exemple de result d'Apache Check

Conclusion

Bref, c’est une première version, qui n’a été testée que sur des serveurs Debian et dérivés. Je suis donc preneur de tous les bugs qu’on puisse trouver sur tous les environnements possibles. Un lien vers le site du projet est disponible ci-dessous.

Les calculs sur les empreintes mémoires sont estimatifs. Sous linux, il est difficile de savoir exactement combien consomme un processus, en partie parce que les librairies sont partagées.

Il reprend le style de sortie du fameux script mysqltuner.sh, mais sans être aussi efficace, hélas. En effet, MySql donne assez facilement toutes les infos voulues par simples requêtes. Apache est beaucoup moins bavard, et il n'est pas question de forcer l'installation les modules status_module et mod_info.

Je suis aussi ouvert à l’ajout d’autres consommateurs de mémoire en complément de PHP-FPM et MySql, tant qu’on me fournit suffisamment d’infos, et tant que leur consommation mémoire est liée à l’augmentation de la charge du serveur Apache : autres langages en FastCGI (les modules utilisés en mpm-prefork sont déjà comptabilisés puisque intégrés dans les processus Apache), système de mise en cache …

Site du projet : Apache Check

  • # Outil intéressant mais...

    Posté par  (site Web personnel) . Évalué à 10 (+24/-0). Dernière modification le 03/08/21 à 09:47.

    Bonjour, le PHP (entre autres) étant mon métier de tous les jours, je me permet de faire quelques remarques sur le code:

    • ce script semble être développé "à l'ancienne", un fichier de 1500 lignes avec très peu de fonctions, et quasi-intégralement procédural, c'est un peu lourd à digérer,

    • je vois beaucoup de display("[..] Foo bar... ça aurait peut être été judicieux de factoriser le code pour générer le [..],

    • tu itères bien souvent avec readdir(), je ne suis pas vraiment sûr que ce soit nécessaire, tu pourrais déduire les noms de fichiers que tu cherches dans la majeure partie des cas et simplement faire un $filename = sprintf('/proc/%d/status', $pid); if (file_exists($filename)) { $status = file_get_contents($filename); /* ... */ },

    • tu fais du shell_exec() sans utiliser escapeshellcmd() ni escapeshellarg(), alors le plus souvent c'est un string literal donc c'est pas grave, mais à garder dans un coin de la tête pour plus tard,

    • de manière générale le code est très redondant, il pourrait être factorisé,

    • tu dis que tu ne trouves pas les regex lisibles, et je suis bien souvent d'accord avec toi, mais je dirais surtout: ça dépend, et dans ton cas en utilisant des regex par endroit, tu aurais du code plus concis (et plutôt lisible je pense, car les regex dont tu aurais besoin serait relativement simples).

    Bon sinon l'outil semble fonctionnellement très bien, par contre je trouve ça dangereux de hardcoder les limites acceptables en ce qui concerne, par exemple, le nombre de threads: en effet, un nombre de process/thread sera bien souvent affiné selon le type d'application (I/O-bound ou CPU-bound), selon le nombre de CPU dédiés sur la machine, selon la RAM disponible, etc… Donc j'ai peur que toutes les valeurs soient de facto fausses pour beaucoup d'environnements.

    Mais, petite note positive, c'est cool de reprendre ce genre de petit projet, je suis sûr que ça peut servir à beaucoup de monde. Apache est trop souvent bien trop mal configuré, alors c'est une bonne chose, ça permet de sensibiliser les gens à la configuration.

    Et puis quant bien même je fais des critiques (que j'espère être constructives) l'important c'est que tu te fasses plaisir à développer cet outil.

    • [^] # Re: Outil intéressant mais...

      Posté par  (site Web personnel) . Évalué à 7 (+6/-0).

      Merci pour tes remarques et le temps passé que tu as passé sur le code.

      Oui, c'est écrit en procédural, à l'ancienne. Le faire en objet, pourquoi pas, mais il n'y a pas vraiment d'intérêt dans ce cas-ci. Et puis comme ça c'est facile à suivre : ce qui sort à l'écran est relativement proportionnel au code, donc facile de trouver la partie du code qui affiche telle ou telle ligne.

      Pour les display("[..] foo bar");, je trouve plus lisible de les avoir comme ça plutôt que d'avoir 4 fonctions display_ok("foo bar");, display_alert("foo bar");
      Mais la lisibilité, c'est sans doute quelque chose d'assez relatif, variable d'une personne à l'autre.

      J'avoue que je n'ai pas compris le truc qui éviterait les readdir(), si tu peux détailler (idéalement avec un exemple) … ça m'intéresse.

      Pour les shell_exec(), il n'y a pas d'entrées utilisateur, pas de noms bizarres, donc j'ai en effet fait l'impasse sur les escapeshellarg(), fainéant que je suis.

      Pour les regex, j'en ai parlé parce que ce script est pour moi un peu l'exception qui confirmerait ma règle (je code principalement des sites web). Je suis d'accord avec toi que dans le cas présent, l'usage de regex pourrait alléger le code, mais comme je n'ai pas l'habitude d'en écrire… ben voilà le résultat.

      Les seuls tests hardcodés pour les threads, c'est ThreadsPerChild :
      1- test si < 20, parce que moins de threads par process, c'est limiter le partage des allocations mémoire, ce qui est justement tout l'intérêt des threads (avec la vitesse de création par rapport à un fork de processus).
      2- test si > 20000, qui est une valeur très très haute (valeur par defaut : 25). Confier 20000 threads à un seul processus, ça me fait penser à cette citation : "Celui qui avale une noix de coco entière fait confiance à son anus." Que le processus merdoie et c'est 20000 threads qui partent en sucette. Il serait probablement plus sage de réduire le nombre de threads et augmenter le nombre de processus. Mais 20000 est pifométrique, je suis prêt à changer pour une autre valeur s'il y a un retour d'expérience qui va dans ce sens.
      Pour les MinSpareThreads et MaxSpareThreads, c'est fonction de la quantité de RAM et de l'activation d'HTTP/2.
      Globalement, ce n'est pas évident de déterminer des seuils ou des valeurs adéquates pour un certain nombre de paramètres en se basant uniquement sur les données que le script peut choper dans mod_status et mod_info. Et puis il y a aussi une très grande diversité dans les uages d'Apache qui complique les choses.

      Tu as tout bien compris : c'est un plaisir de le faire, et j'espère que ça aidera avant tout les gens qui ne maîtrisent pas du tout Apache. Les pros d'Apache (que je ne suis pas, d'ailleurs) n'y trouveront sans doute pas leur compte, mais je suis preneur de leurs remarques, comme des tiennes.

      • [^] # Re: Outil intéressant mais...

        Posté par  (site Web personnel) . Évalué à 9 (+9/-1). Dernière modification le 03/08/21 à 14:32.

        Je ne parlais pas forcément de passer en objet, pour un script comme celui-ci ça ne semble pas indispensable (ce qu'apporte réellement l'objet qui est indispensable pour des gros projets c'est essentiellement l'encapsulation, les interfaces et la composition, afin de rendre le code évolutif, ce qui ici ne serait pas forcément bénéfique), mais plus de plus découper en fonctions. À certains endroits ton code arrive à des niveau de 5 ou 6 indentations successives (entre divers if, while et foreach), ce qui rend le code très profond, et plus difficile à lire. Le cerveau humain est assez bête il aime lire de façon linéaire, et le découpage en fonctions aux endroits opportuns créé une encapsulation qui permet aux algorithmes importants d'évoluer à niveau plus haut ce qui leur donne plus de sémantique et moins de détails techniques, et d'être plus lisibles, car ça les rend plus linéaires.

        Les imbrications de if foreach while et autres joyeusetés créé un code avec énormément de branches, c'est souvent là qu'on perd le lecteur. En arrivant à remplacer les branches par du typage et une gestion d'erreur fine dans des fonctions encapsulés, ça permet de mettre en valeur les erreurs évidentes.

        • [^] # Re: Outil intéressant mais...

          Posté par  (site Web personnel) . Évalué à 7 (+9/-3).

          À certains endroits ton code arrive à des niveau de 5 ou 6 indentations successives (entre divers if, while et foreach), ce qui rend le code très profond, et plus difficile à lire. Le cerveau humain est assez bête il aime lire de façon linéaire, et le découpage en fonctions aux endroits opportuns créé une encapsulation qui permet aux algorithmes importants d'évoluer à niveau plus haut ce qui leur donne plus de sémantique et moins de détails techniques, et d'être plus lisibles, car ça les rend plus linéaires.

          Heu… 5 ou 6 indentations, mouais, bof, c'est quand même super banal.
          Tant qu'il y a une rigueur dans l'indentation (toujours une tabulation, ou toujours 4 espaces… bref un choix constant) et que l'éditeur est un tant soit peu correct (j'utilise Geany qui est assez bien pour ça), ça reste très clair.
          J'avais en mémoire un cas où j'avais indenté comme pas possible, du coup je suis allé vérifier : j'avais en 2003 codé un serveur smtp en PHP genre postfix, et qui gère encore mes mails aujourd'hui. J'avais du monter à 23 niveaux d'indentation. C'est certes un cas particulier, mais pas non plus difficile à lire pour autant.

          Pour ce qui est des fonctions, si tu jettes un oeil au script apache2buddy.pl dont je me suis légèrement inspiré, tu verras que c'est blindé de fonctions … qui ne sont appelées qu'une seule fois. Et comme en plus elles sont dans n'importe quel ordre, pour suivre le fil du programme, faut sans arrêt faire des sauts d'un bout à l'autre du script.
          Pas vraiment pratique à lire, ni linéaire pour le coup.

          J'avoue que je code avec des pratiques qui ne sont sans doute pas celles qui sont utilisées par la majorité des devs, mais ça fonctionne, c'est au moins ça. Et puis à mon âge, il y a une certaine résistance naturelle au changement  ;-)

          • [^] # Re: Outil intéressant mais...

            Posté par  (site Web personnel) . Évalué à 10 (+14/-1). Dernière modification le 03/08/21 à 17:47.

            Bien que je ne sois pas d'accord sur le fond, je t’ai pertinenté parce que j'estime que ton point de vue est tout à fait légitime, je ne pense pas que ça vaille d'être moinssé.

            De manière générale, on essaye au boulot de ne pas faire plus de 2 ou 3 niveau max d'indentation, et dès lors qu'un algo déroulé est plus grand que l'écran (alors c'est subjectif parce qu'on est nombreux à avoir des écrans/résolutions/polices de taille différentes) on découpe.

            Avoir une fonction qui n'est appelée qu'une unique fois n'est pas un soucis, si ce qu'elle fait est particulièrement technique ou dévie un du peu métier (tout en restant nécessaire), ça permet juste de sortir quelques lignes inutiles à la compréhension de son algo d'origine, afin de fluidifier la lisibilité de ce qui est vraiment "métier".

            L'idée ce n'est pas de voir la fonction comme quelque chose de technique, par essence ce n'est pas dédié que à la factorisation, mais juste de séparer un gros problème en un ensemble de plus petits problèmes, parfois décorrélés, parfois non, afin de mettre en valeur, donner une sémantique métier, au code. Par exemple si quelqu'un dit "oui mais un appel de fonction c'est plus lent", il faut bannir cette personne de toute décision architecturale, car même si c'est possiblement vrai, ça ne sera jamais significatif dans un ensemble de code plus gros.

            Le découpage unitaire permet aussi le test unitaire, et ça, c'est vraiment cool. Plus les responsabilités sont découpés, plus il aisé d'écrire des tests, aux limites ou non. Mais c'est aussi plus facile de remplacer un bout de code, il suffit de respecter la signature.

            Ça permet aussi d'éviter que des variables temporaires se trimballent trop longtemps dans le flux d'exécution, ou dans le flux de code, et permet d'éviter des erreurs bêtes (écrasement de variable, erreur de typo qui fait que tu utilises une variable à la place d'une autre).

            Je terminerais avec deux points, qui me semblent importants, dans les deux paragraphes en dessous.

            Le premier, c'est que regarder les scripts des autres (qui plus est en perl), n'est pas un bon indicateur pour comparer la qualité du code. Le choix du langage, tout comme la finalité du code, ou tout simplement les conventions choisies vont influer sur le style adopté. De plus, les scripts perl (ou tout autre langage d'ailleurs) de configuration ou de monitoring sont très souvent écrits par des gens dont le métier est l'administration ou la maintenance de parc, et malheureusement, ce n'est absolument pas le même métier que développeur (d'application métier ou tout autre) et de gestion de projet; bien souvent les scripts d'admin sont de piètre qualité. Je ne remets pas en cause leur compétence ici, attention, mais juste développer n'est pas leur métier (je suis résolument contre le discours qu'il faille absolument être un "devops", ce sont deux métier différent, qui demandent des compétences tout à fait différentes).

            Le deuxième point, et c'est le dernier, c'est que peu importe l'âge, on est toujours capable d'avancer et d'évoluer, c'est plus un choix qu'une contrainte en réalité. Si tu décides d'être conservateur quant à ton style de code, je ne vais pas aller te critiquer, c'est un choix personnel et légitime, et l'important, encore une fois, c'est que tu te fasses plaisir. Par contre, si tu étais développeur dans mon équipe, je serais sûrement derrière ton dos pour t'aider à avancer, et surtout à comprendre, ces "nouvelles" (qui ne sont pas nouvelles du tout en réalité) méthodes de développement, non pas pour forcer le changement, mais tout simplement pour améliorer la communication entre développeur, en espérant qu'à la fin, tout le monde y prenne du plaisir.

            • [^] # Re: Outil intéressant mais...

              Posté par  (site Web personnel) . Évalué à 7 (+6/-0).

              Bien que je ne sois pas d'accord sur le fond, je t’ai pertinenté parce que j'estime que ton point de vue est tout à fait légitime, je ne pense pas que ça vaille d'être moinssé.

              C'est tout à ton honneur, car hélas beaucoup confondent "pertinent/inutile" et "d'accord/pas d'accord", ce qui nuit souvent à l'expression de la diversité des opinions.

              Toutes tes remarques sur la façon de coder sont légitimes et pertinentes.
              Sur ma résistance au changement, en fait j'accepte bien volontiers d'apprendre de nouvelles choses, d'évoluer, de m'adapter… mais force m'est de reconnaître que plus on vieillit, plus c'est difficile et lent.
              Pour prendre un exemple en dehors de l'informatique, je vis en Chine depuis quelques années, donc je constate que lors de l'apprentissage du chinois, c'est plus difficile de retenir tous les nouveaux mots, tous les sinogrammes et toutes les expressions qu'à vingt ans, le cerveau n'est plus aussi souple pour s'adapter rapidement à la façon de tourner les phrases, etc.
              Ca ne veut pas dire que je n'y arrive pas, juste qu'il faut plus de temps et y consacrer plus d'énergie.
              Pour revenir au code, un autre facteur entre en jeu : quand tu as une façon de faire qui marche bien, qui a fait ses preuves (des ERP que j'ai codé il y a 20 ans tournent toujours et d'autres devs les font évoluer encore aujourd'hui), tu as plus de mal à changer pour utiliser une méthode qui à tes yeux présente davantage d'inconnues, avec moins de recul personnel.

              Mais dans le cadre de cet outil, tout ça n'est pas bien grave, car c'est libre !
              Ceux qui souhaitent le même type d'outil codé d'une autre façon (ou dans un autre langage), sont libres aussi de le forker, ou juste de s'en inspirer. Je n'y vois pas d'inconvénients, au contraire.
              Et puis finalement, puisque c'est du libre fait sur mon temps libre, je le fais aussi comme j'aime le faire, tant qu'à faire.

          • [^] # Re: Outil intéressant mais...

            Posté par  . Évalué à 7 (+6/-0).

            Salutations,

            Je n'ai pas encore jeté un œil aux sources, mais je m'incruste sur le sujet des fonctions.
            Pour ma part, je ne suis pas adepte non plus de fonctions à tout va… Cependant, je crois comprendre que la remarque initiale était qu'il y avait des blocs factorisables ; et là je trouve que c'est pertinent pour l'évolution et la maintenabilité future. Cela n'oblige nullement à « sans arrêt faire des sauts d'un bout à l'autre du script » si on respecte certaines règles (une fonction bien nommée et avec une signature bien pensée, on n'a pas à y retourner constamment voir pas du tout…)
            Pour les fonctions qui ne sont utilisées qu'une seule fois, oui ça peut ne pas être utile de faire pareil …sauf si justement ces fonctions permettent de réduire la complexité (au sens de prise en main par d'autres) de la routine principale, ou si un aspect fonctionnel/métier complexe pouvant évoluer et/ou être réutilisé par la suite (on est alors dans le cas d'évolutions/améliorations anticipées.)

            Pour le niveau d'indentation, cinq ne me semble pas trop mais c'est déjà limite. J'ai remarqué qu'à partir du troisième ou quatrième niveau on perd beaucoup de gens (ou plutôt il leur faut plus d'effort pour arriver à suivre.)
            Quand on partage du code, il faut faire quelques compromis pour favoriser la contribution d'un assez grand nombre (sans bien sûr chercher à contenter tout le monde)

            • [^] # Re: Outil intéressant mais...

              Posté par  . Évalué à 7 (+6/-1). Dernière modification le 04/08/21 à 12:11.

              Je me permet moi aussi de me taper l'incruste :)

              Les fonctions permettent de gérer le niveau d'abstraction et de cloisonner un programme.

              Si je prend l'un des plus haut niveau de profondeur du script :

              // search for the pid with this inode in /proc/<pid>/fd  (need to be root)
              $pid = "";
              $process_name = "";
              $process_cmdline = "";
              if ($h1 = @opendir("/proc")) {
                  $found = false;
                  while (false !== ($file1 = readdir($h1))) {
                      if (! $found) {
                          if (is_numeric($file1)) {
                              if ($h2 = @opendir("/proc/$file1/fd")) {
                                  while (false !== ($file2 = readdir($h2))) {
                                      if (! $found) {
                                          if (is_link("/proc/$file1/fd/$file2")) {
                                              $file3 = readlink("/proc/$file1/fd/$file2");
                                              if (strpos($file3, "socket:[$inode]") !== false) {
                                                  display("[..] Found inode for port $port in $file3 in /proc/$file1/fd");
                                                  $status = file_get_contents("/proc/$file1/status");
                                                  $status_ppid = "";
                                                  $status_pid = "";
                                                  $status_lines = explode("\n", $status);
                                                  foreach ($status_lines as $line) {
                                                      if (substr($line, 0, 4) == "Pid:") {
                                                          $status_pid = trim(substr($line, 4));
                                                      }
                                                      if (substr($line, 0, 5) == "PPid:") {
                                                          $status_ppid = trim(substr($line, 5));
                                                      }
                                                      if (substr($line, 0, 5) == "Name:") {
                                                          $process_name = trim(substr($line, 5));
                                                      }
                                                  }
                                                  if ($status_ppid == 1) {
                                                      $pid = $status_pid;
                                                  } else {
                                                      $pid = $status_ppid;
                                                  }
                                                  $process_cmdline = readlink("/proc/$file1/exe");
                                                  $found = true;
                                              }
                                          }
                                      }
                                  }
                                  closedir($h2);
                              }
                          }
                      }
                  }
                  closedir($h1);
              }
              if ($pid == "") {
                  display("[!!] Fatal error : unable to find the main apache process pid in /proc for inode $inode");
                  exit();
              } else {
                  display("[OK] Apache main process '$process_name' found : $process_cmdline (pid $pid)");
              }

              On a un savant mélange de manipulation de chaîne de caractères, de parcourt d'une arborescence et d'affichage. On a aussi des variables qui se balades à tous les niveaux.

              Des fonctions permettent de s'abstraire de tambouille peut utile au fonctionnement globale (savoir comment on gère le format d'un fichier est indépendant de comment on va chercher ce fichier) (bon on est sur un cas pathologique où les conditions sont dans des if alors qu'ils pourraient être dans les while).

              • [^] # Re: Outil intéressant mais...

                Posté par  . Évalué à 2 (+1/-0).

                continue et break seraient de précieux alliés dans ce code.

                Et en python pour ce cas du found il existe une construction que je trouve très élégante le for/while...else

              • [^] # Re: Outil intéressant mais...

                Posté par  (site Web personnel) . Évalué à -1 (+3/-5).

                J'ai l'impression qu'on est en train d'enculer des mouches là.
                Prêcher les "bonnes" pratiques sans donner d'exemples clairs n'est déjà pas très utile.
                Et puisque visiblement la lisibilité d'un code n'est pas quelque chose d'universel, on ne tombera pas tous d'accord sur tout. Par exemple dire que "on est sur un cas pathologique où les conditions sont dans des if alors qu'ils pourraient être dans les while", je trouve personnellement :

                while (false !== ($file1 = readdir($h1)) || ! $found) {
                    ...
                }

                moins clair que :

                while (false !== ($file1 = readdir($h1))) {
                    if (! $found) {
                        ...
                    }
                }

                J'aurais choisi la première option si les performances étaient un point critique, ce qui n'est pas du tout le cas ici. Que le script se termine 0.000003 secondes plus tôt n'a aucun intérêt.
                On pourrait certes utiliser un break 2; (vu que deux while sont imbriqués), mais j'assume le choix de ne jamais utiliser ce truc.

                De toute façon, c'est un bloc qui cherche le pid d'Apache et l'affiche (ou dit qu'il ne le trouve pas). C'est clair et tellement simple qu'un gars qui apprend PHP depuis une semaine serait parfaitement capable de le comprendre au premier coup d'oeil. Et comme ça fait le job, ce n'est pas là-dessus que je vais gaspiller du temps.

                J'aurais largement préféré qu'on me fasse des retours sur les problèmes rencontrés sur des architectures que je n'ai pas pu tester ou sur les paramètres particuliers d'Apache à surveiller, plutôt qu'entamer un débat sur "Faut-il coder avec moins de 3 indentations ?"

                PS : Fut un temps où je développais sur AS/400 en GAP3, une ligne de code = une carte perforée, donc pas d'indentation possible, ce qui n'empêchait pas d'imbriquer de nombreuses portions de code. Niveau lisibilité, c'était autre chose… certains devs imprimaient les codes et dessinaient des accolades pour trouver les portions de code concernées.

                • [^] # Re: Outil intéressant mais...

                  Posté par  . Évalué à 6 (+5/-1).

                  Prêcher les "bonnes" pratiques sans donner d'exemples clairs n'est déjà pas très utile.

                  C'est pour ça que j'ai voulu prendre en un exemple, je voulais installer PHP et montrer ce que ça pourrait être, mais je n'ai pas eu le temps ce matin et je vais finalement m'abstenir.

                  J'aurais largement préféré qu'on me fasse des retours sur les problèmes rencontrés sur des architectures que je n'ai pas pu tester ou sur les paramètres particuliers d'Apache à surveiller, plutôt qu'entamer un débat sur "Faut-il coder avec moins de 3 indentations ?"

                  Très bien.

                  Niveau lisibilité, c'était autre chose… certains devs imprimaient les codes et dessinaient des accolades pour trouver les portions de code concernées.

                  La lisibilité c'est pas une question de testostérone. Tu n'es pas ton code et les remarques que j'ai pu faire sur ton code ne préjugé rien sur toi. Elles ne t'intéressent pas, très bien.

                • [^] # Re: Outil intéressant mais...

                  Posté par  . Évalué à 4 (+2/-0). Dernière modification le 04/08/21 à 15:34.

                  Voici ton code avec une erreur très peu visible introduite (il me semble qu'il s'exécute, même, mais pas sûr), tel qu'écrit à ta façon.

                  Combien de temps vas-tu mettre à la trouver sans comparer au code initial ?

                  // search for the pid with this inode in /proc/<pid>/fd  (need to be root)
                  $pid = "";
                  $process_name = "";
                  $process_cmdline = "";
                  if ($h1 = @opendir("/proc")) {
                      $found = false;
                      while (false !== ($file1 = readdir($h1))) {
                          if (! $found) {
                              if (is_numeric($file1)) {
                                  if ($h2 = @opendir("/proc/$file1/fd")) {
                                      while (false !== ($file2 = readdir($h2))) {
                                          if (! $found) {
                                              if (is_link("/proc/$file1/fd/$file2")) {
                                                  $file3 = readlink("/proc/$file1/fd/$file2");
                                                  if (strpos($file3, "socket:[$inode]") !== false) {
                                                      display("[..] Found inode for port $port in $file3 in /proc/$file1/fd");
                                                      $status = file_get_contents("/proc/$file1/status");
                                                      $status_ppid = "";
                                                      $status_pid = "";
                                                      $status_lines = explode("\n", $status);
                                                      foreach ($status_lines as $line) {
                                                          if (substr($line, 0, 4) == "Pid:") {
                                                              $status_pid = trim(substr($line, 4));
                                                          }
                                                          if (substr($line, 0, 5) == "PPid:") {
                                                              $status_ppid = trim(substr($line, 5));
                                                          }
                                                          if (substr($line, 0, 5) == "Name:") {
                                                              $process_name = trim(substr($line, 5));
                                                          }
                                                      }
                                                      if ($status_ppid == 1) {
                                                          $pid = $status_pid;
                                                      } else {
                                                          $pid = $status_ppid;
                                                      }
                                                      $process_cmdline = readlink("/proc/$file1/exe");
                                                      $found = true;
                                                  }
                                              }
                                          }
                                      }
                                  }
                                  closedir($h2);
                              }
                          }
                      }
                      closedir($h1);
                  }
                  if ($pid == "") {
                      display("[!!] Fatal error : unable to find the main apache process pid in /proc for inode $inode");
                      exit();
                  } else {
                      display("[OK] Apache main process '$process_name' found : $process_cmdline (pid $pid)");
                  }
                  • [^] # Re: Outil intéressant mais...

                    Posté par  (site Web personnel) . Évalué à -1 (+2/-4).

                    Le seul changement qui frappe yeux est le closedir($h2); mal placé.
                    Mais comment arriver à ce type d'erreur ? Car le code ne s'écrit pas linéairement.
                    En général tu commences par :

                    if ($h2 = @opendir("/proc/$file1/fd")) {
                    }

                    puis tu remplis le niveau suivant :

                    if ($h2 = @opendir("/proc/$file1/fd")) {
                        while (false !== ($file2 = readdir($h2))) {
                        }
                        closedir($h2);
                    }

                    et ensuite tu remplis le while.
                    Bref, c'est difficile d'avoir le closedir mal placé, sauf à coder avec un taux d'alcoolémie élevé.

                    • [^] # Re: Outil intéressant mais...

                      Posté par  . Évalué à 7 (+5/-0). Dernière modification le 04/08/21 à 16:24.

                      C'est ça. Ça t'a sauté aux yeux ?

                      Mais comment arriver à ce type d'erreur ?

                      Plein de façons. Dont enlever un if et la mauvaise accolade fermante, refactoring, copier/coller… ou un raccourci clavier dans un IDE pour inverser deux lignes, dont tu n'es même pas au courant, et que tu tape par erreur :)

                      C'est avec des fonctions que c'est beaucoup plus dur (cette erreur, et bien d'autres).

                      et ensuite tu remplis le while.

                      Donc tu fais "comme si" tu avais une fonction, mais tu ne te donne pas la peine de le faire. C'est encore plus dommage, et ça donne un côté très peu lisible/maintenable à ton code, alors qu'il pourrait l'être.

                    • [^] # Re: Outil intéressant mais...

                      Posté par  . Évalué à 10 (+10/-0).

                      J’ai l’impression que tu t’en sors parce que tu as des habitudes et des règles plein la tête sur les trucs bas niveau, un peu comme un compilo qui gère le préambule et la fin des fonctions en quelques instruction assembleur qui sont sytématiquement identiques, sauvegarder la pile, restaurer la valeur des registres …

                      Le truc c’est que la plupart des devs qui lisent ton code auront probablement d’autres règles en tête et surtout pas exactement les mêmes que les tiennes, viendront d’autres contextes. Du coup des détails comme ça qui restent des détails relativement bas niveau, c’est souvent … du bruit qui écarte de la fonctionnalité que remplit le code.

                      Tu le dis toi même, tu attends des retours sur le métier du code et tout le monde te répond que ton code est difficile à lire pour quelqu’un qui ne l’a pas écrit … En général, si on doit lire du code, on aime bien que ce soit structuré du haut niveau qui aide à comprendre ce que c’est supposé faire, et avoir un peu tout ça dans la tête, au bas niveau qui est moins important. Tout ce qui aide à ignorer le second et mettre en valeur le premier, comme structurer le code à l’aide de fonctions, est le bienvenu.

                      Là tu as une fonctionnalité, détecter le PID d’apache, qui prend plus d’une page de code et qu’on doit faire défiler avant de connaître la suite du programme, avec un gros pavé de code avec pleins de détails inintéressants qui distrait et qu’on doit faire l’effort d’ignorer, une fois qu’on a vaguement compris de quoi il s’agissait …

                      Ça explique à mon avis pourquoi la discussion part dans une direction qui te plaît pas. L’autre c’est que la fonction du script est très technique et que c’est toujours compliqué d’avoir des retours pointus du premier coup sans avoir une communauté d’intéressé à cibler.

                • [^] # Re: Outil intéressant mais...

                  Posté par  . Évalué à 5 (+3/-0).

                  Je crois que tu n’as pas croisé un débutant depuis … bien longtemps. Tu as déjà enseigné ou donné des formations ?

                • [^] # Re: Outil intéressant mais...

                  Posté par  (site Web personnel) . Évalué à 8 (+7/-1).

                  je trouve personnellement :

                  while (false !== ($file1 = readdir($h1)) || ! $found) {
                      ...
                  }

                  moins clair que :

                  while (false !== ($file1 = readdir($h1))) {
                      if (! $found) {
                          ...
                      }
                  }

                  Je vais chipoter, mais déjà, je trouve qu’écrire <valeur> != <expression> c’est illisible.

                  De toute façon, c'est un bloc qui cherche le pid d'Apache et l'affiche (ou dit qu'il ne le trouve pas).

                  Je ne sais absolument pas coder en PHP (et j’en suis très content), mais :

                  system("pgrep -f 'apache|httpd'") // Une liste de PID (un par ligne)

                  C’est ce que fait lynis par exemple.

                  J'aurais largement préféré qu'on me fasse des retours sur les problèmes rencontrés sur des architectures que je n'ai pas pu tester ou sur les paramètres particuliers d'Apache à surveiller, plutôt qu'entamer un débat sur "Faut-il coder avec moins de 3 indentations ?"

                  À la base c’est pas un débat, c’est des recommandations de quelqu’un qui fait du PHP au quotidien.

                  Tu les acceptes ou pas, mais si tu espères avoir des contributions d’autres gens tu peux pas dire « moi je fais comme ça, parce que j’ai appris les PHP y a 10 ans et que j’ai toujours fait comme ça » quand on te dit que c’est pas comme ça qu’on fait du PHP moderne.

                • [^] # Re: Outil intéressant mais...

                  Posté par  (site Web personnel) . Évalué à 4 (+3/-0).

                  Il y a aussi la possibilité d'interrompre les boucles et d'éviter certains imbrications de "if"

                      while (false !== ($file1 = readdir($h1))) {
                          if ($found) {
                              continue;
                          }
                          if (!is_numeric($file1)) {
                              continue;
                          }
                          [...]
                  
                      }

                  Sinon quelques basiques pour la mise en forme :

                  https://www.php-fig.org/psr/psr-1/
                  https://www.php-fig.org/psr/psr-12/

                  On peut utiliser un outil comme PHP-CS-Fixer pour harmoniser et appliquer automatiquement ces règles (et de nombreuses autres)

                • [^] # Re: Outil intéressant mais...

                  Posté par  (site Web personnel) . Évalué à 6 (+5/-0).

                  J'aurais largement préféré qu'on me fasse des retours sur les problèmes rencontrés sur des architectures que je n'ai pas pu tester ou sur les paramètres particuliers d'Apache à surveiller, plutôt qu'entamer un débat sur "Faut-il coder avec moins de 3 indentations ?"

                  Pour le coup, je trouve que ce thread va un peu loin, je suis sincèrement désolé de l'avoir lancé.

                  Je suis d'accord avec toi, ces considérations sur le style, ce n'est finalement pas le sujet, fonctionnellement ton outil est intéressant, je n'en ai personnellement pas l'usage (je n'utilise plus apache depuis longtemps). J'espère que d'autres moules vont au moins l'essayer !

          • [^] # Re: Outil intéressant mais...

            Posté par  (site Web personnel) . Évalué à 10 (+11/-1).

            Heu… 5 ou 6 indentations, mouais, bof, c'est quand même super banal.

            Personnellement, au delà de 2 ou 3 blocks imbriqués je refuse une MR.

  • # apachectl -t ?

    Posté par  (site Web personnel) . Évalué à 2 (+1/-0).

    Je vois que l'existence d'apachectl est vérifiée et que des exécutions sont faites avec -V et -M.
    Je ne sais pas si -t est omis parce que c'est hors périmètre (par exemple, si l'usage est de l'exécuter préalablement au script PHP).
    Le paramètre -t permet de vérifier la syntaxe des configurations apache, la disponibilité des modules utilisés, etc. C'est utile pour une validation minimale des modifications de configuration avant de redémarrer Apache.

    • [^] # Re: apachectl -t ?

      Posté par  (site Web personnel) . Évalué à 2 (+1/-0).

      Merci, c'est en effet prévu pour la v1.4 qui est sur le métier, car il n'est pas impossible qu'on parte d'un apache avec des fichiers de config qui merdouillent.

      Le script évolue en fonction des choses que je note en lisant la doc complète d'Apache (je vais en avoir pour un moment, mais ça avance et j'apprends plein de trucs), et les remarques qu'on me fait à droite à gauche.

Envoyer un commentaire

Suivre le flux des commentaires

Note : les commentaires appartiennent à ceux qui les ont postés. Nous n’en sommes pas responsables.