Discussion :

[YannisBoukhalfa]

Bonjour,

je suis stagiaire et je travaille à la mise en place d'un genre de client/serveur entre deux ordinateur, ceux-ci relié par des MODEM Acoustiques. Mon code fonctionne mais étant mauvais en programmation, je souhaitais quelques conseils pour pouvoir l'améliorer. Merci beaucoup pour votre aide.

Voici mon code :

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
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
 
 
 
#include <WinSock2.h>
#include <Windows.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>
 
 
#pragma comment(lib, "ws2_32.lib")
 
#define SEQ "+++"
#define SIZE 250
 
void my_delay(int i);
void delay_s(int i);
void empty_buff(char * b);
 
 
int main()
{
 
	// Declaration des variables 
 
	WSADATA wsa;
	SOCKET sock;
	SOCKADDR_IN addr_in;
 
	char buff_recv[SIZE];
	char buff_send[SIZE];
	char buff_affichage[SIZE];
	char buff_passage_cmd[4];
 
	char nomFichier[30];
	char* nomFichierAEnvoyer = "fichier a envoyer";
	FILE* fichier = NULL;
 
	//variables d'etat des sockets
	u_long bloquant = 0; 
	u_long non_bloquant = 1;
 
	//variable de gestion du programme
	char boucle = 0;
	char choix[3];
 
	char decision_r = 0;
	char decision_e = 0;
	char decision_q = 0;
	char decision_c = 0;
	char decision_fs = 0;
	char decision_fr = 0;
 
	char cmd_act = 0;
	int n = 0;
 
	//gestion des retours de fonctions
	int connection = 1;
	int octets_recus = 1;
	int octets_envoyes = 1;
 
	int total_octets_recus = 0;
	int error = 1;
 
	// Vidage des buffers d'envoi et de réception
	empty_buff(buff_recv);
	empty_buff(buff_send);
 
	// Initialisation de la socket 
	WSAStartup(MAKEWORD(2,0), &wsa);
	sock = socket(AF_INET, SOCK_STREAM, 0);
	addr_in.sin_addr.s_addr = inet_addr("192.168.0.144");
	addr_in.sin_family = AF_INET;
	addr_in.sin_port = htons(9200);
 
	//création et remplissage du fichier à envoyer
	/*do{
	printf("Entrer le nom du fichier a creer/remplir, preciser le format\n");
	gets(nomFichier);
	fichier = fopen(nomFichier, "w+");
	if(fichier != NULL)
	{
		fputs("Debut du fichier de O\n ooooooooooooooooooooooooooooooooooo\n!!!!\n", fichier);
		fclose(fichier);
	}
	else printf("impossible d'ouvrir le fichier %s", nomFichier);
	}while(fichier == NULL);
	*/
	// Connection au serveur 
	do
	{
		connection = connect(sock, (SOCKADDR*) &addr_in, sizeof(addr_in));// On se connecte à notre serveur (dans notre cas, le MODEM Acoustique)	
	}while(connection != 0);
 
	if(!connection)
		{
			printf("Connection etablie\n");
		}
 
	// Mode Socket non bloquante
	ioctlsocket(sock,FIONBIO,&non_bloquant);
 
	// Boucle de communication 
	while(!boucle)
	{
		printf("Choix du traitement : \nR : Reception de data\nE : Envoi de data\nC : Envoi de commande\n1 : Reception de fichier\n2 : Envoi de fichier\nQ : Quitter\n");
 
		printf("decision : ");
		n = getchar();
		sprintf(choix,"%d", n);
		decision_r = (strcmp("82",choix)==0);
		decision_e = (strcmp("69",choix)==0);
		decision_q = (strcmp("81",choix)==0);
		decision_c = (strcmp("67",choix)==0);
		decision_fs = (strcmp("50",choix)==0);
		decision_fr = (strcmp("49",choix)==0);
 
		fflush(stdin);
		if(decision_r)
		{
			empty_buff(buff_recv);
			do{
				octets_recus = recv(sock, buff_recv, sizeof(buff_recv), 0);
				if(octets_recus > 0) 
				{
					printf("%d octets recus : %s\n", octets_recus, buff_recv);
					total_octets_recus += octets_recus;
					//printf("Affichage de notre buffer de reception : %s \n", buff_recv);
 
				}else error = WSAGetLastError();
			}while(error != 10035);
 
			empty_buff(buff_recv);
			total_octets_recus = 0;
 
		}else if(decision_e)
		{
			empty_buff(buff_send);
 
			//scanf("%SIZE2s", buff_send);
			fgets(buff_send,SIZE, stdin);
			send(sock, buff_send, sizeof(buff_send),0);				// Fonction d'envoi de chaine de caractère à notre serveur
			printf("%s", buff_send);				
			empty_buff(buff_send);
 
		}else if (decision_q)
		{
			shutdown(sock, SD_BOTH); 
			closesocket(sock);
			WSACleanup();
			boucle = 1;			
 
		}else if (decision_c)
		{
			do
			{
				empty_buff(buff_passage_cmd);
				send(sock, "+++ATC\n", sizeof("+++ATC\n"), 0);
				error = recv(sock, buff_passage_cmd, sizeof(buff_passage_cmd),0);
				if ( error > 0 )
					printf("Octets recus: %d\n", error);
 
				printf("%s\n", buff_passage_cmd);
 
				if(strncmp(buff_passage_cmd, "OK\r\n",4) != 0)
				{
					printf("Echec passage en mode commande\n");
				}else 
				{
					printf("Mode commande active\n");
					cmd_act = 1;
				}
				my_delay(1);
 
			}while(!cmd_act);
 
			while(cmd_act)
			{
				printf("Entrez votre commande : \n");
				scanf("%s",buff_send);
				strcat(buff_send,"\n");
				send(sock,buff_send,sizeof(buff_send),0);
				if(strcmp("ATO\n",buff_send) == 0)
				{
					printf("Mode Data Activé\n");
					cmd_act = 0;			
				}else 
				{
					do
					{
						error = recv(sock, buff_recv, sizeof(buff_recv),0);
 
						if ( error > 0 )
						{
							printf("Octets recus: %d\n", error);
							printf("%s\n", buff_recv);
						}else {
							error = WSAGetLastError();
						}
 
					}while(error != 10035);
				}					
			}
		}else if(decision_fs)
		{
			fichier = fopen("nomFichierAEnvoyer.txt", "r+");
			empty_buff(buff_send);
			if(fichier != NULL)
			{
				while(fgets(buff_send, SIZE, fichier) != NULL)
				{
					printf("%s\n", buff_send);
					send(sock,buff_send,sizeof(buff_send),0);
					empty_buff(buff_send);
				}
				fclose(fichier);
			}else printf("Erreur dans l'ouverture du fichier %s", nomFichier);
 
 
 
		}else if(decision_fr)
		{
			fichier = fopen("fichier recu.txt", "w+");
			if(fichier != NULL)
			{
				empty_buff(buff_recv);
				do{
					octets_recus = recv(sock, buff_recv, sizeof(buff_recv), 0);
					if(octets_recus > 0) 
					{
						printf("%d octets recus : %s\n", octets_recus, buff_recv);
						total_octets_recus += octets_recus;
 
					}else error = WSAGetLastError();
				}while(error != 10035);
				fputs(buff_recv, fichier);
				empty_buff(buff_recv);
				fclose(fichier);
			}else printf("Erreur dans l'ouverture du fichier ""fichier recu""");
 
		}
 
 
 
 
 
 
	//empty_buff(choix);
	fflush(stdin);
 
 
	}
 
	// Mise en pause du système pour pouvoir lire ce qu'il y a dans la console 
	system("pause");
 
 
	// Fin du programme 
	return 0 ;
}
 
 
 
void delay_s(int i)    // Pause l'application pour i seconds (non blocant)
{
    i*=1000;
	Sleep(i);
}
 
void my_delay(int i)    // Pause l'application pour i seconds
{
    clock_t start,end;
    start=clock();
    while(((end=clock())-start)<=i*CLOCKS_PER_SEC);
}
 
void empty_buff(char * b)
{
	memset(b,0,sizeof(b));
}

[Médinoc]

Pour commencer, remplace ta prise de décision par une conversion char* -> int (avec la fonction strtol()) et un switch.

[ternel]

Code :

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

1
2
3
4
void empty_buff(char * b)
{
	memset(b,0,sizeof(b));
}

Cette fonction est fausse.
En effet, sizeof(b) est sizeof(char*).
C'est la taille d'un pointeur, qui n'a rien à voir avec le tableau que tu pourrais vouloir réinitialiser.
C'est bien pour cela que memset prend une taille en argument.

[Bktero]

Code :

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

fflush(stdin);

Ce n'est conseillé de faire ça : http://c.developpez.com/faq/?page=Ge...buffer-clavier

Code :

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

system("pause");

Ne marche pas si le système de la commande "pause" (et ça arrive plus souvent qu'on ne le pense). Utilise plutôt getchar().

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
void delay_s(int i)    // Pause l'application pour i seconds (non blocant)
{
    i*=1000;
	Sleep(i);
}
 
void my_delay(int i)    // Pause l'application pour i seconds
{
    clock_t start,end;
    start=clock();
    while(((end=clock())-start)<=i*CLOCKS_PER_SEC);
}

Sleep() est bloquante (arrrg ! la faute de français !)donc delay_s le sera aussi. Pourquoi faire 2 fonctions qui font la même chose ? Pourquoi ne pas mettre des noms plus explicites comme delay_second(int seconds) ?

[YannisBoukhalfa]

Merci pour vos réponses, les changements apportés ont allégés mon code (beaucoup de variables ne sont plus utilisées). Pour "memset", j'ai regardé le prototype et j'ai compris mon erreur. Voici mon code après les modifications :

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
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
 
 
#include <WinSock2.h>
#include <Windows.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>
 
 
#pragma comment(lib, "ws2_32.lib")
 
#define SEQ "+++"
#define SIZE 250
 
void empty_buff(char * b);
void delay_seconds(int seconds);
void clean_stdin();
 
int main()
{
 
	// Declaration des variables 
 
	WSADATA wsa;
	SOCKET sock;
	SOCKADDR_IN addr_in;
 
	char buff_recv[SIZE];
	char buff_send[SIZE];
 
	char buff_passage_cmd[4];
 
	char nomFichier[30];
	char* nomFichierAEnvoyer = "fichier a envoyer";
	FILE* fichier = NULL;
 
	//variables d'etat des sockets
	u_long bloquant = 0; 
	u_long non_bloquant = 1;
 
	//variable de gestion du programme
	char boucle = 0;
 
	int decision = 0;
 
	char cmd_act = 0;
	int n = 0;
 
	//gestion des retours de fonctions
	int connection = 1;
	int octets_recus = 1;
	int octets_envoyes = 1;
 
	int total_octets_recus = 0;
	int error = 1;
 
	// Vidage des buffers d'envoi et de réception
	empty_buff(buff_recv);
	empty_buff(buff_send);
 
	// Initialisation de la socket 
	WSAStartup(MAKEWORD(2,0), &wsa);
	sock = socket(AF_INET, SOCK_STREAM, 0);
	addr_in.sin_addr.s_addr = inet_addr("192.168.0.144");
	addr_in.sin_family = AF_INET;
	addr_in.sin_port = htons(9200);
 
	//création et remplissage du fichier à envoyer
	do{
	printf("Entrer le nom du fichier a creer/remplir, preciser le format\n");
	gets(nomFichier);
	fichier = fopen(nomFichier, "w+");
	if(fichier != NULL)
	{
		fputs("Debut du fichier de O\n ooooooooooooooooooooooooooooooooooo\n!!!!\n", fichier);
		fclose(fichier);
	}
	else printf("impossible d'ouvrir le fichier %s", nomFichier);
	}while(fichier == NULL);
 
	// Connection au serveur 
	do
	{
		connection = connect(sock, (SOCKADDR*) &addr_in, sizeof(addr_in));// On se connecte à notre serveur (dans notre cas, le MODEM Acoustique)	
	}while(connection != 0);
 
	if(!connection)
		{
			printf("Connection etablie\n");
		}
 
	// Mode Socket non bloquante
	ioctlsocket(sock,FIONBIO,&non_bloquant);
 
	// Boucle de communication 
	while(!boucle)
	{
		printf("Choix du traitement : \nR : Reception de data\nE : Envoi de data\nC : Envoi de commande\n1 : Reception de fichier\n2 : Envoi de fichier\nQ : Quitter\n");
 
		printf("decision : ");
		decision = getchar();
 
		clean_stdin();
		switch(decision)
		{
		case 82: 
			empty_buff(buff_recv);
			do{
				octets_recus = recv(sock, buff_recv, sizeof(buff_recv), 0);
				if(octets_recus > 0) 
				{
					printf("%d octets recus : %s\n", octets_recus, buff_recv);
					total_octets_recus += octets_recus;
					//printf("Affichage de notre buffer de reception : %s \n", buff_recv);
 
				}else error = WSAGetLastError();
			}while(error != 10035);
 
			empty_buff(buff_recv);
			total_octets_recus = 0;
		break;
		case 69:
 
			empty_buff(buff_send);
 
			//scanf("%SIZE2s", buff_send);
			fgets(buff_send,SIZE, stdin);
			send(sock, buff_send, sizeof(buff_send),0);				// Fonction d'envoi de chaine de caractère à notre serveur
			printf("%s", buff_send);				
			empty_buff(buff_send);
		break;
		case 81:
 
			shutdown(sock, SD_BOTH); 
			closesocket(sock);
			WSACleanup();
			boucle = 1;			
		break;
		case 67
			do
			{
				empty_buff(buff_passage_cmd);
				send(sock, "+++ATC\n", sizeof("+++ATC\n"), 0);
				error = recv(sock, buff_passage_cmd, sizeof(buff_passage_cmd),0);
				if ( error > 0 )
					printf("Octets recus: %d\n", error);
 
				printf("%s\n", buff_passage_cmd);
 
				if(strncmp(buff_passage_cmd, "OK\r\n",4) != 0)
				{
					printf("Echec passage en mode commande\n");
				}else 
				{
					printf("Mode commande active\n");
					cmd_act = 1;
				}
				delay_seconds(1);
 
			}while(!cmd_act);
 
			while(cmd_act)
			{
				printf("Entrez votre commande : \n");
				scanf("%s",buff_send);
				strcat(buff_send,"\n");
				send(sock,buff_send,sizeof(buff_send),0);
				if(strcmp("ATO\n",buff_send) == 0)
				{
					printf("Mode Data Activé\n");
					cmd_act = 0;			
				}else 
				{
					do
					{
						error = recv(sock, buff_recv, sizeof(buff_recv),0);
 
						if ( error > 0 )
						{
							printf("Octets recus: %d\n", error);
							printf("%s\n", buff_recv);
						}else {
							//printf("Erreur de reception, code : %d\n", WSAGetLastError());
							error = WSAGetLastError();
						}
 
					}while(error != 10035);
				}					
			}
		break;
		case 49 :
 
			fichier = fopen("nomFichierAEnvoyer.txt", "r+");
			empty_buff(buff_send);
			if(fichier != NULL)
			{
				while(fgets(buff_send, SIZE, fichier) != NULL)
				{
					printf("%s\n", buff_send);
					send(sock,buff_send,sizeof(buff_send),0);
					empty_buff(buff_send);
				}
				fclose(fichier);
			}else printf("Erreur dans l'ouverture du fichier %s", nomFichier);
 
 
 
		break;
		case 50 :
 
			fichier = fopen("fichier recu.txt", "w+");
			if(fichier != NULL)
			{
				empty_buff(buff_recv);
				do{
					octets_recus = recv(sock, buff_recv, sizeof(buff_recv), 0);
					if(octets_recus > 0) 
					{
						printf("%d octets recus : %s\n", octets_recus, buff_recv);
						total_octets_recus += octets_recus;
						//printf("Affichage de notre buffer de reception : %s \n", buff_recv);
 
					}else error = WSAGetLastError();
				}while(error != 10035);
				fputs(buff_recv, fichier);
				empty_buff(buff_recv);
				fclose(fichier);
			}else printf("Erreur dans l'ouverture du fichier ""fichier recu""");
		break;		
		}
	clean_stdin();
	}
 
	// Mise en pause du système pour pouvoir lire ce qu'il y a dans la console 
	getchar();;
 
 
	// Fin du programme 
	return 0 ;
}
 
void delay_seconds(int seconds)    // Pause l'application pour i seconds
{
    clock_t start,end;
    start=clock();
    while(((end=clock())-start)<=seconds*CLOCKS_PER_SEC);
}
 
void empty_buff(char * b)
{
	memset(b,0,SIZE);
}
 
void clean_stdin()
{
	int c;
	do
	{
		c = getchar();
	}while(c != '\n' && c != EOF);
}

[Sve@r]

Bonjour

Ton code n'est pas trop mal écrit. Bien entendu tu manques d'habitude et ça se sent mais c'est prometteur.

Quelques remarques

• évite les nombres magiques => while(error != 10035). Pourquoi 10035 ? Que signifie ce nombre ? Certainement qu'il y a quelque part un include approprié qui te permettra d'écrire à la place while(error != ERROR_TRUC). Non seulement c'est plus parlant mais en plus ça facilite la portabilité
• prends l'habitude de terminer tes printf() par un "\n". Déjà l'affichage est alors immédiat mais en plus cet affichage est propre à l'écran (au lieu d'avoir des messages se succèdant à la file les uns des autres).
• c'est dommage un gros main énorme et assez imbuvable. Essaye de découper tes tâches en petites fonctions dédiées. Ainsi une fonction pour établir ta connexion, une autre pour le dialogue etc.
• à l'inverse du point précédent, il est inutile de créer une fonction empty_buf() qui ne fait rien d'autre que d'appeler memset(). Même si le nom de l'autre ne te plait pas, avec l'habitude ça te parlera et ça deviendra ensuite tout aussi simple d'utiliser directement memset().
• évite sizeof autant que possible. Leternel explique bien pourquoi. sizeof(tableau) renverra la taille du tableau mais sizeof(pointeur) renverra la taille du pointeur (c.a.d. 4). Or suffit que tu déportes un travail manipulant un tableau et utilisant sizeof()dans une fonction recevant alors le tableau sous forme de pointeur pour que le code ne fonctionne plus.

Ca me semble à peu près tout. Ah oui bien entendu on ne fflushe jamais stdin. Tu dois en permanence maitriser ton clavier et savoir ce qu'il contient et pourquoi ça le contient et comment alors vider proprement ce qui ne convient pas. Surtout qu'un programme contenant un fflush(stdin) (même dans le cas où cette instruction est implémentée) se comportera de façon anarchique si tu le places derrière un pipe (ce qui arrive parfois dans le monde Unix/Linux).

Bon courage

[YannisBoukhalfa]

Merci pour tous vos conseils. Je comptais le passer sous forme de petites fonctions mais je me sentais plus à l'aise de le tester comme ça au début. Je m'y met maintenant. Je vous recontacterai si je rencontre d'autres difficultés. Encore merci pour tout.