Bien coder en C : avis sur bout(s) de code

Bonjour,

Je mettrai ici de temps en temps des bouts de code issus du programme que je suis en train de faire dans le but d'avoir des retours critiques sur la "propreté" de mon code.

Est-ce bien codé ? Tant sur la forme que sur le fond. Le but est de prendre les bons réflexes et d'éviter les mauvaises habitudes.

Je commence par cette fonction :

Code:

---

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38

int get_handlepath(char path[]) { int i_path; int i_handle; int i_found; int i_subpath;   for (i_path = 0; i_path < compter_ch(path); i_path++) { if (path[i_path] == '\\') { i_found = i_path; i_path = 0; if ((t_registre.handle = (char*)malloc((i_found+1)*sizeof(char))) && (t_registre.path = (char*)malloc((compter_ch(path)-i_found)*sizeof(char))) != NULL) { for (i_handle = 0; i_handle < i_found; i_handle++) { t_registre.handle[i_handle] = path[i_path]; i_path++; }   t_registre.handle[i_found] = '\0'; i_subpath = 0; for (i_found = i_found+1; i_found < compter_ch(path); i_found++) { t_registre.path[i_subpath] = path[i_found]; i_subpath++; }   t_registre.path[i_subpath] = '\0'; return (0); break; } } } return (1); }

---

Merci par avance.

;)

La premiere des chose et la plus flagrante : absence de commentaire.
Il faudrait mettre un minimum de commentaire, sur ce que fais la fonction globalement, ce qu'elle renvoie, a quoi correspond ce renvoie ...

Bref, expliquer comment tu va proceder pour que quelqu'un puisse comprendre dans les grandes lignes ce que tu va faire. Evite surtout de tomber dans l'exces inverse, c'est a dire de trop commenté et/ou de paraphraser :

Code:

---

1 2 3

/* Alloue dynamiquement un tableau de trois int */ Tableau = (int*) malloc (3*sizeof(int));

---

Ce genre de commentaire est néfaste, stupide et inutile.

Citation:

---

La premiere des chose et la plus flagrante : absence de commentaire.

---

Certaines "normes" déconseillent les commentaires au sein des fonctions : ils doivent être juste au-dessus et signalés d'une certaine façon.

J'aurais tendance à aller dans ce sens, puisque je trouve que les commentaires nuisent à l'aspect visuel du code.

Par contre, tout à fait d'accord pour faire une entête qui stipule ce que fait la fonctions, ses arguments, ce qu'elle renvoie etc.

Puisque tu demandes des critiques :

- i_path semble gérer par le for(ipath =0 ;... ; ipath++). Or on trouve dans la boucle des instructions ipath = 0 et ipath++ ce qui ne facilite pas la lecture du code.

- Tu utilises une variable globale, t_registre, ce qui est toujours à éviter

- Dans ce code, le break ne sert à rien :

Code:

---

1 2	return (0); break;

---

- L'écriture du if est incohérente

Code:

---

1 2	if ((t_registre.handle = ...) // pourquoi ici pas de comparaison && (t_registre.path =...) != NULL) // et ici on en fait une

---

Code:

---

1 2	if ((t_registre.handle = ...)!= NULL && (t_registre.path =...) != NULL)

---

Si je comprend ton code, il s'agit d'analyser une chaine pour chercher \. Ce qui le précède est placé dans une chaine pointée par t_registre.handle et ce qui le suit dans une autre pointée par t_registre.path.
Ton code me semble bien compliqué si c'est le cas. pourquoi n'utilises-tu pas les fonctions de la bibliothèque ? Je crois que ceci fait la même chose :

Code:

---

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20

int get_handlepath( struct x *t_registre, char path[]) { size_t len1, len2; char * p = strchr(path,'\\'); if(p != NULL) { len1 = p-path ; p++; len2 = strlen(p); if ( (t_registre->handle = malloc(len1+1)) != NULL && (t_registre->path = malloc(len2+1)) != NULL) { strncpy(t_registre->handle,path,len1); t_registre->handle[len1] = '\0'; strcpy(t_registre->path,p); } else free(t_registre->handle); } return p == NULL; }

---

Citation:

---

Envoyé par Merillym

Est-ce bien codé ? Tant sur la forme que sur le fond. Le but est de prendre les bons réflexes et d'éviter les mauvaises habitudes.

---

Tout d'abord, ça manque de commentaire. Au minimum un commentaire décrivant le rôle de cette fonction, ses paramètres et ses valeurs de retour.
Là je ne sais même pas ce que fait cette fonction sans rentrer dedans en détail.

En dehors de ce point, voici quelques autres détails (je pars du principe que tout le code autour - appelant, inclusion des en-tête, etc. - est correct) :

• Le paramètre path n'étant pas modifié par la fonction il devrait être constant.
• Si le rôle de la ligne if (path[i_path] == '\\') est bien d'identifier les séparateur dans un path, je testerais également avec le caractère '/'.
• D'où vient t_registre ? Est-ce une variable globale ? Dans ce cas et sans plus de précision, c'est probablement une mauvaise idée. D'une manière générale, les variables globales sont à éviter.
• Les deux styles de test (l'un explicitant la valeur NULL et l'autre pas) pour le retour du malloc ne sont pas du meilleur effet. Sans rentrer dans la discussion de quelle pratique est la meilleure, il est largement préférable de se tenir à un seul style dans un programme.
• Au passage en cas d'erreur sur le second malloc, tu as une jolie fuite mémoire. Je suppose qu'en cas de réussite, c'est l'appelant qui finit par tout libérer, dans le cas contraire, tu as une autre fuite mémoire.
• Le cast du retour de malloc() n'est pas utile en C.
• Plutôt que d'utiliser sizeof(char) (qui par définition vaut 1), j'utiliserais sizeof(*t_registre.handle) et sizeof(*t_registre.path) dans les malloc.
• Pourquoi avoir sortie l'initialisation et l'incrément de certain compteur du for ?
• J'ignore ce que fait en détail la fonction compter_ch, mais si comme son nom le laisse penser il s'agit d'un clone de strlen(), il serait plus judicieux (puisque path n'est pas modifié) de l'appeler une seule fois et de stocker le résultat plutôt que de l'appeler à chaque tour de boucle.

Au passage, pourquoi ne pas utiliser les fonctions de la bibliothèque standard ? Sauf contrainte particulière c'est une très mauvaise idée.

Bonsoir,

Merci à tous pour vos avis. Je vais les lire soigneusement et les prendre en compte.

Citation:

---

Au passage, pourquoi ne pas utiliser les fonctions de la bibliothèque standard ? Sauf contrainte particulière c'est une très mauvaise idée.

---

Je m'étais "amusé" à recoder certaines fonctions pour m'entraîner et j'ai pris la fâcheuse habitude d'utiliser mes fonctions... je vais aussi rectifier ça.

Merci :ccool:

Citation:

---

- Tu utilises une variable globale, t_registre, ce qui est toujours à éviter

---

Oki, je pense qu'elle pourrait se justifier ici, la voici :

Code:

---

1 2 3 4 5 6 7 8 9 10 11

struct s_registre { char *handle; char *path; char *pathKeyEnd; char *option; char *value; char *type; char *data; char *fichier; };

---

Mais en y réfléchissant, rien ne justifie vraiment d'en faire une variable globale.

Citation:

---

- L'écriture du if est incohérente

---

Une erreur d'inattention, je suis tout à fait d'accord, merci d'avoir pointé l'erreur.

Citation:

---

Si je comprend ton code, il s'agit d'analyser une chaine pour chercher \. Ce qui le précède est placé dans une chaine pointée par t_registre.handle et ce qui le suit dans une autre pointée par t_registre.path.
Ton code me semble bien compliqué si c'est le cas. pourquoi n'utilises-tu pas les fonctions de la bibliothèque ? Je crois que ceci fait la même chose :

---

Tu as tout compris, je vais arrêter d'utiliser mes fonctions et apprendre à utiliser celles issues de la bibliothèque standard.

Citation:

---

Si le rôle de la ligne if (path[i_path] == '\\') est bien d'identifier les séparateur dans un path, je testerais également avec le caractère '/'.

---

Ici non, puisqu'il s'agit de parser une clé de registre windows.

Citation:

---

J'ignore ce que fait en détail la fonction compter_ch, mais si comme son nom le laisse penser il s'agit d'un clone de strlen(), il serait plus judicieux (puisque path n'est pas modifié) de l'appeler une seule fois et de stocker le résultat plutôt que de l'appeler à chaque tour de boucle.

---

En effet.

Merci pour tous ces commentaires détaillés.

;)

J'ai pris le temps de bien tout reprendre et effectivement ton code fait la même chose Diogene :ccool:

Je posterai un autre bout de code d'ici quelques jours ( une autre fonction ) et vous pourrez me dire si c'est mieux.

Merci.

Pour une gestion correcte des erreurs d'allocation de la mémoire, dans le code que j'ai posté, il faut modifier un peu :

Code:

---

1 2 3 4 5 6 7 8

if ( (t_registre->handle = malloc(len1+1)) != NULL && (t_registre->path = malloc(len2+1)) != NULL) { strncpy(t_registre->handle,path,len1); t_registre->handle[len1] = '\0'; strcpy(t_registre->path,p); } else free(t_registre->handle);

---

PS : je modifie en conséquence le code du post précédent.

Personnellement, l'appel à malloc() dans un if me gène un peu...
Je préfère dissocier les affectations des tests conditionnels.
Je pense qu'il est préférable de faire le malloc() en premier et ensuite tester la valeur du pointeur. Le code fera 2 lignes de plus mais plus clair a mon sens.
De plus, un compilo bien réglé doit te générer un warning avec ce code (affectation d'un valeur lors d'un d'un test conditionnel).

Vincent Rogier :

Citation:

---

Je préfère dissocier les affectations des tests conditionnels.
Je pense qu'il est préférable de faire le malloc() en premier et ensuite tester la valeur du pointeur. Le code fera 2 lignes de plus mais plus clair a mon sens.

---

Oui, je suis assez d'accord sur la question de la lisibilité (surtout si la ligne est longue et pleine de parenthèses).
Mais dans le cas présent, je ne suis pas certain que le code soit plus clair ici:

Code:

---

1 2 3 4 5 6 7 8 9 10 11

t_registre->handle = malloc(len1+1) if(t_registre->handle != NULL) { t_registre->path = malloc(len2+1); if(t_registre->path != NULL) { ... } else free(t_registre->handle); }

---

que là :

Code:

---

1 2 3 4 5 6 7

if ( (t_registre->handle = malloc(len1+1)) != NULL && (t_registre->path = malloc(len2+1)) != NULL) { ... } else free(t_registre->handle);

---

Citation:

---

De plus, un compilo bien réglé doit te générer un warning avec ce code (affectation d'un valeur lors d'un d'un test conditionnel).

---

Il ne devrait pas. Dans ce cas, il n'y a pas de confusion possible du programmeur entre = et == dans ce contexte.
Par contre, ce doit être le cas pour

Code:

---

1 2 3

if ( (t_registre->handle = malloc(len1+1)) && (t_registre->path = malloc(len2+1)))

---

Merci pour ces dernières remarques. Pour ma part, je serais plutôt de l'avis de Diogene concernant le malloc, mais débutant en C, je m'abstiendrai de tout jugement. Je sollicite vos avis, c'est bien pour changer mes mauvaises habitudes et prendre en compte vos remarques, que j'apprécie beaucoup, merci.

Toujours sur ce bout de code, je voudrai être sûr de partir sur une bonne base et j'ai quelques lacunes (encore) sur pointeurs, structures...

J'ai repris ton exemple Diogene et je vais désormais recoder tout mon tool en utilisant les fonctions standard et en intégrant toutes vos remarques. J'ai 10.000 lignes de codes à reprendre, c'est pour ça que j'insiste beaucoup sur le début, afin de partir sur les meilleurs bases possibles. Et pour information, c'est un projet personnel que je fais en loisir, rien à voir avec mes études ;)

Code:

---

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46

#include <windows.h> #include <stdio.h> #include <string.h> #include "headers.h"   int get_handlepath(struct s_registre *ptr_registre, const char path[]) { size_t len1; size_t len2; char *pos;   pos = strchr(path, '\\'); if (pos != NULL) { len1 = pos-path; pos++; len2 = strlen(pos); if ( (ptr_registre->handle = malloc(len1+1)) != NULL && (ptr_registre->path = malloc(len2+1)) != NULL) { strncpy(ptr_registre->handle, path, len1); ptr_registre->handle[len1] = '\0'; strcpy(ptr_registre->path, pos); } } return (*pos); }   int main (int argc, char *argv[]) { struct s_registre t_registre; struct s_registre *ptr_registre; const char *path; size_t n;   ptr_registre = &t_registre; n = strlen(argv[2])*sizeof(char); if ((path = malloc(n+1)) == NULL) return (1);   memcpy(path, argv[2], n); get_handlepath(ptr_registre, path); printf("path = %s\n", ptr_registre->path); printf("handle = %s\n", ptr_registre->handle); return (0); }

---

La structure :

Code:

---

1 2 3 4 5 6 7 8 9 10 11

struct s_registre { char *handle; char *path; char *pathKeyEnd; char *option; char *value; char *type; char *data; char *fichier; };

---

Le compilo me renvoie un warning sur memcpy et effectivement le résultat obtenu n'est pas correct.

J'aimerais bien une petite explication à nouveau sur structure, pointeur de structure et pointeur de structure passé comme argument à une fonction, car je m'embrouille encore. Je sens que ça commence à venir, mais ce n'est pas encore ça.

Dans le bout de code ci-dessus, je suis sûr que ma déclaration de structure, son initialisation, la déclaration et initialisation du pointeur et enfin l'appel de la fonction ne sont pas bons.

-

Citation:

---

Le compilo me renvoie un warning sur memcpy et effectivement le résultat obtenu n'est pas correct.

---

C'est de ta faute : tu as précisé

Code:

---

const char *path;

---

soit "path est un pointeur sur des caractères constants". Donc tu ne peux pas écrire dans la zone pointée par path, ce qui indispose le memcpy.
-

Code:

---

n = strlen(argv[2])*sizeof(char);

---

sizeof(char) vaut toujours 1
-

Code:

---

get_handlepath(ptr_registre, path);

---

ptr_registre est inutile dans ton code, passe directement l'adresse de t_registre. La fin du code devient :

Code:

---

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15

int main (int argc, char *argv[]) { struct s_registre t_registre; char *path; size_t n;   n = strlen(argv[2]); if ((path = malloc(n+1)) == NULL) return (1); memcpy(path, argv[2], n); get_handlepath(&t_registre, path); printf("path = %s\n", t_registre.path); printf("handle = %s\n", t_registre.handle); return (0); }

---

J'ai trouvé aussi une erreur sur le retour de la fonction et sa déclaration : je retourne un pointeur de type char 'pos' et non '*pos', donc j'ai modifié le prototype de la fonction en conséquence.

Citation:

---

soit "path est un pointeur sur des caractères constants". Donc tu ne peux pas écrire dans la zone pointée par path, ce qui indispose le memcpy.

---

Compris.

Citation:

---

sizeof(char) vaut toujours 1

---

SoftEvans m'avait conseillé de toujours laissé sizeof(char) plutôt que 1. Je ne sais pas trop quoi faire à ce sujet, quelle "norme" appliquer etc.

Citation:

---

ptr_registre est inutile dans ton code, passe directement l'adresse de t_registre. La fin du code devient :

---

Inutile pour le moment, mais je vais avoir besoin de passer un pointeur vers la structure s_registre de type t_registre assez souvent, donc je pense qu'il est préférable de passer un pointeur plutôt qu'une structure toute entière non ?

Car si j'enlève ptr_pointeur, je suis désormais obligé de passer la structure en paramètre et non un pointeur vers cette structure, si j'ai bien compris ?

edit:

Là ça marche nickel, est-ce mieux niveau syntaxe du code ?

Code:

---

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24

int main (int argc, char *argv[]) { struct s_registre t_registre; struct s_registre *ptr_registre; char *action; // == argv[1] char *path; // == argv[2] char *option1; // == argv[3] char *option2; // == argv[4] size_t n; char *pos;   ptr_registre = &t_registre; n = strlen(argv[2])+1; if ((path = malloc(n * sizeof(*path))) == NULL) return (1);   memcpy(path, argv[2], n); if ((pos = get_handlepath(ptr_registre, path)) == NULL) return (1);   printf("path = %s\n", ptr_registre->path); printf("handle = %s\n", ptr_registre->handle); return (0); }

---

Citation:

---

SoftEvans m'avait conseillé de toujours laissé sizeof(char) plutôt que 1. Je ne sais pas trop quoi faire à ce sujet, quelle "norme" appliquer etc.

---

C'est sans doute affaire de goût. Personnellement, je ne le met pas : je trouve que ça allonge les lignes et distrait inutilement lors de la lecture du code.

Citation:

---

Inutile pour le moment, mais je vais avoir besoin de passer un pointeur vers la structure s_registre de type t_registre assez souvent, donc je pense qu'il est préférable de passer un pointeur plutôt qu'une structure toute entière non ?

---

Il faut voir pour la suite. En général, je trouve qu'on n'a pas besoin d'ajouter des trucs tant qu'ils ne s'avèrent pas effectivement utiles : ça alourdi le code pendant le développement et polarise le programmeur sur des options choisies parfois prématurément.

Citation:

---

Car si j'enlève ptr_pointeur, je suis désormais obligé de passer la structure en paramètre et non un pointeur vers cette structure, si j'ai bien compris ?

---

Non, tu peux passer l'adresse de la structure plutôt que la valeur d'un pointeur qui contient l'adresse de la structure. Comme ici dans le code que j'ai posté

Code:

---

get_handlepath(&t_registre, path);

---

Citation:

---

Envoyé par diogene

C'est sans doute affaire de goût. Personnellement, je ne le met pas : je trouve que ça allonge les lignes et distrait inutilement lors de la lecture du code.
[/code]

---

En fait, mon prof de C m'avait dit que char pouvait avoir differente valeurs (selon le processeur je crois) et donc avait qu'il fallait toujours laisser le sizeof(char) ...

Recemment, j'ai apris que la norme etait sizeof(char) == 1, ce que je ne savais pas. Apres, c'est effectivement une affaire de gout : ceux qui ont vu tellement de fois sizeof(char) en ont marre et raccourcice l'affaire.

La seule norme en langage est donné par la norme. Apres, il y a plus ou moins des style de codage en vogue, mais tout le reste est une affaire de gout.

Bonsoir,

Je vous mets une autre fonction, dont le but est similaire, c'est-à-dire qu'elle doit parser une chaîne.

J'ai essayé de prendre en compte au maximum toutes vos remarques et j'aimerais vos avis, savoir s'il y a du progrès ou pas, quels sont les défauts, etc. Mon objectif est de finir par "coder proprement" le plus naturellement possible. Donc soyez fermes :aie:

De même je suis intéressé pour avoir vos avis concernant l'optimisation en terme de temps d'exécution du programme : ce qu'il faut faire, ce qu'il faut éviter de faire.

Je pense mettre des bouts de code, ou quelques fonctions assez régulièrement, toujours dans la même optique. Si jamais cela vous dérange, n'hésitez pas à me le dire :)

Voilà une autre fonction ( la structure est la même que dans l'autre exemple ) :

Code:

---

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17

char *get_pathKeyEnd(struct s_registre *ptr_registre) { char *pos; size_t n;   pos = strstr(ptr_registre->path, "|*|"); if (pos != NULL) { n = strlen(ptr_registre->path)+1-(pos-(ptr_registre->path))-4; if ((ptr_registre->pathKeyEnd = malloc(n * sizeof(ptr_registre->pathKeyEnd))) != NULL) { strcpy(ptr_registre->pathKeyEnd, pos+4); ptr_registre->path[pos-(ptr_registre->path)-1] = '\0'; } } return (pos); }

---

Concernant le fait d'utiliser un pointeur de structure ou la structure elle-même, je ne sais pas trop... en terme de vitesse d'exécution, vous préférez quoi ?

Merci :)

C'est pas mal.
- Mais, supprime les parenthèses inutiles qui aloudissent la lecture :

Code:

---

1 2 3

n = strlen(ptr_registre->path)+1-(pos-ptr_registre->path)-4; ... ptr_registre->path[(pos-ptr_registre->path)-1] = '\0';

---

Par contre,
D'accord pour laisser +1 et -4 car ils commentent bien le code. -3 aurait été peu clair et on se serait demandé d'où il vient
D'accord pour les parenthèses autour de (pos-ptr_registre->path), qui montrent bien le calcul d'un indice

-

Code:

---

if ((ptr_registre->pathKeyEnd = malloc(n * sizeof(ptr_registre->pathKeyEnd))) != NULL)

---

Le sizeof est faux (l'opérande est du type char*). D'ailleurs, comme une ligne plus loin on fait un strcpy, on se doute que tu veux allouer n char.
On peut se contenter de :

Code:

---

if ((ptr_registre->pathKeyEnd = malloc(n )) != NULL)

---

-

Code:

---

ptr_registre->path[pos-(ptr_registre->path)-1] = '\0';

---

Code assez obscur qui ne montre pas à l'évidence où ce 0 va être placé et qui doit être équivalent à :

Code:

---

*(pos-1) = '\0';

---

- Sur le fonctionnement (là, c'est toi qui connait les réponses) :
Est-il correct de sauter dans la copie le caractère suivant "|*|" ?
Est-il correct de placer le '\0' à la place du caractère qui précède "|*|" et qui donc l'écrase ?
Est-il certain qu'il existe au moins un caractère avant la sous-chaîne "|*|" ?

Citation:

---

Code assez obscur qui ne montre pas à l'évidence où ce 0 va être placé et qui doit être équivalent à :
*(pos-1) = '\0';

---

Je ne suis pas encore bien familier avec les fonctions de la bibliothèque standard puisque jusqu'à présent je n'utilisais que les miennes :mouarf:

Je commence à mieux comprendre.

Citation:

---

Est-il correct de sauter dans la copie le caractère suivant "|*|" ?

---

Oui.

Citation:

---

Est-il correct de placer le '\0' à la place du caractère qui précède "|*|" et qui donc l'écrase ?

---

Oui.

Citation:

---

Est-il certain qu'il existe au moins un caractère avant la sous-chaîne "|*|" ?

---

Oui, sauf si un utilisateur s'amuse à faire n'importe quoi, mais comme le tool est destiné à des gens qui sont censés savoir utiliser un command line tool... Mais sur le fond, tu as raison : il vaut mieux par principe tout contrôler.

Merci beaucoup de ta réponse.

Ca m'aide vraiment beaucoup d'avoir des personnes compétentes pour me corriger au niveau lisibilité et syntaxe : c'est ce qui fait le plus défaut dans un apprentissage en autodidacte.

:ccool:

Voici une autre fonction, qui compare en ignorant la casse : je ne suis pas très satisfait de mes incrémentations de i et de j, surtout d'être obligé de partir à i = -1... si vous avez mieux, je suis preneur.

Code:

---

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19

int compare_ch(char str1[], char str2[]) { int i; int j;   i = -1; j = 0; if ((strlen(str1) == strlen(str2)) && str1 != NULL && str2 != NULL) { while (i++ < strlen(str1)); { if (str1[i] != str2[j] && str1[i] != str2[j]+32 && str1[i] != str2[j]-32) return(1); j++; } return(0); } return(1); }

---

Bonjour,

Voilà une autre fonction :

Code:

---

1 2 3 4 5 6 7 8 9 10 11 12 13

unsigned int check_handle_HKLM(struct s_registre *ptr_registre) { HKEY hKey; if ((compare_ch(ptr_registre->handle, "HKLM")) == 0 || (compare_ch(ptr_registre->handle, "HKEY_LOCAL_MACHINE")) == 0) { if (RegOpenKeyEx(HKEY_LOCAL_MACHINE, ptr_registre->path, 0, KEY_READ, &hKey) == ERROR_SUCCESS) return (hKey); else return (-1); } return (-1); }

---

J'aimerais retourner un handle de clé, mais je ne suis pas sûr que ma fonction marche... le type d'un handle de clé est HKEY, donc je me demande si je mets unsigned int ou HKEY en type de retour. J'attends d'avoir une réponse à cette question avant de tester la fonction ci-dessus ;)

Je vous mets ma fonction compare_ch ( qui ignore la casse ) :

Code:

---

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20

int compare_ch(char str1[], char str2[]) { int i; int j;   i = 0; j = 0; if ((strlen(str1) == strlen(str2)) && str1 != NULL && str2 != NULL) { while (i < strlen(str1)) { if (str1[i] != str2[j] && str1[i] != str2[j]+32 && str1[i] != str2[j]-32) return(1); j++; i++; } return(0); } return(1); }

---

Comme pour les bouts de code précédents, j'attends vos remarques/critiques/conseils.

Merci.

;)

Sur ta fonction check_handle_HKLM()
- Elle ne peut pas retourner un unsigned int : tu retournes -1
- Elle doit retourner HKEY. Mais alors, comment signaler une erreur ? (je ne connais pas les valeurs possibles pour un HKEY valide ni leur type effectif)

La meilleure solution n'est-elle pas de retourner un indicateur de réussite et de passer par la liste des paramètres le HKEY ?

..... check_handle_HKLM(struct s_registre *ptr_registre, HKEY * hKey)

En ce qui concerne la fonction compare_ch,

- Je n'aime pas beaucoup l'utilisation de ces strlen() qui font à chaque fois parcourir la chaine pour trouver le zéro terminal, alors que nous allons être obligé de parcourir les chaînes pour les comparer (donc on pourra détecter à cette occasion le zéro terminal).

- Je n'aime pas non plus ces +- 32 (en plus, ces tests sont faux si les caractères ne sont pas alphabétiques).

- Le type des paramètres pourrait indiquer que les chaîne ne sont pas modifiées par la fonction (qualificatif const)

Compte tenu de mes allergies, je verrais plutôt quelque chose comme :

Code:

---

1 2 3 4 5 6 7 8 9 10 11

int compare_ch(char const *str1, char const *str2) { int res = str1 == NULL || str2 == NULL; while(res == 0 && (*str1 != 0 || * str2 != 0)) { res = tolower(*str1) != tolower(*str2); str1++; str2++; } return res; }

---

Citation:

---

Envoyé par diogene

Sur ta fonction check_handle_HKLM()
- Elle ne peut pas retourner un unsigned int : tu retournes -1
- Elle doit retourner HKEY. Mais alors, comment signaler une erreur ? (je ne connais pas les valeurs possibles pour un HKEY valide ni leur type effectif)

La meilleure solution n'est-elle pas de retourner un indicateur de réussite et de passer par la liste des paramètres le HKEY ?

..... check_handle_HKLM(struct s_registre *ptr_registre, HKEY * hKey)

---

Merci Diogene, je regarde tout cela, idem pour la fonction comparer_ch.

:ccool: