Discussion :

[YukiShao]

Bonjour à tous,

Je ne suis pas du genre à poster des messages demandant de l'aide, mais ce problème me rend fou et j'ai rien trouvé d'interessant à propos de ça sur le net, alors je me tourne vers vous.
Voici le problème :

Dans une fonction, j'appelle mon strlen modifié qui compte les caractères d'un pointeur sur char jusqu'au \0.
Seulement, lorsque je printf le return de ce strlen modifié, il affiche la mauvaise valeur.
Exemple, je demande de compter combien il y a de caractères dans une chaîne qui fait 150 caractères ... la fonction me renverra une valeur négative, par exemple.
La chose curieuse, c'est que ce strlen modif marche très bien pour les valeurs < 100, mais dès que ça commence à être un peu plus gros il déconne.
Chose encore plus étonnante, si je printf la valeur avant de return, il a bien la valeur escomptée, même si la chaîne est supérieure à 100 caractères.

Pour résumer et illuster, je vous fait un petit code :

Code :

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

1
2
3
4
5
6
7
8
 
int    main(void)
{
   char *str = "chaine de 130 caractères";
   // dans l'exemple, c'est fait à l'arrache, mais cette chaîne est allouée et terminée par un \0 dans mon programme
   printf("%d\n", strlen_mod(str));
   return (0);
}

L'output correspondrait à -88, ou une valeur érronée, peu importe.

Cependant :

Code :

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

1
2
3
4
5
6
7
8
9
 
int   strlen_mod(char *str)
{
int   i = 0;
while (str[i] != 0)
  i++;
printf("%d\n", i);
return (i);
}

Ce printf en revanche me donne la bonne valeur, il a bien compté jusqu'au bout...

Des idées ? Je peux contourner en envoyant un pointeur sur int à la place de ce "i", et ça marche très bien, mais bon c'est chiant de pas pouvoir utiliser les valeurs de retour quoi.

Merci d'avance. :3

[Obsidian]

Bonjour et bienvenue,

En effet, c'est curieux. Ne voyant rien de flagrant dans ton exemple, je l'ai compilé et allongé la chaîne. J'obtiens de bons résultats dans tous les cas. Il doit donc s'agir d'un effet de bord.

Ton programme est-il multi-threadé ? Généralement, ce genre de problème apparaît soit lorsque la chaîne n'est pas correctement terminée et que le curseur part s'enfoncer dans les tréfonds de la mémoire, soit lorsqu'il se fait écraser par un dépassement de tableau ou un écrasement de pile. Comme rien de tout cela ne peut se produire dans l'exemple concerné, peux-tu nous présenter le code exact et, si possible, en entier ?

D'autre part, tu compiles sur quelle machine, avec quel système d'exploitation et quelle version de ton compilateur ?

À part cela, sur la forme uniquement, les parenthèses après return sont inutiles. « return » n'est pas une fonction.

[ternel]

Ton code d'exemple est valide.

Par contre, si tu stocke le retour dans un char, ca peut se comprendre.

Par ailleurs, ta fonction devrait prendre un char const * comme argument, permettant ainsi de l'appeler directement avec une chaine littérale.

[YukiShao]

Re,

Pour répondre au dernier message, je ne stocke pas dans un char, mais bien dans un int. (Pour pouvoir tester le retour, je printf("%d\n", strlen_mod(str));)

Étant dans une école où les règles sont un peu spéciales (pour ne rien citer), vous envoyer le code serait long, et fastidieux pour vous à examiner (d'où les parenthèses sur le return etc etc).
Mon programme n'est pas multithreadé, il s'agit d'un recode d'un shell simple avec les fonctions de base et quelques builtins.

Je vais donc check toutes mes chaînes, voir si je met bien le \0 sur toutes, et je reviens vers vous si jamais ça plante encore. (dans l'immédiat je ne peux pas mais je vous tiens au courant le plus rapidement possible)

Par ailleurs, je suis d'habitude sous OpenSUSE Dartmouth 12.3, mais je me suis installé une distrib Ubuntu Gnome à côté et c'est sur celle ci que je code. Elle est évidemment en LTS.
Sinon, pour compiler, je fais ça "à la main" entre guillemets (emacs comme éditeur, gcc comme compilo, avec un makefile qui va bien, valgrind pour les leaks).

Merci à vous !

Edit : Fait curieux, un ami m'a fait remarquer qu'avec des chaînes de 127 caractères, ça marchait, mais pas 128. Pourtant, je ne store pas cette valeur dans un char, j'en suis sûr à 100%. (Quand je teste avec 128 et que je printf("%d => témoin\n", my_strlen(str)); l'output me donne : "-128 => témoin")

[Médinoc]

Ça ressemble pourtant vraiment à un retour en char, ça...

[Médinoc]

Voici un code minimal et compilable, qui marche chez moi (code::blocks 12.11):

Code C :

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
/*
http://www.developpez.net/forums/d1469035/c-cpp/c/debuter/mauvais-return/#post7961192
*/
#include <stdio.h>
 
int strlen_mod(char const *str)
{
	int i = 0;
	while(str[i] != 0)
		i++;
	printf("%d\n", i);
	return (i);
}
 
int main(void)
{
	char const *str = "chaine de 130 caractères5678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890";
	// dans l'exemple, c'est fait à l'arrache, mais cette chaîne est allouée et terminée par un \0 dans mon programme
	printf("%d\n", strlen_mod(str));
	return (0);
}

Que fait ce code chez toi?

[ternel]

"127+1=-128" pour moi, c'est char (signed)
bingo ... perdu!

essaie "%ld"
Ton makefile contient-il les options de compilation -Wall -Wextra?
Si ce n'est pas le cas, ajoute les.

Dans les deux cas, considère chaque warning comme une faute probable.
un warning signifie "si tu as de la chance, ca peut marcher"

[YukiShao]

... Et pourtant, le code que je vous ai linké est similaire à 99% du mien qui bug, j'alloue juste à coup de malloc ma chaîne et je la termine par un \0.

Le code compilé et exécuté que tu m'as donné marche très bien, aucun soucis. Je vais check une énième fois ma chaîne, que je mette le \0 un cran trop loin ou je ne sais quoi.

En tout cas, merci à tous.

Edit : Makefile avec -g3, -Wall, Wextra et même Werror, histoire que je passe à côté de rien, et ça depuis le début.
Si je teste avec %ld, ça me donne une valeur énorme. 4 294 967 178 pour être précis.

[ternel]

C'est que tu poses une question, et montre autre chose.
Ton code ne contenait pas de malloc...

%ld affiche un long, visiblement ta valeur est tordue.

[YukiShao]

Effectivement, ce n'est pas à la lettre près le code que j'ai chez moi, mais je doute qu'éplucher 25 ".c" soit jouissif pour vous, j'ai donc essayé d'être le plus clair possible. :/

[Médinoc]

Si je teste avec %ld, ça me donne une valeur énorme. 4 294 967 178 pour être précis.

C'est-à-dire, -118 sur 32 bits.
PS: Es-tu sur une plate-forme 64 bits LP64? Autrement, c'est bizarre que %ld te l'affiche comme un int non-signé...

[YukiShao]

Re,

Ui, je suis sur une plateforme 64bits. Par contre, LP64, je ne sais pas ce que ça signifie. J'ai cru vaguement comprendre que sur ce genre de plateforme LP64, les longs et les pointeurs valent 64bits.

[Médinoc]

C'est ça. Les deux modèles 64-bits les plus répandus sont:

• LP64 ("long and pointer 64-bit", systèmes unixoïdes, sizeof(long)==8)
• LLP64 ("long long and pointer 64-bit", Windows, sizeof(long)==4)

Chez toi, les long sont donc sur 64 bits, ce qui explique l'affichage de 4 milliards en %ld (mais n'explique par la négativité du retour).

[YukiShao]

D'accord, c'est toujours bon à savoir !

Je vais essayer de passer un coup de gdb pour voir ce que ça donne, même si je suis un piètre utilisateur de ce debug tool.

Edit : Il se pourrait que j'écrase le bit de poid fort de mon int, ce qui provoquerait ce "-" sorti de nulle part, d'après ce que mon ami m'a dit (effet de bord, comme quelqu'un l'a suggéré plus haut).

Je considère donc ce problème comme résolu.

Merci à tous de votre aide et de votre sympathie.

[Obsidian]

Effectivement, ce n'est pas à la lettre près le code que j'ai chez moi, mais je doute qu'éplucher 25 ".c" soit jouissif pour vous, j'ai donc essayé d'être le plus clair possible. :/

C'est effectivement une bonne intention mais pourrait-on au moins avoir le fichier *.c qui contient la fonction en question et celui qui l'appelle, quitte à les mettre en pièce jointe ?

[YukiShao]

Re,

Désolé du contretemps, j'étais occupé.
Si vous y tenez, je me mets de ce pas à commenter le code, histoire que vous compreniez parce que c'est un beau fouillis.

get_next_line.c

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
#include "my.h"
 
char		*replace(char *finale, int cmp, char *buffer, int *i)
{
  char		*replace;
  int		j;
 
  //seulement, ça ne peut pas marcher, puisque my_strlen() fait des siennes.
  if (finale != 0)
    j = my_strlen(finale);
  else
    j = 0;
  replace = malloc((j + cmp + 1) * sizeof(char*));
  if (finale != NULL)
    my_strncpy(replace, finale, j);
  else
    my_strncpy(replace, "", j);
  my_strncpy((replace + j), (buffer + *i), cmp);
  replace[j + cmp] = 0;
  free(finale);
  *i = (*i + cmp + 1);
  return (replace);
}
 
char		*get_next_line(const int fd)
{
  static char	buffer[SIZE_BUFFER + 1];
  static int	read_buff = 0;
  static int	i;
  char		*finale;
  int		cmp;
 
  //ici, vous l'aurez compris, on récupère tout ce qui a été tapé sur l'entrée standard.
  finale = 0;
  cmp = 0;
  while (1)
    {
      if (i >= read_buff)
	{
	  i = 0;
	  if (!(read_buff = read(fd, buffer, SIZE_BUFFER)))
	    return (finale);
	  cmp = 0;
	}
      if (buffer[i + cmp] == '\n')
	return (finale = replace(finale, cmp, buffer, &i));
      if (i + cmp == (read_buff - 1))
	finale = replace(finale, (cmp + 1), buffer, &i);
      cmp = cmp + 1;
    }
}

main.c

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
#include "my.h"
 
/* int		check_builts(t_chars *chain)
{
  if (my_strncmp(chain->cmd, "cd", 2) != 0
      && my_strncmp(chain->cmd, "setenv", 6) != 0
      && my_strncmp(chain->cmd, "unsetenv", 8) != 0
      && my_strncmp(chain->cmd, "exit", 4) != 0
      && my_strncmp(chain->cmd, "env", 3) != 0)
    return (0);
  else
    return (1);
}
*/
void		my_settings(t_chars *chain)
{
  chain->buff = NULL;
  chain->cmd = NULL;
  chain->opt = NULL;
  chain->remain = NULL;
  my_putstr("$> ");
  chain->buff = get_next_line(0);
  // ici, on store dans chain->buff tout ce qui a été tapé.
  rm_useless_spaces(chain->buff);
}
 
int		main(int argc, char **argv, char **env)
{
  t_chars	chain;
 
  if (cpy_env_to_chain(&chain, env, 0) == -1)
    return (0);
  // On commence par copier l'environnement pour pouvoir faire des setenv/unsetenv tranquillement.
  while (42)
    {
      my_settings(&chain);
      // Ici, on set les paramètres de base
      // à savoir, chain->buff qui va contenir tout ce qui a été tapé, chain->remain les options à passer à execve();
      // chain->opt est un char **, chaque ligne contiendra une option, tirées de chain->remain.
      if (sep_cmd_from_remain(&chain) == 0 || sep_options(&chain) == 0 ||
	  (chain.opt = add_cmdname(&chain)) == NULL)
	return (0);
      if (check_builts(&chain) == 1)
	{
	  if (launch_builts(&chain) == -1)
	    {
	      free_dchar(chain.env);
	      free_chain(&chain);
	      return (0);
	    }
	}
      else if (chain.cmd[0] != 0)
	{
	  my_paths(&chain);
	  free_chain(&chain);
	}
    }
  return (0);
}

Edit : Aussitôt dit, aussitôt fait. Bon courage pour comprendre ça. :s
NB : SIZE_BUFFER = 150 dans mon cas. :3
Edit2: Je n'ai pas trouvé nécessaire de linker mon strlen, vu qu'il est là haut. Et si vous voulez plus de renseignements sur le code (ce que je doute, vous êtes certainement bien plus qualifiés que moi), je reste à dispo.

[ternel]

Il y a plusieurs changements à apporter, plus ou moins conséquents:
ajouter systématiquement des accolades quand tu utilises un else.
envisager l'usage du ternaire ?:

par exemple, dans replace():

Code :

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

1
2
3
4
	if (finale != 0)
		j = my_strlen(finale);
	else
		j = 0;

devient avantageusement

Code :

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

1
2
3
4
5
	if (finale != 0) {
		j = my_strlen(finale);
	} else {
		j = 0;
	}

voire mieux:

Code :

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

	j = (finale != 0) ? my_strlen(finale) :  0;

my_strlen() pourrait peut être retourner 0 si son argument est NULL, tu aurais alors tout simplement j = my_strlen(finale);.
En fait, je crois que c'est déjà le cas.

Encore une fois, envisage aussi d'utiliser plus de const sur tes pointeurs.
Je suis persuadé que ton problème peut venir de là.
Au pire, ce n'est pas le cas, et ton code sera quand meme plus sûr.
En l'état, tu ne pourrais pas écrire my_strlen("bidule"); sans avoir un warning assez grave.

Pour la petite histoire, const a couté plusieurs milliers d'euros à un de mes précédents employeurs.

D'une manière générale, plus le code sera limpide, plus il sera facile de le corriger.

[Sve@r]

Si vous y tenez, je me mets de ce pas à commenter le code, histoire que vous compreniez parce que c'est un beau fouillis.

Bonjour

J'ai eu le courage d'examiner ce code. C'est vrai qu'il n'est pas clair !!!

Ce que j'ai de suite remarqué, c'est que dans la fonction char *replace(...), tu as char *replace=malloc((j + cmp + 1) * sizeof(char*)). Bon c'est pas terrible mais un char* étant plus large qu'un char, je ne pense pas que ce soit ça qui fasse merder le truc. Toutefois tu fais beaucoup d'appels à my_strncpy() que je n'ai vu nulle part (ni dans le code, ni dans ce topic). Attention aussi, parfois tu compares finale avec 0 et parfois avec NULL. Pour un pointeur, c'est tout le temps NULL.
Ceci dit, ta fonction replace() me semble un poil compliquée. A ce que je comprends, tu l'utilise pour remplacer une chaine allouée (passée en paramètre) par une autre nouvellement allouée (la première est libérée tandis que la seconde est renvoyée à l'appelant). Bref c'est une espèce de realloc et c'est là que je me dis que peut-être realloc() aurait pu être utilisé. De plus il y a un lien étroit entre la chaine reçue et la variable "i" qui est modifiée en parallèle dans la fonction. Peut-être alors que tu pourrais regrouper ta chaine et cette variable dans une structure dédiée passée à la fonction...

Sinon j'ai vu autre chose qui m'embête: au premier appel, tu as finale qui vaut 0, tu as cmp qui vaut 0 et tu as j qui vaut 0. Tu fais donc un replace=malloc de (j + cmp + 1) soit 1 (char* mais ce devrait être un char). Ensuite tu fais un my_strncpy(replace, "", j) (que je trouve inutile vu que tout est à 0) puis un my_strncpy((replace+j), (buffer+*i), cmp) (avec des parenthèses inutiles). Mais ici replace n'est allouée que de 1. Ne peut-il pas y avoir débordement ?

Sinon quoi qu'il en soit, mets ton strlen_mod() en unsigned long et affiche la valeur renvoyée au format %lu. Ca m'énerve de voir apparaitre des valeurs négatives et peut-être que si tu forces à rester dans la plage des positifs on/tu comprendras mieux ce qui merde...

PS: while(42) ??? Oui pourquoi pas mais while(1) est quand-même plus conventionnel non ?
PS2: il manque "my.h"

[YukiShao]

Bonjour à tous,

Tout d'abord merci des réponses constructives, j'en apprends de jour en jour !
@leternel : Tu as tout à fait raison, je peux utiliser un ternaire ici, mais je ne suis pas trop familier avec eux, donc je n'y ai pas pensé sur le moment. Je règle ça de ce pas.
N'étant pas familier non plus des consts (voir plus bas pour explications), je n'en utilise pas sauf lorsque ça m'est demandé. J'ai cru comprendre (et comme son nom l'indique) que cela servait à créer une "variable non variable" en quelques sortes, quelque chose que l'on ne peut modifier..

@Sve@r : J'avais prévenu haha (mais merci à toi). Pour ce qui est du sizeof(char*), là, c'est totalement de ma faute et je me demande comment je ne l'ai pas vu avant.. C'est grave là.
Pour le NULL, je pensais que NULL était une macro pour 0.. Dans ce cas là, ça ne me dérangeait pas de l'utiliser (le 0) pour des pointeurs sur char ... :s
Pour le replace();, tu as tout à fait raison, il s'agit d'un réalloc modifié (mon école ne m'autorise que très peu d'appels systèmes et nous force à recoder pas mal de choses, sinon j'aurais utilisé fgets(), ou autre, pour récupérer un input sur l'entrée standard).
Je n'avais pas réellement fait attention à cette histoire d'allocation de 1, je me focalisais surtout sur le strlen. Je me penche dessus desuite ! (ceci dit, les parenthèses inutiles m'aidait à lire).

Pour le PS N°1, oui, j'aurais pu tout à fait mettre while (1), mais bon, vous connaissez surement l'ouvrage "Le guide du voyageur galactique" ..
Pour le PS N°2, je n'ai pas jugé nécéssaire de le linker vu qu'il ne contient que des includes systèmes, mon define de SIZE_BUFFER ainsi que le prototype des fonctions (et aussi le typedef de ma structure).

Merci à vous, je corrige ça au plus vite (ainsi que pour le %lu).

Edit : Je sais pourquoi je ne faisais pas attention à cette allocation de 1 ... Parce que je n'alloue jamais de 1. Tant que le \n n'est pas rencontré, cmp continue d'être itéré, donc cmp augmente jusqu'à un maximum de SIZE_BUFFER et ensuite, replace() est appelé.
Pour le my_strncpy, c'est un strncpy modifié avec exactement le même comportement que celui du système, et je l'ai testé sous toutes ses formes.

Edit 2 : En transformant ce strlen modifié pour renvoyer un unsigned long, la valeur affichée pour une chaîne de 149 caractères est : 18 446 744 073 709 551 509. Véridique. (Mais marche très bien jusqu'à 127 caractères);

[Obsidian]

Re,

Désolé du contretemps, j'étais occupé.
Si vous y tenez, je me mets de ce pas à commenter le code, histoire que vous compreniez parce que c'est un beau fouillis.

Edit : Aussitôt dit, aussitôt fait. Bon courage pour comprendre ça. :s
NB : SIZE_BUFFER = 150 dans mon cas. :3
Edit2: Je n'ai pas trouvé nécéssaire de linker mon strlen, vu qu'il est là haut. Et si vous voulez plus de renseignements sur le code (ce que je doute, vous êtes certainement bien plus qualifiés que moi), je reste à dispo.

Merci beaucoup pour ces documents mais la fonction strlen() que tu utilises est-elle rigoureusement celle que tu nous as présentée dans tes premiers posts ?
D'autre part, quelle est la version exacte du compilateur que tu utilises ?

Autre piste intéressante : as-tu pensé à faire un « make clean » ou ce qui en tient lieu quand tu as rencontré ce bug ? Il arrive parfois que certains programmes soient compilés avec la mauvaise version de leurs fichiers objets, justement parce qu'une dépendance a été mal spécifiée et que la recompilation ne s'est pas faite. Autre cas similaire : la liaison avec une bibliothèque dynamique différente mais à la même interface. Par exemple, je travaillais avec l'OpenClient de Sybase et des applications que j'avais écrites et compilées plusieurs mois auparavant se sont mises brutalement à segfaulter dans tous les sens sans explication apparente et sans recompilation préalable. En fait, j'avais installé le package FreeTDS pour l'évaluer et j'ai mis un temps avant de me rendre compte que l'interface était identique. Le système, à l'exécution, liait donc silencieusement mon programme à ces nouvelles bibliothèques, et sans erreur.

Il y a plusieurs changements à apporter, plus ou moins conséquents:
ajouter systématiquement des accolades quand tu utilises un else.

par exemple, dans replace():

Code :

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

1
2
3
4
	if (finale != 0)
		j = my_strlen(finale);
	else
		j = 0;

devient avantageusement

Code :

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

1
2
3
4
5
	if (finale != 0) {
		j = my_strlen(finale);
	} else {
		j = 0;
	}

Ça, en revanche, je suis personnellement contre. Instaurer des règles de codage pour conserver une cohérence, c'est généralement une bonne idée mais il faut encore qu'elles soient justifiées.

Les accolades ont une signification sémantique. La grammaire du C définit le « if » comme suit : « if ( expression ) statement ». C'est la déclaration elle-même (statement) qui peut prendre plusieurs forme, et notamment celle de « déclaration composée » (compound statement). Ça veut dire qu'à ce stade, les accolades servent à faire exactement ce à quoi elles servent en français et en mathématiques : insérer plusieurs entités groupées là où on en attend en principe qu'une seule. C'est aussi pour cela que les accolades peuvent ouvrir un cadre de pile, ce qui permet entre autre d'y déclarer de nouvelles variables locales: il s'agit de sauvegarder un contexte qui va être modifié par les appels successifs que le bloc contient. Du coup, ouvrir une accolade pour n'y insérer qu'un seul élément n'a pas beaucoup de sens, sauf à vouloir explicitement définir un singleton, puisque par défaut le C attend une déclaration unique.

Chez les développeurs du noyau Linux, par exemple, on impose dans le cas du if-else de mettre soit les accolades aux deux conditions (« if (…) {…} else {…} »), soit de ne pas en mettre du tout, mais en aucun cas il est imposé de systématiquement les ajouter, surtout sans raison valable.