Salut,
J'ai le code suivant qui compile bien. Mais à son lancement il me donne un segfault. Je lance gdb dessus et je vois que le problème vient de la 65e itérations de la boucle while (au getline).
Si le bug apparaissait à la première itération j'aurais compris (enfin j'espère), mais ce qui me chiffonne c'est ce délai de 65 itérations.
À noter que si je change ITEM_SIZE_STEP à 10 le problème apparait à la 331e itération, et pour ITEM_SIZE_STEP=100 à la 3301e itération.
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
int main(void)
{
FILE *fp = stdin;
char **items;
const unsigned int ITEM_SIZE_STEP = 1;
unsigned int itemsSize = ITEM_SIZE_STEP;
unsigned int itemLen = 0;
unsigned int i = 0;
if ((items = (char **) malloc(itemsSize * sizeof(char **))) == NULL)
{
puts("Couldn't allocate memory for items list");
exit(1);
}
while (getline(items+i, &itemLen, fp) != -1)
{
++i;
if (i >= itemsSize)
{
char **tmpItems;
tmpItems = (char **) realloc(items, (itemsSize+ITEM_SIZE_STEP)*sizeof(char **));
if (tmpItems == NULL)
{
puts("Error");
exit(2);
}
else
{
itemsSize += ITEM_SIZE_STEP;
items = tmpItems;
}
}
}
free(items);
return 0;
}
# vive le man
Posté par gaaaaaAab . Évalué à 2.
man getline:
le problème ici c'est que tu laisses getline faire l'allocation de chaque ligne, mais tu ne t'assures pas que items + i soit bien NULL avant chaque entrée dans getline.
Au passage, il faudrait aussi que tu libères les blocs alloués par getline.
[^] # Re: vive le man
Posté par tuxg . Évalué à 1.
Euh, en fait j'avais un memset dans la boucle qui s'assurait de mettre les nouvelles mémoires à 0, mais le problème était le même. Du coup je l'ai enlevé (je comptais sur le getline mais apparemment j'avais pas bien compris).
N'empêche, le problème reste entier. :(
[^] # Re: vive le man
Posté par gaaaaaAab . Évalué à 2.
hmmm faut voir comment il était. Peux-tu poster la version avec memset ?
(sinon, pour info, j'ai fait 2/3 tests avant de poster, j'ai une version qui marche que je pourrais mettre si tu veux, mais je pense que c'est plus intéressant si on réfléchit sur ton code)
[^] # Re: vive le man
Posté par tuxg . Évalué à 2.
J'avais un :
à la fin du else.
[^] # Re: vive le man
Posté par gaaaaaAab . Évalué à 2.
et oui, mais ça, ça ne va pas !
ITEM_SIZE_STEP vaut 1, mais tu veux mettre 0 pour un pointeur, qui tient sur plus qu'un seul octet.
ça serait mieux comme ça:
tu aurais pu ajouter un log en fin de boucle pour afficher *(items + i), ça t'aurais permis de te rendre compte que memset ne faisait pas ce que tu attendais (en tout cas, c'est comme ça que j'ai fait).
Pour conclure (désolé, je radote :), n'oublie pas de libérer la mémoire allouée par getline.
[^] # Re: vive le man
Posté par gaaaaaAab . Évalué à 2.
ah oui, j'ai aussi enlevé un niveau d'indirection sur items dans le memset. Ton memset travaillait juste sur l'entrée i de ton tableau items au lieu de travailler sur le tableau.
[^] # Re: vive le man
Posté par gaaaaaAab . Évalué à 2.
heu non, il travaillait pas sur l'entrée i, mais il travaillait pas au bon endroit, ça, c'est sûr :)
[^] # Re: vive le man
Posté par benja . Évalué à 2.
bien vu j'allais le signaler aussi ;)
[^] # Re: vive le man
Posté par tuxg . Évalué à 2.
Merci ça marche maintenant.
Pour la libération, je le fais. C'est juste que je l'ai pas mis dans ce que j'ai collé.
[^] # Re: vive le man
Posté par benja . Évalué à 2.
Un memset (même correctement utilisé :p) pour mettre un pointeur à null, c'est un peu overkill, non ? *(items+i)=0;
[^] # Re: vive le man
Posté par gaaaaaAab . Évalué à 2.
tant que ITEM_SIZE_STEP vaut 1, oui, mais ça devient intéressant pour des valeurs plus élevées.
[^] # Re: vive le man
Posté par tuxg . Évalué à 1.
C'est pas un pointeur mais plusieurs.
Et je ne sais pas si une boucle serait plus efficace.
[^] # Re: vive le man
Posté par benja . Évalué à 1.
Oui et, quel est l'intérêt ?
[^] # Re: vive le man
Posté par benja . Évalué à 1.
Bon aller ok je te l'accorde mais je trouve pas cela très lisible. Tant qu'à faire dans le dégueu, tu pourrais essayer getline((*(items+i)=0,items+i),...) (suis pas sur que ce soit du C valide par contre)
As-tu penser à corriger ton utilisation de &itemLen ? Si non, enlève-le il ne sert à rien, sauf p-ê à perturber le relecteur.
[^] # Re: vive le man
Posté par tuxg . Évalué à 1.
Effectivement le itemLen ne me sert a rien. Mais je ne connais pas d'autre solution. D'après le man mettre 0(NULL) retourne une erreur.
Si t'as un moyen moins dégueu, je suis preneur.
[^] # Re: vive le man
Posté par benja . Évalué à 2.
Effectivement tu as bien raison. C'est que je trouve le C dégueu en général (enfin y a bien pire hein, et puis on a pas toujours le choix ;) )
Sinon pour la clareté, quid d'utiliser une variable temporaire, ou d'aliaser ton pointeur que le compilo optimise de toute façon. Histoire que le relecteur ne doivent pas, lui aussi, se taper la lecture du man pour comprendre toutes les subtilités :p genre avoir un *(items+i)=newItem. Bon c'est vrai que c'est une histoire de goût, mais ça t'aurais sûrement permis d'éviter le bug dès le départ.
Aller, bonne continuation !
# Je suppose...
Posté par benja . Évalué à 1.
Je suppose que tu essaye d'enregister chaque ligne... Petit exercice, essaye de les afficher à chaque itération. Tu vois où est ton erreur ?
Un indice: vu que chaque ligne est supposée de longueur différente (et à priori, inconnue); tu ne peux pas utiliser un simple tableau à 2 dimensions. (Enfin si tu pourrais mais ce ne serait ni simple ni efficace, je te laisse chercher pourquoi, cela serait un bon exercice ceci dit :p).
La solution c'est d'allouer un buffer pour chaque ligne (cf. man, getline va faire une realloc dessus s'il est trop petit) et d'ajouter son addresse dans ton tableau, que tu devra effectivement redimentionner à chaque coup.
[^] # Re: Je suppose...
Posté par benja . Évalué à 1.
Comme noté plus haut, pas besoin de d'allouer un buffer toi-même si tu passe une addresse null à getline. 'bref
[^] # Re: Je suppose...
Posté par tuxg . Évalué à 1.
Ça me donne le même résultat que gdb (ça s'affiche normalement jusqu'au 65e élément).
[^] # Re: Je suppose...
Posté par benja . Évalué à 1.
Essaye de toutes les afficher à chaque itération...
Le truc c'est que tu avance ton pointeur d'une seule "case" à la fois, donc getline écrase tout ce qui se trouve à la suite de cette case... Aussi, tu lui passe un pointeur "invalide" dans le sens qu'il est illegal pour appeller realloc (ce que getline fait forcément vu que itemLen=0*) car ce n'est plus l'addresse retournée par malloc -> bug (cf. man realloc). Bref t'essaye de mettre dans la zone pointée par ton char** en même temp et un tableau de pointeur et des strings... Contente toi de passer un null à getline, il va t'allouer ta ligne tout seul ensuite t'as plus qu'à ajouter le pointeur à ton tableau comme tu le fais (il me semble) correctement.
newItem = NULL;
while ( getline(&newItem,0,fp) != -1) { ... ajout de newItem à items; newItem=NULL};
Aussi, again, lis les man page de chaque fonction que tu utilise !!! et ne code que lorsque tu es sûr d'avoir compris PARAMETERS et RETURN VALUES.
*:ensuite itemLen est mal utilisé vu que tu ne comptes pas le fait que tu avance le pointeur, pour rester cohérent, tu devrais réduire itemLen. Néanmoins, cela changera rien au fait que getline ne peut pas faire de realloc dessus -> tjrs un bug.
[^] # Re: Je suppose...
Posté par benja . Évalué à 1.
'fin à ce momemt là (itemLen=0) ce n'est pas un bug vu que items+i = items, ensuite cela le devient...
Suivre le flux des commentaires
Note : les commentaires appartiennent à celles et ceux qui les ont postés. Nous n’en sommes pas responsables.