Valgrind me démoralise un peu

Bonjour tout le monde,

Suite à une erreur que je n'arrive pas à traquer, je suis aller voir pour utiliser d'autre méthode de recherche, et je suis tomber sur valgrind.

Le problème, c'est que si je comprend bien, j'ai des erreurs partout dans mon code ... Par exemple il me pointe ce petit bout de code la :

Code:

---

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

void em_string_check (em_string_t * string, unsigned int newSize) { if ( string->size == 0 ) { string->ptr = em_malloc(newSize * sizeof(char)); } else if ( string->size < newSize) { string->ptr = em_realloc(string->ptr, newSize * sizeof(char)); }   string->size = newSize; string->ptr[newSize] = '\0'; }

---

Il m'écrit "Invalid write of size 1" en ce qui concerne la ligne : string->ptr[newSize] = '\0';
Et je ne comprend pas bien pourquoi ? Il faut savoir que ma mes fonctions em_malloc() et em_realloc() s'occupe de flinguer le programme si l'allocation a foiré.

Il m'écrit le même genre d'alerte avec un :

Code:

---

1 2 3 4 5

if ( strcmp(test, "un_mot") == 0 ) { // ... }

---

D'après ce que je comprend, on aime pas que les blocs soient écrit en dur avec valgrind ??

Merci d'avance

Si tes em_malloc() et em_realloc() fonctionne comme malloc() et realloc(), alors effectivement il y a une écriture invalide :

Code:

---

1 2 3 4 5 6

.... string->ptr = em_malloc(newSize * sizeof(char)); .... string->ptr = em_realloc(string->ptr, newSize * sizeof(char)); .... string->ptr[newSize] = '\0';

---

Les indices de string->ptr doivent être entre 0 et newSize -1 inclus.

Pour l'autre diagnostic, on manque d'informations

Ah oui juste !

Effectivement pour ma fonction, je donne une taille dans laquelle je compte la fin de chaine.
Donc je dois faire newsize-1. Un peu distrait...

Je pense même avoir mon problème de base. Merci.

Je vais quand même revoir pour strcmp()...

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

em_string_t * em_parser_getGenericValue (em_vars_t * vars, const char * section, const char * var) { unsigned int i;   if ( section == NULL || var == NULL ) return NULL;   for ( i = 0; i < vars->sectionsCount; i++ ) { if ( strcmp(section, vars->sections[i].name.ptr) == 0 ) { unsigned int j;   for ( j = 0; vars->sections[i].varsCount; j++ ) { if ( strcmp(var, vars->sections[i].vars[j].name.ptr) == 0 ) return &vars->sections[i].vars[j].value; } } }   em_trace(_MSG_ERROR, "The var %s in %s section doesn't exists !", var, section);   return NULL; }

---

voila ce que m'écrit valgring a propos de mon utilisation de strcmp()

==3976== Use of uninitialised value of size 8
==3976== at 0x5427578: __strcmp_ssse3 (strcmp.S:2070)
==3976== by 0x40276C: em_parser_getGenericValue (em_lib3_parser.c:374)
==3976== by 0x4028BB: em_parser_getBoolean (em_lib3_parser.c:405)
==3976== by 0x402D4C: em_configure (em_p_base.c:38)
==3976== by 0x402DF3: emeraude_start (em_p_base.c:62)
==3976== by 0x402EA9: main (em_p_entrypoint.c:10)

Donc en fait j'ai une structure et dedans je vais chercher dans la section un tel, la variable un tel. Voila, là je comprend pas ce que je fais de mal ? Certainement un truc qui ne me saute pas aux yeux... lol

Il y a une erreur dans le second for :

Code:

---

for ( j = 0; vars->sections[i].varsCount; j++ )

---

Ici la condition de continuation est toujours vrai, tu as donc une boucle infinie et tu accédes en dehors de tes éléments. A priori le for devrait plutôt être :

Code:

---

for ( j = 0; j < vars->sections[i].varsCount; j++ )

---

mmm ... Je me sens un peu biesse là.

Mais a force de lire mon code, je vois plus rien lol.

Merci beaucoup.

Bon ben j'ai un autre exemple de code ou valgrind me point l'utilisation de strcmp()

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

bool em_parser_getBoolean (em_vars_t * vars, const char * section, const char * var) { bool value = FALSE; em_string_t * str = em_parser_getGenericValue(vars, section, var);   if ( str != NULL ) { if ( strcmp(str->ptr, "1") == 0 ) value = TRUE; else if ( strcmp(str->ptr, "0") == 0 ) value = FALSE; else if ( strcmp(str->ptr, "true") == 0 ) value = TRUE; else if ( strcmp(str->ptr, "True") == 0 ) value = TRUE; else if ( strcmp(str->ptr, "TRUE") == 0 ) value = TRUE; else if ( strcmp(str->ptr, "false") == 0 ) value = FALSE; else if ( strcmp(str->ptr, "False") == 0 ) value = FALSE; else if ( strcmp(str->ptr, "FALSE") == 0 ) value = FALSE; else if ( strcmp(str->ptr, "on") == 0 ) value = TRUE; else if ( strcmp(str->ptr, "On") == 0 ) value = TRUE; else if ( strcmp(str->ptr, "ON") == 0 ) value = TRUE; else if ( strcmp(str->ptr, "off") == 0 ) value = FALSE; else if ( strcmp(str->ptr, "Off") == 0 ) value = FALSE; else if ( strcmp(str->ptr, "OFF") == 0 ) value = FALSE; }   return value; }

---

valgrind :

==20196== Invalid read of size 8
==20196== at 0x562535A: __strcmp_ssse3 (strcmp.S:100)
==20196== by 0x4042A3: em_parser_getGenericValue (em_lib3_parser.c:373)
==20196== by 0x4043FA: em_parser_getBoolean (em_lib3_parser.c:404)
==20196== by 0x408A0B: em_configure (em_p_base.c:39)
==20196== by 0x408B72: emeraude_start (em_p_base.c:82)
==20196== by 0x409065: main (em_p_entrypoint.c:10)
==20196== Address 0x8539690 is 0 bytes inside a block of size 5 alloc'd
==20196== at 0x4C241A7: malloc (vg_replace_malloc.c:195)
==20196== by 0x402A8B: em_malloc (em_lib0_memory.c:264)
==20196== by 0x403241: em_string_check (em_lib1_string.c:27)
==20196== by 0x4033F8: em_string_setnchar (em_lib1_string.c:103)
==20196== by 0x403C13: em_parser_parseSectionName (em_lib3_parser.c:106)
==20196== by 0x404173: em_parser_parseFile (em_lib3_parser.c:332)
==20196== by 0x40898E: em_configure (em_p_base.c:28)
==20196== by 0x408B72: emeraude_start (em_p_base.c:82)
==20196== by 0x409065: main (em_p_entrypoint.c:10)
==20196==
==20196==


Venant de php, je ne suis pas très fière de ce code, mais bon, je ne sais pas comment faire ça plus propre.

Pour la question avec strcmp(), probablement que le problème vient de str->ptr (peut-il être non initialisé au lieu d'être initialisé par une chaine vide ?).

Pour le code lui-même, ceci est équivalent à

Code:

---

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

bool value = FALSE; em_string_t * str = em_parser_getGenericValue(vars, section, var); if ( str != NULL ) { if ( strcmp(str->ptr, "1") == 0 ) value = TRUE; else if ( strcmp(str->ptr, "true") == 0 ) value = TRUE; else if ( strcmp(str->ptr, "True") == 0 ) value = TRUE; else if ( strcmp(str->ptr, "TRUE") == 0 ) value = TRUE; else if ( strcmp(str->ptr, "on") == 0 ) value = TRUE; else if ( strcmp(str->ptr, "On") == 0 ) value = TRUE; else if ( strcmp(str->ptr, "ON") == 0 ) value = TRUE; } return value; }

---

Tu peux réduire le nombre de comparaisons en mettant tout en majucules si tu as le droit de modifier la chaine renvoyée par em_parser_getGenericValue() :

Code:

---

1 2 3 4 5 6 7 8 9 10 11

#include <ctype.h> .... char *p; bool value = FALSE; em_string_t * str = em_parser_getGenericValue(vars, section, var); if ( str != NULL ) { for(p = str->ptr; *p != '\0'; p++) *p = toupper(*p); value = strcmp(str->ptr, "1") == 0 || strcmp(str->ptr, "TRUE")==0 || strcmp(str->ptr, "ON") == 0 ; } return value;

---

Ah ouai sympa l'idée pour la réduction, le problème c'est que ce sont des clés avec des valeurs. Et a tout moment je dois pouvoir les traiter de manière différentes.
Je pense que si je crée une copie temporaire de la chaîne pour faire la comparaison, ce sera un peu beaucoup, non?

Sinon pour en revenir avec valgrind. J'ai une structure qui me permet de conserver une chaîne avec son nombre de caractère.

Code:

---

1 2 3 4 5 6

typedef struct { unsigned int size; char * ptr; } em_string_t;

---

Au début de mon parser, qui me sert a lire un fichier de configuration, j'alloue dynamiquement un tableau de em_string_t, donc de ce fait, ils ne sont pas initialisés.

Ensuite j'ai une fonction qui me sert a initialiser ma structure :

Code:

---

1 2	em_string_init (em_string_t * string, const char * buffer);

---

Je peux l'initiliaser a rien, comme si je faisais :

Code:

---

1 2	em_string_t uneChaine = {0, NULL};

---

Ou directement avec une chaine, ce que je fais avec mon parser.
Dans le deux cas, effectivement quand je passe mon pointeur, il pointe sur une variable qui n'est pas initialisée.

Mais comment est ce que je peux arranger ça?

Citation:

---

Je pense que si je crée une copie temporaire de la chaîne pour faire la comparaison, ce sera un peu beaucoup, non?

---

Probablement.

Citation:

---

Au début de mon parser, qui me sert a lire un fichier de configuration, j'alloue dynamiquement un tableau de em_string_t, donc de ce fait, ils ne sont pas initialisés.

Ensuite j'ai une fonction qui me sert a initialiser ma structure :
.....
Mais comment est ce que je peux arranger ça?

---

Je n'ai pas une vue assez précise de ton programme, notamment de sa structure, pour donner des conseils utiles.

Ben mon programme commence à être un peu trop conséquent pour pouvoir être copier/coller en exemple... D'ou le fait que valgrind me fait un peu peur...
Quoique je constate, que très peu de programme sont épargnés, donc est ce que valgrind n'est pas un peu sur-alarmant ?

Sinon ben merci beaucoup pour les remarques et conseils, j'ai quand même appris plusieurs choses :)

Bonjour,
Hier, j'avais préparé un message, mais suite à un interruption du réseau il est resté dans ma machine:cry: Le voilà tout de même.
Bonjour,
Etes-vous sûre que str->ptr est un char* ?
Je ne connais pas la définition de de em_string_t.
Tout cela me fait bien penser à des listes chainées, mais il
faudrait en être sûr.
Concernant votre code, pas de souci. La seule amélioration
que je verrais serait de remplacer "value = TRUE;" par
"return TRUE;", mais c'est discutable.
Donc, il faudrait que vous nous donniez la définition des
différentes structures (y compris em_vars_t)
FIN du message d'hier.

Quand vois initialisez un em_string_t la fonction qui le fait doit être quelque-chose comme

Code:

---

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

em_string_t *Initialise(int size, chai *Texte) { em_string_t *str = (em_string_t*)malloc(sizeof(int) + sizeof(char)*(size+1)); if (str) { str->size = size; str->prr = Texte; // donc l'espace est réservé pour ptr[] cad *ptr return str; } else { printf("ERREUR\n"); return NOLL; } }

---

Donc, si la structure em_string_t est initialisée, le pointeur ptr sur un char* est initialisé, mais peut être pas encore renseigné.
Il me semble que la valeur de size doit être soigneusement étudiée et précisée. En particulier elle doit être au moins égale à la longueur +1 de la chaine susceptible s'être stockée.

A mon avis, c'est la seule utilité de la structure em_string_t, sinon, aucune information supplémentaire à char*.

Donc voila en gros comment j'ai codé ma fonction :

Code:

---

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

typedef struct { unsigned int size; char * ptr; } em_string_t;   void em_string_init (em_string_t * string, const char * buffer) { if ( buffer != NULL ) { string->size = strlen(buffer) + 1; string->ptr = em_malloc( string->size * sizeof(char) ); strcpy(string->ptr, buffer); } else { string->size = 0; string->ptr = NULL; } }

---

a savoir, ma fonction s'applique sur des variables et non des pointeurs, par exemple.

Code:

---

1 2 3 4 5 6 7 8

em_string_t uneChaine;   em_string_init(&uneChaine, "Un test"); printf("La phrase est : %s\n", uneChaine.ptr);   em_string_set(&chaine, "Un autre test"); printf("La nouvelle phrase est : %s\n", uneChaine.ptr);

---

Tout en sachant que em_malloc bloquera automatiquement le programme si une erreur est arrivée.

em_string_set(), va automatiquement agrandir la place dans em_string_t.

Ces fonctions me servent uniquement pour que mon parseur soit pratique.

Maintenant je fais peut-être mal quelquechose.

Quand vous appelez cette fonction,

Code:

---

void em_string_init (em_string_t * string, const char * buffer)

---

vous avez forcément déclaré la variable string.
Tout le problème est de savoir comment.
Si vous faites seulement

Code:

---

1 2 3

em_string_t * string; // puis em_string_init ( string, buffer);

---

c'est normal que ça marche pas, string peut valoir n'importe quoi.

ah oui la, je suis d'accord. Si j'emplois ma fonction sur un pointeur NULL ou indéfinis, ca va planter.

Dans mon code, soit je l'utilises comme dans mon exemple.


Ou alors, dans le cas de mon parseur, je détecte d'abord le nombre de clé/valeurs. Ensuite j'alloue un tableau contenant x structures de ce type :

Code:

---

1 2 3 4 5 6

typedef struct { em_string_t name; em_string_t value; } em_var_t;

---

Et donc je repasse après remplir chaque variable em_string_t avec la fonction em_string_init();

Voila en gros comme je charge mes variables.

Ensuite, quand j'utilise une fonction standard en C, strcmp(), valgrind me dit "Invalid read of size xxx". Comme dans les exemples que j'ai cité plus haut. C'est là que je ne comprends pas très bien en fait ? Parce que pour moi, c'est correctement mis en mémoire, j'ai une fonction de print prévu pour m'afficher tout le contenu.

En fait, voila le parseur en mémoire :

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

typedef struct { em_string_t name; em_string_t value; } em_var_t;   typedef struct { em_string_t name;   unsigned int varsCount; em_var_t * vars; } em_varsSection_t;   typedef struct { /* Informations */ em_string_t name; em_string_t filepath; unsigned int version; unsigned int genDate;   /* Sections */ unsigned int sectionsCount; em_varsSection_t * sections; /* malloc */ } em_vars_t;

---

Donc, il y a un conteneur général que j'alloue dynamiquement dans le besoin. Ensuite j'alloue x sections, dans lesquelles j'alloue x entrées.

Et c'est quand je fais mes recherches pour sortir une valeur que crie valgrind.

Bonjour,
Evidemment, c'est un peu difficile sans avoir la totalité du code.
Ce que je pense : cette organisation a pour but principal de pouvoir réserver des zones sans savoir la longueur nécessaire, puis en cas de modification, pouvoir réalouer de l'espace mémoire.
Ceci suppose que la première allocation soit effective, c'est à dire qu'il existe un pointeur sur une zone (peut-être de longueur nulle) qui est fixe, ferme et définitif. C'est le pointeur sur em_string_t. Il contient la longueur de la zone (y compris le zéro terminal) et le poiteur sur cette zone. Il est possible que si la longueur de la chaine est nulle, la longueur de la zone doive être = 1.

Une solution pour comprendre ce qui se passe : repérer la ligne où ça se plante (c'est fait) et juste avant imprimer les choses qu'on veut vérifier, en particulier la valeur des pointeurs
printf("Un pointeur <%p> \n",ptr); // j'ai l'habitude de mettre <...>, c'est plus clair.
Et s'arranger pour être sûr de voir cette impression, même si ça plante.
Le gros avantage d'imprimer un pointeur avec %p, c'est que ça ne se plante jamais dans le printf. Alors que avec %s, si la chaine n'est pas valide, ça plante et on ne voit rien.

Ben oui j'en reviens à la réponse de diogene, je saurais pas tout copier-coller.

Je viens de faire un tour complet. J'ai rajouté un petit système qui conserve l'historique des allocations dynamiques (taille et pointeur). Comme ça je sais s'ils sont libérés ou non et a quel ligne ils ont été alloués. Tout a l'air d'être correct. Je sais pas ...

Mais j'ai remarqué que la valgrind crie très rapidement.

N'oubliez pas que Valgrind attend peut-être une chaine de 1 caractère (le zéro terminal) pour une chaine vide.
Si Valgind crie très rapidement, c'est qu'on lui a appris à le faire et ce n'est pas forcément un défaut.
Je suppose que vous ne disposez pas des sources. Pour une fonction que vous ne comprenez pas très bien ou que vous soupçonnez de crier trop vite, essayez de la réécrire dans votre unité, je crois que normalement, elle va remplacer dans le link la fonction déjà compilée et présente dans la librairie.