Erreur de segmentation et concaténation

Bonjour à tous,
j'essaie de concaténer trois chaînes de caractères tel que je puisse obtenir:
Image_XX.ppm

mon code est le suivant:

Code:

---

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

char* s1 = "Image_"; char* s2 = (char*)num; char* s3 = ".ppm"; char* s4 = NULL; char* nom_image=NULL; s4 = malloc (strlen (s1) + strlen (s2) + 1); s4 = strcat(s1,s2); printf("%s",s4); nom_image = malloc (strlen (s4) + strlen (s3) + 1); nom_image = strcat(s4,s3); printf("%s",nom_image);   printf("En cours d'ecriture de l'image...\n"); fp = fopen(nom_image, "w");

---

Le problème, c'est qu'il me renvoie une erreur du type: erreur de segmentation
je ne comprends pas l'erreur...
Quelqu'un saurait-il ce qui cloche dans mon code?
Merci
Bonne journée

Salut, quand tu fais

Code:

---

1 2	strcat(s1,s2)

---

tu colles à la fin de s1 la string s2. Mais ta string s1 n'a pas suffisament d'espace pour contenir la string s2 à sa suite d'où ton segmentation fault.

Je te propose la chose suivante (non testée) :

Code:

---

1 2 3 4 5 6 7 8 9 10 11

char* s1 = "Image_"; char* s2 = (char*)num; char* s3 = ".ppm"; char s4[strlen(s1)+strlen(s2)+strlen(s3)+10]; strcat(s4,s1); strcat(s4,s2); strcat(s4,s3); printf("%s",s4); char * nom_image = strdup(s4); printf("%s",nom_image);

---

n'oublie pas de faire un

Code:

---

1 2	free(mon_image); mon_image=NULL;

---

et n'oublie pas qu'après un appel à malloc, il faut vérifier que ton pointeur ne soit pas NULL

Je débute en C, ca sert à quoi les dernières lignes de code (le free et remettre image à null)?

le "free" c pour liberer la memoire que tu as allouer avec "malloc"

deja,tu dois convertir ce que malloc te renvoi.
ecri ca :

Code:

---

1 2	s4 = (char* ) malloc (strlen (s1) + strlen (s2) + 1);

---

c ar s4 est de type char *

Je sais pas, ca marche toujours pas, je vois pas d'ou vient l'erreur (j'ai essayé le code tel quel)....

Même en donnant le type:

Code:

---

1 2	s4 = (char*)malloc (strlen (s1) + strlen (s2) + 10);

---

Ca ne marche pas non plus

Citation:

---

Envoyé par scorpion.os

deja,tu dois convertir ce que malloc te renvoi.
ecri ca :

Code:

---

1 2	s4 = (char* ) malloc (strlen (s1) + strlen (s2) + 1);

---

c ar s4 est de type char *

---

Justement non, il ne faut pas convertir le retour de malloc.
Pour apprendre à utiliser correctement malloc(), cf. l'excellent site d'Emmanuel : http://emmanuel-delahaye.developpez....tes.htm#malloc

Cela dit, ton code initial semble trop compliqué. Pourquoi n'utilises-tu pas sprintf(), ou, mieux, snprintf() ?
Par exemple, comme ça :

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

#include <stdlib.h> #include <stdio.h> #include <string.h>   int main(void) { const char* num = "123"; char* nom_image = NULL; int longueur = 11+strlen(num);   nom_image = malloc(longueur); if (nom_image == NULL) { perror("Erreur sur malloc"); exit(EXIT_FAILURE); } memset(nom_image, '\0', sizeof(nom_image));   snprintf(nom_image, longueur, "Image_%s.ppm", num);   printf("nom_image = '%s'\n",nom_image);   free(nom_image); nom_image = NULL;   exit(EXIT_SUCCESS); }

---

Bien, j'ai fais ainsi:

Code:

---

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

char* nom_image = NULL; int longueur = 15;   nom_image = malloc(longueur); if (nom_image == NULL) { perror("Erreur sur malloc"); exit(EXIT_FAILURE); } memset(nom_image, '\0', sizeof(nom_image)); snprintf(nom_image, longueur, "Image_%d.ppm", num); printf("nom_image = '%s'\n",nom_image);     printf("En cours d'ecriture de l'image...\n"); fp = fopen(nom_image, "w"); free(nom_image); nom_image = NULL;

---

Ca fonctionne!!
En revanche j'ai une erreur de segmentation ensuite, mais ca ne concerne plus la concaténation de la chaine... Encore du boulot en perspective!

POur savoir ou se produit une erreur de segmentation, il faut utiliser perror???

Citation:

---

Envoyé par Flophx

POur savoir ou se produit une erreur de segmentation, il faut utiliser perror???

---

Pas forcément. La fonction perror permet d'afficher le message correspondant à la dernière erreur rencontrée lors de l'appel d'une fonction système ou de la bibliothèque. Elle est associée à la variable errno.

Les erreurs de segmentation font partie de ces erreurs difficiles à localiser, parce qu'elles peuvent très bien se produire à des endroits inattendus...
Pour les détecter plus facilement, il faut commencer par systématiquement tester le retour des fonctions, pour détecter si une erreur s'est produite.
Un bon paramétrage du compilateur permet aussi d'identifier certains problèmes potentiels dans le code (variables non initialisées, etc.).

Dans tous les cas, le site http://emmanuel-delahaye.developpez.com/notes.htm constitue une bonne source d'informations.

Il y plusieurs erreurs dans les messages précédents....

1) En ce qui concerne l'allocation des chaînes de caractères, c'est toujours mieux d'utilser calloc() que malloc() :

calloc() intialise l'ensemble de la place allouée à NULL. Pour ce qui est des nombres, en général, ça ne change rien, mais pour une chaîne, où la fin de la chaîne est déterminée par NULL ou '\0', ça change tout.. En effet malloc() ne fait que réserver la place.Si il y a à cet endroit en mémoire quelque chose (un reste de code, n'importe quoi, mais pas des NULL), et si les opérations que l'on fait sur la chaîne (par exemple strlen(), ou un déplacement de pointeur) ne sont pas précédées d'une fonction qui effectivement copie une chaîne (strcpy() par exemple) qui, elle, insère une NULL à la fin, on se retrouve avec une chaîne qui éventuellement n'a pas de marqueur de fin. D'où crash...

Ceci évite d'avoir à faire le memset...

2) Si tu t'attends à avoir "Image_XX" et que tu fais

Citation:

---

snprintf(nom_image, longueur, "Image_%d.ppm", num);

---

ça ne marchera pas pour des nombres inférieurs à 10..

A ce compte là il faut faire :

Citation:

---

sprintf(nom_image, "Image_%.0d.ppm", num);

---

le "." apès le % indique que même si le nombre ne prendrait qu'un chiffre (0 à 9) on l'écrit avec 2. Le "0" indique qu'on remplace le blanc par un "0". Enfin le 2 signifie que l'on met 2 chiffres significatifs :

Donc on aura "Image_12" ou "Image_02"..
Si on avait pas mis le "0", on aurait "Image_12" ou "Image_ 2"
Si on avait pas mis le "." on aurait "Image_12" ou "Image_2"

Mais en mettant 2, on limite à 100 le nombre de noms possibles. En effet,

la 101ième image aura alors le nom "Image_10"...

3) La réassignation à NULL à la fin ne sert que si tu te resserviras de la variable dans la même routine plus tard..
De même le free ne sert à rien si la suite est un exit... La mémoire est automatiquement libérée quand on fait un exit... Si c'est la sortie d'une routine oui ça sert..


4) ton "num" au début était un char*, maintenant tu le traites comme un int..
(%d)

5) Pour débugger, il y a plusieurs manières:

la plus simple (et à mon avis la plus rapide si a) soit le code est très complexe, b) soit le code est très simple, est de faire d'abord un "printf ("Passage ici numéro 1") ...." aux endroits ou tu soupçonnes le problème..

Sinon le debugger ddd est par exemple très bien pour te dire où ça a planté.. (ou gdb)

6) Quand zooro écrit

Citation:

---

int longueur = 11+strlen(num);

---

C'est pas vraiment une manière correcte.. ça marche, oui.. Mais à mon avis, pour rendre le code a) plus facle à lire et b) plus facilement portable (autres architectures, autres langages, etc), c'est mieux de séparer déclarations et assignations si l'on fait appel à une fonction....

7) Quand salseropom écrit :

Citation:

---

printf("%s",s4);
char * nom_image = strdup(s4);
printf("%s",nom_image);

---

c'est totalement inutile, puisque s4 a déjà été allouée, et que donc il suffit d'appeler s4 nom_image...

Opps



au point 2, il fallait lire :

il faut faire :

Citation:

---

sprintf(nom_image, "Image_%.02d.ppm", num);

---

Ca compile sans erreur:

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

#include <stdlib.h> #include <stdio.h> #include <string.h>   int main(void) { FILE * fp = NULL; const char* num = "123"; char* nom_image = NULL; int longueur = 15;     nom_image = malloc(longueur); if (nom_image == NULL) { perror("Erreur sur malloc"); exit(EXIT_FAILURE); } memset(nom_image, '\0', sizeof(nom_image)); snprintf(nom_image, longueur, "Image_%s.ppm", num); printf("nom_image = '%s'\n",nom_image);     printf("En cours d'ecriture de l'image...\n"); fp = fopen(nom_image, "w"); free(nom_image); nom_image = NULL;   exit(EXIT_SUCCESS); }

---

Tu avais un problème ici:

Code:

---

snprintf(nom_image, longueur, "Image_%s.ppm", num);

---

avant tu utilisais %d alors que c'est une chaîne de caractères.

Là ca compile sans erreur avec GCC et les options:

Citation:

---

-W -Wall -pedantic

---

et pas de problème à l'exécution !

Citation:

---

Envoyé par souviron34

Il y plusieurs erreurs dans les messages précédents....
1) En ce qui concerne l'allocation des chaînes de caractères, c'est toujours mieux d'utilser calloc() que malloc() :
(...)
Ceci évite d'avoir à faire le memset...

---

Effectivement. Mais bon, c'est un choix personnel, du moment que l'on n'oublie pas le memset.

Citation:

---

Envoyé par souviron34

3) La réassignation à NULL à la fin ne sert que si tu te resserviras de la variable dans la même routine plus tard..
De même le free ne sert à rien si la suite est un exit... La mémoire est automatiquement libérée quand on fait un exit... Si c'est la sortie d'une routine oui ça sert..

---

Il faut TOUJOURS libérer la mémoire qu'on a allouée. Et remettre les pointeurs libérés à NULL est une bonne habitude à prendre.

Citation:

---

Envoyé par souviron34

6) Quand zooro écrit (...)
C'est pas vraiment une manière correcte.. ça marche, oui.. Mais à mon avis, pour rendre le code a) plus facle à lire et b) plus facilement portable (autres architectures, autres langages, etc), c'est mieux de séparer déclarations et assignations si l'on fait appel à une fonction....

---

Pourquoi ça ne serait pas correct ? Ce n'est pas la meilleure solution, sans doute, mais c'est tout à fait correct pour un exemple.
Y aurait-il un intérêt quelconque à l'écrire plutôt en deux lignes ?

Code:

---

1 2	int longueur; longueur = 11+strlen(num);

---

je ne vais pas polémiquer, mais :

Citation:

---

zooro a écrit :
Il faut TOUJOURS libérer la mémoire qu'on a allouée. Et remettre les pointeurs libérés à NULL est une bonne habitude à prendre.

---

c'est faux dans les cas que j'ai cité :

1) remettre les pointeurs à NULL ne sert à rien si on ne s'en sert plus
2) libérer la mémoire ne sert à rien si on sort du programme (le free ne libère que dans la portion allouée au programme au runtime par le loader, et donc quand on sort du programme, cette portion est comprise dans la portion gérée par le loader et libérée par lui) : il suffit de faire un "top" en lancant un programme allouant 20 megs et de regarder ce qui se passe sans libérer mais en quittant....

Par contre OUI il faut toujours libérer la mémoire si on est dans une routine, ou si on ne fait pas d'exit...

Et il FAUT IMPERATIVEMENT initialiser les pointeurs à NULL dans les déclarations si on s'en sert pour faire de l'allocation dynamique dans des routines (voir autre discussion http://www.developpez.net/forums/sho...d.php?t=269868
dans le forum ), car cela peut générer des crash suivant le niveau de profondeur et le nombre de paramètres de la routine.

Citation:

---

Envoyé par souviron34

je ne vais pas polémiquer, mais :
c'est faux dans les cas que j'ai cité :
(...)

---

Un principe important à garder à l'esprit est que celui qui alloue une ressource (dont la mémoire) est chargé de la libérer.

Si ce n'est pas fait, dans un cas particulier, il est intéressant de le mentionner par un commentaire dans le code, ce qui permettra de s'en souvenir lors d'une modification ultérieure, ou d'une relecture de code.

Tu as raison, dans ce cas précis.
Mais prendre de bonnes habitudes permet d'éviter un oubli qui peut s'avérer problématique dans une autre situation.

Oulà, passionnante cette discussion!!
POur ma part, j'ai utilisé %d parce que j'avais déclaré un int num!
Comme je voulais concaténer directement deux char, il fallait bien que je le caste!
Mais maintenant ca va nickel
Merci de vous êtes penchés sur mon cas
A plus
F.

Bien, je suis de retour avec une nouvelle erreur de segmentation, lié à ce bout de code:

Code:

---

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

int longueur_streaming= ((unsigned)strlen(imgbuffer)); printf("taille: %d\n",longueur_streaming);   unsigned char* Y[4*longueur_streaming/6]; unsigned char* U[longueur_streaming/6]; unsigned char* V[longueur_streaming/6];   int i; for(i=0; i<4*longueur_streaming/6; i++){ *Y[i]=imgbuffer[i]; }   for(i=4*longueur_streaming/6; i<5*longueur_streaming/6; i++){ *U[i-4*longueur_streaming/6]=imgbuffer[i]; }   for(i=5*longueur_streaming/6; i<longueur_streaming; i++) { *V[i-(5*longueur_streaming/6)]=imgbuffer[i]; }

---

Pour résumé, j'ai dans imgbuffer un flux d'octet correspondant à une image avec une palette YUV420P.
POur reconvertir l'image au format RGB24, je dois reconstituer trois tableaux YUV (en fait dans le flot, la première partie correspond d'abord au Y, puis au U, et enfin au V, il n'y a pas d'entrelacement, avec une partie Y 4 fois plus importante que U et V)

Mais il me renvoie de nouveau une erreur de segmentation....
Qu'ai je encore pas compris??? Ces erreurs sont vraiment pernicieuses....

Ah oui, et j'oubliais, le printf initial me renvoie une valeur de 0, ce qui est impossible....:aie:

Citation:

---

Envoyé par souviron34

calloc() intialise l'ensemble de la place allouée à NULL.

---

Non, calloc n'intialise pas tout a NULL mais a "all bits zero". La difference est importante car NULL ne signifie pas forcement tout les bits a 0.
En outre NULL n'a de sens que dans un contexte pointeur, il ne doit pas etre utilise a la place de 0 ou de '\0' (l'inverse est par contre possible, 0 peut etre utilise a la place de NULL pour initialiser un pointeur, mais on perd inutilement l'interet documentaire de NULL).

Citation:

---

Envoyé par souviron34

6) Quand zooro écrit

Citation:

---

int longueur = 11+strlen(num);

---

C'est pas vraiment une manière correcte..

---

Pourquoi ?

Citation:

---

Envoyé par souviron34

Mais à mon avis, pour rendre le code a) plus facle à lire et

---

Question de gout, personellement je prefere d'intialiser immediatement les variables plutot que de le faire plus (trop?) tard. Et je sais en lisant la definition la valeur intiale.

Citation:

---

Envoyé par souviron34

b) plus facilement portable (autres architectures, autres langages, etc),

---

Je ne vois pas trop a quel probleme tu fais allusion

Citation:

---

Envoyé par souviron34

1) remettre les pointeurs à NULL ne sert à rien si on ne s'en sert plus
2) libérer la mémoire ne sert à rien si on sort du programme (le free ne libère que dans la portion allouée au programme au runtime par le loader, et donc quand on sort du programme, cette portion est comprise dans la portion gérée par le loader et libérée par lui) : il suffit de faire un "top" en lancant un programme allouant 20 megs et de regarder ce qui se passe sans libérer mais en quittant....

---

Certes, dans ce cas particulier ce n'est pas indispensable.
Mais il est tout de meme preferable de liberer la memoire et de remettre le pointeur a NULL car

* La plupart du temps il faut le faire, cet exemple est plutot le cas particulier. Or il est plus simple d'acquerir et d'appliquer systematiquement des automatismes plutot que de se poser a chaque la question de savoir si c'est indispensable ou non, au risque de se tromper ou d'oublier de le faire.
* Un code est tres souvent amener a evoluer, et ce qui n'est pas indispensable dans une premiere version peut le devenir par la suite. Autant le faire correctement du premier coup non ? D'autant que les probabilites d'oublier de desallouer et de remettre a NULL lors d'une telle evolution ne sont pas negligeable.