Discussion :

[enzo68290]

Bonjour,

Je réalise en ce moment un programme sous linux en C.

Celui-ci fait une gestion utilisateur / contact et d'autre chose dont je ne pourrais pas parler ici.

J'ai un petit problème avec le fonctionnement de mon programme

J'ai une structure utilisateur qui comprends une sous - structure contact tableau.

En effet, on considèrera que pour utiliser le logiciel que je développe, un utilisateur logique peut avoir plusieurs contacts physique.


Dans l'ordre, j'ai une fonction de saisie utilisateur,.

Celle-ci contient une boucle while, qui dit que tant que je n'ai pas atteint le nombre de contact que je veux pour l'utilisateur en cours, je boucle sur une sous-fonction de création de contact.

de mon main j'appel donc ma fonction :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
 
int createuser(S_User *user)

qui elle même appelle une ou plusieurs fois :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
 
int createcontact(S_Contact *contact)

Le problème est que createcontact me retourne une structure ou toute les valeurs sont remplie sauf une (la première), qui est soit vide, soit rempli de caractère bizarre.

Ne maitrisant pas trop le C pour l'instant, et me servat de ce projet pour m'amélioré, je me permet de faire appel à vous pour comprendre.

Concernant la saisie, j'ai lu que fgets était ce qu'il y à de plus souple pour faire quelque chose de pro, c'est comme cela que je procède, sauf pour les nombres ou j'utilise scanf.

Aussi, j'ai remarquer que la structure user retourner au main avait le même problème, je m'en fiche un peu plus car c'est createuser qui fera l'insertion dans la base.

Je vous transmet les deux structures user et contact présent dans mon header :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

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
 
#define MAX_CONTACT 10
 
typedef struct T_S_Contact
{
	long id_con;	
 
	char *nom_con;
	char *prenom_con;
	char *service_con;
 
	char *tel_fix_con;
	char *tel_mob_con;
	char *fax_con;	
 
	char *mail_con;
 
 
} S_Contact;
 
 
 
typedef struct T_S_User
{
 	long id_util; 
 
 	char *raison_social_util;
 	char *adr_util;
 	char *ville_util;
 	char *cp_util;
 
 	int fab_cli;
 
 	S_Contact contacts[MAX_CONTACT];
 	int nb_contact;
 
 	char *iso_pays;			
 	int id_sect;
 
} S_User;

En espérant que comme toujours vous puissiez m'aider à éclaircir ce problème dont je ne trouve pas pour l'instant de solution sans pour autant désespérer.

[diogene]

Il faut au minimum avoir le code de createcontact() pour pouvoir dire quelque chose.

[enzo68290]

Voici le code de la fonction createcontact :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

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
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
 
int createcontact(S_Contact *contact)
{
 
 
	char nom_con[251];
	char prenom_con[251];
	char service_con[251];
 
	char tel_fix_con[21];
	char tel_mob_con[21];
	char fax_con[21];
 
	char mail_con[251];
 
	contact->id_con = 0;
 
	int test = 0;
 
	purger();
 
	printf("Enter the surname of contact\n");
	fgets(nom_con, sizeof nom_con, stdin);
	contact->nom_con = (char *)nom_con;
	clean(nom_con);
 
	printf("Enter the first name of contact\n");
	fgets(prenom_con, sizeof prenom_con, stdin);
	contact->prenom_con = (char *)prenom_con;
	clean(prenom_con);
 
	printf("Enter the service of contact\n");
	fgets(service_con, sizeof service_con, stdin);
	contact->service_con = (char *)service_con;
	clean(service_con);
 
	printf("Enter the phone number of contact\n");
	fgets(tel_fix_con, sizeof tel_fix_con, stdin);
	contact->tel_fix_con = (char *)tel_fix_con;
	clean(tel_fix_con);
 
	printf("Enter the cellphone number of contact\n");
	fgets(tel_mob_con, sizeof tel_mob_con, stdin);
	contact->tel_mob_con = (char *)tel_mob_con;
	clean(tel_mob_con);
 
	printf("Enter the fax number of contact\n");
	fgets(fax_con, sizeof fax_con, stdin);
	contact->fax_con = (char *)fax_con;
	clean(fax_con);
 
	printf("Enter the mail of contact\n");
	fgets(mail_con, sizeof mail_con, stdin);	
	contact->mail_con = (char *)mail_con;
	clean(mail_con);
 
	/*
 
	printf("================ valeur pendant saisie ========\n");
	printf("id_con:      %ld\n", contact->id_con);
	printf("nom_con:     %s\n", contact->nom_con);
	printf("prenom_con:  %s\n", contact->prenom_con);
	printf("service_con: %s\n", contact->service_con);
	printf("tel_fix_con: %s\n", contact->tel_fix_con);
	printf("tel_mob_con: %s\n", contact->tel_mob_con);
	printf("fax_con:     %s\n", contact->fax_con);
	printf("mail_con:    %s\n", contact->mail_con);
 
	*/
 
 
 
	test = analysecontact(contact);
 
	if(test != 0)
	{
 
		return test;
 
	}
 
		return 0;
 
 
}

avec mes excuses pour avoir oublié cette partie ...

[jouana]

A vu de nez je dirais juste une chose porté des variables.

Si on fait la diff entre tableau et pointeur ce n'est pas pour rien

Deux, trois points à revoir.

• Vérification des valeurs de retour des fonctions utilisées c'est bien
• Avec strlen tu peux récupérer la longueur de ta chaine afin de connaitre sa taille et faire un malloc proprement
• Et tu peux utiliser une variable buffer[1024] et boucler dessus je pense que c'est pas mal aussi

Sinon tes tableaux sont font partie de la mémoire de ta fonction donc quand ta fonction et écrasé tes variables aussi en gros c'est ça la porté des variables d'ou les pointeurs qui ne se situe pas dans la même stack, si je ne dit pas de connerie

[enzo68290]

Oulah, tu as l'air dêtre bien plus expert que moi car je n'ai pas tout compris.

- vérification des valeurs de retour des fonctions utilisées c'est bien :

c'est un petit minimum ^^.

- Avec strlen tu peux récupérer la longueur de ta chaine afin de connaitre sa taille et faire un malloc proprement :

je suis d'accord, mais sur toutes les variables ? je n'ai pas compris à quoi je dois appliquer ça

- Et tu peux utiliser une variable buffer[1024] et boucler dessus je pense que c'est pas mal aussi

pareille, je n'ai pas compris ou et dans quel cas appliquer ça.

[Médinoc]

- Avec strlen tu peux récupérer la longueur de ta chaine afin de connaitre sa taille et faire un malloc proprement :

je suis d'accord, mais sur toutes les variables ? je n'ai pas compris à quoi je dois appliquer ça

Sur tous les champs de type char* de la structure.

Pour t'aider, tu peux faire une fonction pour dupliquer une chaîne avec malloc():

Code C :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
char * strdup(char const *s)
{
	size_t lengthInChars = strlen(s);
	size_t sizeInChars = lengthInChars + 1;
	size_t sizeInBytes = sizeInChars * sizeof(*s);
	char * ret = malloc(sizeInBytes);
	if(ret != NULL)
		strcpy(ret, s);
	return ret;	
}

[enzo68290]

Merci à toi, je vais essayé ça se soir pour savoir si c'est bien la qu'est le problème, je te tiendrais au courant demain pour savoir si effectivement il manquait un malloc par chaine.

[Ngork]

Medinoc te propose une bonne fonction avec strdup(), et du coup, tu n'as plus qu'à écrire pour chacun des char * de ta structure (ici, prenom_con) :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

contact->prenom_con = strdup(prenom_con);

Toutefois, soit tu vérifies ta valeur de retour de srtdup(), soit plus simplement tu modifies légèrement la fonction pour qu'elle te retourne une chaîne vide et non un NULL quand l'allocation ne peut pas se faire :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
char * strdup(char const *s)
{
  size_t lengthInChars = strlen(s);
  size_t sizeInChars = lengthInChars + 1;
  size_t sizeInBytes = sizeInChars * sizeof(*s);
  char * ret = malloc(sizeInBytes);
 
  if (ret != NULL)
    strcpy(ret, s);
  else
    ret="";
 
  return ret;
}

[jouana]

Je me permet un petite modification

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
 
char * strdup(char const **s)
{
  char * ret = malloc( (strlen(*s)) + 1 * sizeof(*ret));
 
  if (ret != NULL)
    return strcpy(ret, *s);
 ret[0] = '\0';
  return ret;
}

[Ngork]

Je me permet un petite modification

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
 
char * strdup(char const **s)
{
  char * ret = malloc( (strlen(*s)) + 1 * sizeof(*ret));
 
  if (ret != NULL)
    return strcpy(ret, *s);
 ret[0] = '\0';
  return ret;
}

Avec cette modification, tu introduis une écriture dans une mémoire non réservée en cas d'échec de l'allocation, idéale pour faire planter le programme et justement ce que je voulais éviter !

Par ailleurs, deux return dans une même fonction, c'est le début des ennuis.
Certes, avec une fonction de 5 lignes, c'est facile à maintenir, mais si ta fonction fait 200 lignes et comporte 40 return, cela va être sympa le jour où tu devras changer tes valeurs de retour !

Donc, je persiste dans ma proposition :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
  if (ret != NULL)
    strcpy(ret, s);
  else
    ret="";

Ainsi, il n'y a qu'un seul return et, en cas d'échec de l'allocation, le pointeur retourné pointe sur une chaîne vide.

[diogene]

@Ngork

soit plus simplement tu modifies légèrement la fonction pour qu'elle te retourne une chaîne vide et non un NULL

Ce n'est pas une bonne idée : lorsqu'il va falloir désallouer ce qui l'a été avec ce strdup(), on va planter inopinément sur le free() de cette chaine vide qui n'a pas été allouée dynamiquement.

@jouana
Quel est l'intérêt d'un paramètre en char const ** ?
Plante sur ret[0] = '\0'; puisque ret ==NULL.

[Ngork]

@Ngork Ce n'est pas une bonne idée : lorsqu'il va falloir désallouer ce qui l'a été avec ce strdup(), on va planter inopinément sur le free() de cette chaine vide qui n'a pas été allouée dynamiquement.

Tu as raison, il faut prévoir le cas lors de la libération de la structure, en ne libérant que si la chaîne comporte au moins un caractère, ce qui implique de modifier légèrement la fonction strdup() pour qu'une entrée vide de l'utilisateur ne donne pas lieu non plus à allocation de mémoire, ce qui relève de fait d'une meilleure gestion.

Soit la fonction strdup() modifiée :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
 
char * strdup(char const *s)
{
  size_t lengthInChars = strlen(s);
  size_t sizeInChars = lengthInChars + 1;
  size_t sizeInBytes = sizeInChars * sizeof(*s);
  char * ret = NULL;
 
  if (lengthInChars != 0)
    {
      ret = malloc(sizeInBytes);
      if (ret != NULL)
        strcpy(ret, s);
    }
 
  if (ret == NULL)
    {
      ret = "";
    }
 
  return ret;
}

Et une nouvelle fonction strfree() :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
 
void strfree(char const *s)
{
  if (s != NULL)
    {
      if (s[0] != 0)
        {
          free(s);
        }
    }
}

Ainsi strdup() garde sa facilité d'utilisation, sans besoin de contrôler la valeur de retour, et pour la libération de la structure, strfree() s'utilise en lieu et place de free() ...

[diogene]

Pas valable :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
char * p = strdup("");
strfree(p); // PLANTAGE

[jouana]

Ouais désolé beaucoup de fautes énorme dans le code.
J'aurais du éviter le ré edit (conseil du code de la route la 1ère intuition est souvant la bonne ), à la base un retour NULL avec un char * et non pas ** qui ne sere clairement à rien.

Pour la réponse de la fonction a 200 lignes avec des return partt.
Euh j'ai envie de dire, rien que l'idée qu'une fonction dépasse 25 - 30 lignes me donne envie de vomir et que quand je tombe sur ce type de fonction dépassé l'heure de digestion et des tic nerveux estompé je peux commencer à travailler la dessus.

[Ngork]

Pas valable :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
char * p = strdup("");
strfree(p); // PLANTAGE

Bah, non !
Pas de plantage, car p pointe vers "" et que le cas est couvert par strfree(), relis mon code ...

[Médinoc]

Fuite de mémoire, alors.

[Ngork]

Fuite de mémoire, alors.

Bah non !
Pas de fuite de mémoire non plus :

si strdup() reçoit une chaîne vide -> pas de réservation mémoire, valeur de retour pointe vers ""
si strdup() reçoit une chaîne non vide et que l'allocation réussit, valeur de retour pointe vers la chaîne allouée
si strdup() reçoit une chaîne non vide et que l'allocation échoue, valeur de retour pointe vers ""

si strfree() reçoit un pointeur NULL, il ne fait rien
si strfree() reçoit une chaîne vide, il ne fait rien
si strfree() reçoit une chaîne non vide, il libère la mémoire

Donc, si ces deux fonctions sont utilisées, je ne vois pas comment il pourrait y avoir une quelconque fuite de mémoire !

Tu as des précisions ou il s'agissait juste d'un baroud d'honneur ?

[Médinoc]

Ah, j'avais mal compris le fonctionnement de ta strdup() modifiée.

Mon baroud d'honneur, c'est plutôt ça: Ça ne passera pas en -Wwrite-strings, ni en C++.

[Ngork]

Ah, j'avais mal compris le fonctionnement de ta strdup() modifiée.

Mon baroud d'honneur, c'est plutôt ça: Ça ne passera pas en -Wwrite-strings, ni en C++.

Bon, j'ai renommé les fonctions strdup() en copie_chaine() et strfree() en libere_chaine() pour ne pas créer de confusion avec les fonctions existantes :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

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
47
48
49
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
 
char * copie_chaine(char const *s)
{
  size_t lengthInChars = strlen(s);
  size_t sizeInChars = lengthInChars + 1;
  size_t sizeInBytes = sizeInChars * sizeof(*s);
  char * ret = NULL;
 
  if (lengthInChars != 0)
    {
      ret = (char *) malloc(sizeInBytes);
      if (ret != NULL)
        strcpy(ret, s);
    }
 
  if (ret == NULL)
    {
      ret = (char *) "";
    }
 
  return ret;
}
 
void libere_chaine(char *s)
{
  if (s != NULL)
    {
      if (s[0] != 0)
        {
          free(s);
        }
    }
}
 
int main()
{
  char *texte = copie_chaine("Hello World !");
 
  printf(texte);
 
  libere_chaine(texte);
 
  getchar();
 
  return 0;
}

Et voici ce que j'obtiens à la compilation avec l'option -Wwrite-strings :

-------------- Clean: Release in test ---------------

Cleaned "test - Release"

-------------- Build: Release in test ---------------

gcc -Wextra -Wall -Wwrite-strings -O2 -c main.c -o main.o
g++ -o test main.o -s
Output size is 3,66 KB
Process terminated with status 0 (0 minutes, 0 seconds)
0 errors, 0 warnings

Donc pas de souci avec l'option -Wwrite-strings !

Quant à la compilation comme du code C++, pas de problème non plus :

-------------- Clean: Release in test ---------------

Cleaned "test - Release"

-------------- Build: Release in test ---------------

g++ -Wextra -Wall -Wwrite-strings -O2 -c main.cpp -o main.o
g++ -o test main.o -s
Output size is 3,66 KB
Process terminated with status 0 (0 minutes, 0 seconds)
0 errors, 0 warnings

Allez, j'apprécie ton baroud, mais il faut savoir renoncer, non ?

[Invité]

Bouh le tricheur !
Comme par hasard un cast qui apparaît

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

ret = (char *) "";