Erreur flagrante? Serveur (sockets + threads)

Marsh Posté le 08-11-2005 à 23:38:13

Bien le bonjour a tous! :hello:

Voila j'ai cette annee un projet tutore a faire(2e annee de dut info gestion).Pour cela je suis en train de developper un serveur multiclient en C.J'utilise donc les threads et les sockets, et je travaille sous linux (y'aura ptet du windows dans l'affaire plus tard mais c'est optionnel si le temps le permet)
Ne connaissant pas grand chose a la programmation reseau, j'ai cherche sur le net des tuto, des exemples et tout ce qui pouvait m'aider et grace a tout ca et pas mal de temps j'en ai sorti quelquechose qui devrait etre potable (non non c'est pas portable :lol: )
Seulement avant de continuer plus loin, et au vu de ma tres petite experience dans la prog reseau je souhaiterai savoir si dans le code de mon serveur, il vous apparait des erreurs grossieres, des failles assez moches ou memes des fuites de memoires.
Enfin je voudrais avoir au final un code si possible le plus propre possible donc je demande un peu d'aide exterieure

Code :

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <netdb.h>
#include <unistd.h>
#include <pthread.h>
#ifndef LONGUEUR_MAX
#define LONGUEUR_MAX 256
#define MAX_CLIENTS 10
#define NB_FILE_CLIENTS 5
#define PSEUDO "Bienvenue...\nEntrez votre pseudo : "
/*erreurs*/
#define ERR_OPEN_SOCK "Erreur d'ouverture de la socket!\n"
#define ERR_BIND "Erreur de bind!\n"
#define ERR_LISTEN "Erreur de listen!\n"
#define ERR_ALLOCATE_MEM_CLIENT "Erreur d'allocation memoire pour le client!\n"
#define ERR_PSEUDO "Erreur de pseudo! Celui choisi est deja en cours d'utlisation!\n"
#define ERR_WRITE "Erreur d'ecriture!\n"
#define ERR_ALLOCATE_MEM_PSEUDO "Erreur lors de l'allocation memoire pour le pseudo!\n"
#define ERR_RESEAU "Erreur de reseau!\n"
#define ERR_ACCEPT "Erreur d'acceptation d'un client\n"
#endif
#define PORT 1798
struct s_client{
pthread_t id;
int socket;
char * pseudo;
};
typedef struct s_client client;
typedef struct sockaddr_in struct_socket;
client tab_clients[MAX_CLIENTS];
char nom_serveur[LONGUEUR_MAX];
int case_libre=0;
int nb_clients;
/**************************************************************/
/* Creation serveur */
/**************************************************************/
int creation_serveur(int port){
/*variables utiles a la creation du serveur*/
int sock;
struct_socket adresse_socket;
/*initialisation du nom du serveur*/
gethostname(nom_serveur,sizeof(nom_serveur));
if ((sock=socket(PF_INET,SOCK_STREAM,0))<0){
printf (ERR_OPEN_SOCK);
exit(-1);
}
/*initialisation du serveur*/
memset(&adresse_socket,0,sizeof(struct sockaddr_in));
adresse_socket.sin_family=AF_INET;
adresse_socket.sin_port=htons(port); // htons convertit un entier court en entier "reseau"
adresse_socket.sin_addr.s_addr=htonl(INADDR_ANY); // htonl : de meme que htons mais pour un entier long INADDR_ANY=0 -> designe l'adresse du poste sur lequel est lance le programme
/*affectation d'un nom a la socket*/
if (bind (sock,(struct sockaddr *)&adresse_socket,sizeof(struct sockaddr_in))<0){
perror (ERR_BIND);
exit(-1);
}
/*"initialisation" des "options" d'ecoute de la socket*/
if (listen(sock,NB_FILE_CLIENTS)<0){
perror (ERR_LISTEN);
exit(-1);
}
return sock;
}
/**************************************************************/
/* Envoi de message a un seul client */
/**************************************************************/
int envoi_message(int sock, char * message){
if ((write(sock,message,strlen(message)))==-1){
perror(ERR_WRITE);
return 1;
}
return 0;
}
/**************************************************************/
/* Envoi de message a tous les clients sauf celui specifie */
/**************************************************************/
int envoi_limite(char * message,int sock){
int i;
for (i=0;i<case_libre;i++){
if(tab_clients[i].socket != sock){
envoi_message(tab_clients[i].socket,message);
}
}
return 0;
}
/**************************************************************/
/* Envoi d'un message a tous les clients du serveur */
/**************************************************************/
int envoi_general(char * message){
int i;
for (i=0;i<case_libre;i++){
envoi_message(tab_clients[i].socket,message);
}
return 0;
}
/**************************************************************/
/* Depart d'un client */
/**************************************************************/
void client_quit(client * client_courant,char * raison){
int i,j;
char buf[8192+1];
if ((strcmp(raison,'\0'))>0){
snprintf(buf,8192,"%s part vers d'autres cieux...(%s)\n",client_courant->pseudo,raison);
}
else {
snprintf(buf,8192,"%s part vers d'autres cieux...(%s)\n",client_courant->pseudo,raison);
}
envoi_limite(buf,client_courant->socket);
/*recherche du client dans tab_clients*/
for (i=0;(tab_clients[i].socket != client_courant->socket);i++);
/*liberation des ressources du client*/
close(client_courant->socket);
free(client_courant->pseudo);
free(client_courant);
/*reorganisation de tab_clients*/
for (j=i+1;j<case_libre;j++){
tab_clients[j-1]=tab_clients[j];
}
case_libre--;
nb_clients--;
}
/**************************************************************/
/* Acceptation d'un client par le serveur */
/**************************************************************/
int client_accepte(int socket_serveur){
int socket_client;
if((socket_client = accept(socket_serveur,NULL,0)) < 0)
{
if(errno == EINTR)
{
shutdown(socket_serveur,SHUT_RDWR);
close(socket_serveur);
nb_clients--;
return -1;
}
else
{
perror(ERR_ACCEPT);
exit(-1);
}
}
fcntl(socket_client,F_SETFD,1);
return socket_client;
}
/**************************************************************/
/* Interaction avec le client : thread */
/**************************************************************/
void * thread_client(void *args){
char pseudo[255];
char message_emis[8192+1];
char message_recu[4096+1];
int case_libre=0;
int len,i;
int sock=*((int *)args);
client *client_courant=NULL;
if ((client_courant=malloc (sizeof(struct s_client)))==NULL){
perror (ERR_ALLOCATE_MEM_CLIENT);
close (sock);
nb_clients--;
pthread_exit(NULL);
}
envoi_message(sock,PSEUDO);
len = read(sock,message_recu,4096);
if(len <= 0) {
printf("\nErreur\n" );
close(sock);
free(me);
client_courant = NULL;
nb_clients--;
pthread_exit(NULL);
}
/*recuperation du pseudo du client courant*/
for (i=0;(pseudo[i]!='\0')&&(pseudo[i]!='\n');i++); //on va a la fin du pseudo
pseudo[i]='\0'; // on met un \0 pour le terminer proprement
for (i=0;i<case_libre;i++){
if (strcmp(pseudo,tab_clients[i].pseudo)==0){ // si strcmp==0 alors les chaines sont identiques
if ((write(sock,ERR_PSEUDO,strlen(ERR_PSEUDO)))==-1){
perror(ERR_WRITE);
}
close (sock); //fermeture de la socket du client courant
free(client_courant); //liberation de la memoire allouee pour le client courant
nb_clients--;
pthread_exit(NULL);
}
}
/*initialisation de la structure du client courant*/
if(client_courant->pseudo=malloc(sizeof(pseudo))==NULL){
perror (ERR_ALLOCATE_MEM_PSEUDO);
nb_clients--;
pthread_exit(NULL);
}
client_courant->id=pthread_self();
client_courant->socket=sock;
strcpy(client_courant->pseudo,pseudo);
/*placement du client dans le tab_client a la premiere "case libre"*/
tab_clients[case_libre]=*client_courant;
case_libre++; //le prochaine case libre du tableau est un rang plus loin
snprintf(message_emis,8192,"Nouveau Client : %s\n",client_courant->pseudo);
message_emis[8192]='\0';
if ((write(sock,message_emis,strlen(message_emis)))==-1){
perror(ERR_WRITE);
}
while (1){
len=read(sock,message_recu,8192); //read retourne le nombre d'octets lus, -1 en cas d'echec, 0 si rien ou en fin de fichier
if (len<=0){ //s'il y a erreur de lecture ou fin du message
client_quit(client_courant,ERR_RESEAU);
pthread_exit(NULL);
}
message_recu[len]='\0';
snprintf(message_emis,8192,"%s : %s\n",client_courant->pseudo,message_recu);
message_emis[8192]='\0';
envoi_general(message_emis);
}
return NULL;
}
/**************************************************************/
/* Fonction principale : lancement du serveur */
/**************************************************************/
int main(int argc, char **argv)
{
int socket_serveur,socket_client;
pthread_t id;
socket_serveur = create_server(PORT);
while(1)
{
if ((socket_client = client_accepte(socket_serveur))==-1){
perror(ERR_ACCEPT);
exit(-1);
}
if(nb_clients < MAX_CLIENTS)
{
pthread_create(&id,NULL,thread_client,(void *)&socket_client);
nb_clients++;
printf("Nouveau client! Il y a actuellement %d clients sur le serveur\n",nb_clients);
}
else close(socket_client);
}
return 0;
}

J'avoue c'est un peu long alors je remercie d'avance ceux qui auront la patience de tout lire et de m'aider
Merci

Message édité par gocho le 10-11-2005 à 00:14:39

Reply

Marsh Posté le 08-11-2005 à 23:38:13

Reply

Marsh Posté le 09-11-2005 à 23:45:01

y'as pas de fautes ?

Reply

Marsh Posté le 09-11-2005 à 23:47:42

exit, c'est moche

Code :

if ((write(sock,message,strlen(message)))==-1)

a mon avis ton test est pas bon, tu dois vérifier que write te retourne bien le meme nombre d'octet que tu lui a demandé d'ecrire

Message cité 1 fois
Message édité par chrisbk le 09-11-2005 à 23:48:39

Reply

Marsh Posté le 09-11-2005 à 23:51:33

Code :

if ((strcmp(raison,'\0'))>0){

tordu

Code :

if (strlen(raison)>0)

Code :

for (i=0;(pseudo[i]!='\0')&&(pseudo[i]!='\n');i++);

pour ce genre de chose, je trouve while plus logique est plus lisible

sinon spa trop mal, par contre je suis vraiment pas allé dans le detail

Reply

Marsh Posté le 10-11-2005 à 00:02:55

chrisbk a écrit :

exit, c'est moche

Code :

if ((write(sock,message,strlen(message)))==-1)

a mon avis ton test est pas bon, tu dois vérifier que write te retourne bien le meme nombre d'octet que tu lui a demandé d'ecrire

exit C'est juste moche ou c'est deprecated (j'aime bien ce mot )? return c'est mieux?

pour le write j'ai teste si ca avait pas echoue.Ce serait mieux si je faisais ca?

Code :

if ((write(sock,message,strlen(message)))!=(strlen(message))

Citation :

Code :

for (i=0;(pseudo[i]!='\0')&&(pseudo[i]!='\n');i++);

pour ce genre de chose, je trouve while plus logique est plus lisible

voui c'est sur...vais changer ca.

Citation :

sinon spa trop mal, par contre je suis vraiment pas allé dans le detail

merci.si en superficiel deja ca le fait....

Message cité 1 fois

Reply

Marsh Posté le 10-11-2005 à 00:06:54

gocho a écrit :

exit C'est juste moche ou c'est deprecated (j'aime bien ce mot )? return c'est mieux?

juste moche, il vaut mieux faire remonter l'erreur aussi haut que possible, des fois qu'il y ait des liberations de ressources a faire

Citation :

pour le write j'ai teste si ca avait pas echoue.Ce serait mieux si je faisais ca?

Code :

if ((write(sock,message,strlen(message)))!=(strlen(message))

ca te fais deux strlen, et trois appels de fonction sur une meme ligne, ca devient guere lisible

Citation :

size_t truc = strlen(message);

et zou avec truc

gocho a écrit :

merci.si en superficiel deja ca le fait....

bin ptet je traine trop ici, a force de voir des horreurs accablante on perds la faculté a chercher la ptite bete [:moule_bite]

Reply

Marsh Posté le 10-11-2005 à 00:08:27

Code :

else close(sock);

y sort d'ou, ce sock ?

(bon sinon les vars globales, ca aussi c'est moche)

Reply

Marsh Posté le 10-11-2005 à 00:10:08

heuh mais ca compile et ca se lance, ton truc ? [:le kneu]

Reply

Marsh Posté le 10-11-2005 à 00:16:41

euh j'ai po trop compris le "zou avec truc " ^^
le sock c'est quand j'ai renomme mes variables, j'ai oublie celle la (et je m'etonnais de me taper un segfault quand je terminais....je comprends un peu maintenant
Sinon ouais ca se compile et ca se lance (enfin aujourd'hui parce que j'ai fait quelques modifs dessus toute a l'heure et j'ai pas eu le temps de les reporter ici )

Reply

Marsh Posté le 10-11-2005 à 00:17:18

bin des "sock" t'en a plein ton programme, donc je m'etonne un peu que ca compile [:el g]

Reply

Marsh Posté le 10-11-2005 à 00:17:18

Reply

Marsh Posté le 10-11-2005 à 00:18:06

et pour les globales vaut mieux que je les declare dans le main et que je les passe en param de chaque fonction ou j'en ai besoin?

Reply

Marsh Posté le 10-11-2005 à 00:19:26

pour le sock ca depend des fonctions en fait, il me semble que c'etait le seul.
Dans les autres c'est normal (enfin va falloir que je repasse ca au crible : : )

Reply

Marsh Posté le 10-11-2005 à 00:19:41

bin vu leur tronche, deja je dirais qu'elles iraient bien dans une structure, vu qu'elles servent a decrire une seule et meme chose (un serveur et ses clients), ca te limitera le paquet

apres comme t'es en environnement multithread, il faut que tu geres avec attention l'utilisations de ce genre de variable partagée entre plusieurs thread. (utilisation de synchronisation)

Reply

Marsh Posté le 10-11-2005 à 00:21:56

ah...j'pensais bien que ca viendrait la.
Comme j'ai pas encore vraiment bien capte tout ca :
La synchro c'est comment on la fait? c'est juste empecher deux threads d'acceder a la meme variable en meme temps avec les mutex c'est ca? (j'ai jete un coup d'oeil mais pas encore eu le temps de m'atteler a la tâche)

Message cité 2 fois

Reply

Marsh Posté le 10-11-2005 à 00:24:44

et pour la structure un truc du genre :

Code :

struct _serveur{
char nom_serveur[LONGUEUR_MAX];
int nb_clients;
int case_libre;
client tab_clients[MAX_CLIENTS];
};
typedef struct _serveur serveur;

ca le fait?

Message cité 1 fois
Message édité par gocho le 10-11-2005 à 00:25:00

Reply

Marsh Posté le 10-11-2005 à 00:24:52

gocho a écrit :

La synchro c'est comment on la fait? c'est juste empecher deux threads d'acceder a la meme variable en meme temps avec les mutex c'est ca? (j'ai jete un coup d'oeil mais pas encore eu le temps de m'atteler a la tâche)

ouais, par exemple.

Si on prends une de tes boucles

Code :

for (i=0;i<case_libre;i++){

la c'est dangeureux car tu te bases sur une borne qui peut potentiellement varier. Imagine la cas le plus moche,

thread 1 : compare i (2) avec case_libre (3)
thread 1: entre dans la boucle (condition valide)

thread 2 : decremente case_libre, libere la vieille socket situé en client[2]

thread 1 : essaye d'ecrire sur client[2]

Blam

Reply

Marsh Posté le 10-11-2005 à 00:25:53

gocho a écrit :

et pour la structure un truc du genre :

Code :

struct _serveur{
char nom_serveur[LONGUEUR_MAX];
int nb_clients;
int case_libre;
client tab_clients[MAX_CLIENTS];
};
typedef struct _serveur serveur;

ca le fait?

ouais, un peu bourrin mais disons que tu debutes alors oui

par contre tu peux t'eviter des lignes

Code :

typedef struct _serveur{
client tab_clients[MAX_CLIENTS];
char nom_serveur[LONGUEUR_MAX];
int nb_clients;
int case_libre;
}serveur;

Reply

Marsh Posté le 10-11-2005 à 00:26:11

ah j'avoue ca va faire un gros bouzin si ca se passe ca...hop poubelle le programme

Message cité 1 fois

Reply

Marsh Posté le 10-11-2005 à 00:27:19

gocho a écrit :

ah j'avoue ca va faire un gros bouzin si ca se passe ca...hop poubelle le programme

vala. Donc faut proteger au mutex. Ca allourdi le code donc essaye de faire ca proprement

(le multithread c'est clairement pas chose facile)

Reply

Marsh Posté le 10-11-2005 à 00:28:31

oue j'aime bien voir toutes les lignes, l'optimisation des lignes c'est pas mon truc

Sinon pourquoi bourrin? qu'est ce qu'il y aurait de moins bourrin?(ve apprendre moi )

Message cité 1 fois

Reply

Marsh Posté le 10-11-2005 à 00:29:26

gocho a écrit :

Sinon pourquoi bourrin? qu'est ce qu'il y aurait de moins bourrin?(ve apprendre moi )

Bin ce qu'il y a de bourrin c'est le codage direct du nombre de client max et ce genre de truc. Ca fait des gros tableaux dans une structure, c'est pas forcement élegant. une alloc dynamique serait de bon aloy, quoi

Message cité 1 fois
Message édité par chrisbk le 10-11-2005 à 00:29:46

Reply

Marsh Posté le 10-11-2005 à 00:30:23

ben d'apres ce que j'ai vu sur les mutex ca donnerai un truc dans le style :

Code :

int main(int argc, char*argv[]){
//je declare le mutex
//je mets toutes mes conneries
//je block
//je touche aux variables sensibles
//je debloke
}

Is it right?

Reply

Marsh Posté le 10-11-2005 à 00:31:15

chrisbk a écrit :

Bin ce qu'il y a de bourrin c'est le codage direct du nombre de client max et ce genre de truc. Ca fait des gros tableaux dans une structure, c'est pas forcement élegant. une alloc dynamique serait de bon aloy, quoi

un p'ti coup de malloc donc...j'vais commencer a bien le gerer celui la a la fin :lol:

edit : attends attends...si je declare mon tableau de client avec un malloc.
il est donc NULL au debut.
Mais si je le remplis, ca veut dire realloc a chaque nouveau client/depart de client ?c'est bien ca ( :cry: inside)?

Message cité 1 fois
Message édité par gocho le 10-11-2005 à 00:32:50

Reply

Marsh Posté le 10-11-2005 à 00:33:34

en gros, mais pas que dans le main hein ? tes fonctions envoi_limite et envoi_general en profiterait bien aussi, par exemple.

Y'a de la bonne base, mais y'aurait pas mal de chose a faire sur ton code pour le rendre plus chouette et solide, seulement la chui un peu claqué [:pingouino] Essaye de voir si certaines parties peuvent etre redécoupée en fonction (pour racourcir d'autre fonctions), et betement aussi essaye de sortir le code de main() qui n'a rien a y faire (grosso modo, tout ) et qui donc serait mieux dans une fonction

Reply

Marsh Posté le 10-11-2005 à 00:36:13

oui oui le mutex quand je disai dans le main ,j'entendai par la dans chaque fonction ou je modifie une variable.

comment ca redecoupees? enfin je comprends c'que ca veut dire mais quoi par exemple?
Sinon pour le main en fait je compte laisser au final que de l'appel de fonction, c'est juste que tout ce qui reste n'a pas encore ete fait

Reply

Marsh Posté le 10-11-2005 à 00:36:38

gocho a écrit :

un p'ti coup de malloc donc...j'vais commencer a bien le gerer celui la a la fin :lol:

edit : attends attends...si je declare mon tableau de client avec un malloc.
il est donc NULL au debut.
Mais si je le remplis, ca veut dire realloc a chaque nouveau client/depart de client ?c'est bien ca ( :cry: inside)?

bon, globalement, je verrais un truc comme ca (vite fait, hein, en me basant sur ton code)

Code :

typedef struct _MonServeur
{
client * tab_client;
char * nom_serveur;
int case_libre;
int nb_client;
}MonServeur;
MonServeur * CreerServeur(const char *nom, int maxClient)
{
MonServeur res = malloc(sizeof(struct _MonServeur));
res->tabClient = malloc(sizeof(client) * maxclient);
res->case_libre = res->nb_client = 0:
res->nom_serveur = strdup(nom);
return res;
}

(je torche un brin, pas de test ni rien)

et pour la suite des operations tu bosses sur ta structure "MonServeur"

Reply

Marsh Posté le 10-11-2005 à 00:38:27

ok.donc du dynamique au maximum...on va essayer hein.
Par contre : a quoi sert le const? je l'ai deja vu mais jamais encore reussi a capter a quoi qu'il sert.

Message cité 1 fois

Reply

Marsh Posté le 10-11-2005 à 00:39:17

gocho a écrit :

ok.donc du dynamique au maximum...on va essayer hein.
Par contre : a quoi sert le const? je l'ai deja vu mais jamais encore reussi a capter a quoi qu'il sert.

c'est du "je regarde, mais je touche pas"

donc la ca signifie que je prends une chaine en entree, mais que je "m'engage" (avec l'aide du compilo) a ne pas la modifier

Reply

Marsh Posté le 10-11-2005 à 00:41:28

ok. parametre non modifiable par la fonction donc...
il me semble aussi en avoir deja vu sur la fonction non?
du genre const int machin_truc (arg1, arg2) c'est possible ca?
Et si oui ca sert a quoi?

Reply

Marsh Posté le 10-11-2005 à 00:41:58

bin la ca veut dire que tu renvoie un entier const, ce qui est d'une utilité discutable

Reply

Marsh Posté le 10-11-2005 à 00:42:07

(bon chu pu la, hein ? )

Reply

Marsh Posté le 10-11-2005 à 00:43:08

ouep c'est clair...enfin c'est pas le sujet : je m'egare.
je sais maintenant comment occuper mon demain aprem'
edit : oue pareil moi non plus (bonne nuit inside) et pour ceux qui passent par la n'hesitez pas a poster pour me dire si vous voyez d'autres choses

Message édité par gocho le 10-11-2005 à 00:43:59

Reply

Marsh Posté le 10-11-2005 à 07:07:45

gocho a écrit :

ah...j'pensais bien que ca viendrait la.
Comme j'ai pas encore vraiment bien capte tout ca :
La synchro c'est comment on la fait? c'est juste empecher deux threads d'acceder a la meme variable en meme temps avec les mutex c'est ca? (j'ai jete un coup d'oeil mais pas encore eu le temps de m'atteler a la tâche)

http://mapage.noos.fr/emdel/pthreads.htm

---------------
Des infos sur la programmation et le langage C: http://www.bien-programmer.fr Pas de Wi-Fi à la maison : http://www.cpl-france.org/

Reply

Marsh Posté le 10-11-2005 à 08:45:16

ok.donc faut que je vois du cote du mutex en parallele avec le pthread_join pour que ce soit optimal donc.
Plus qu'a faire tout ca maintenant...

Reply

Marsh Posté le 13-11-2005 à 22:45:46

Re bonsoir.
Alors apres un peu de retard j'ai pu me replonger dans mon petit projet.
j'ai retravaille tout ca pour regler toutes les erreurs que j'avais lors de la compilation mais il en reste une que je comprends pas :
Quand je compile gcc me renvoie un desagreable : collect2 : ld returned 1 exit status

Est ce que quelq'un pourrait me dire ce que ca veut dire (dois je reposter le code?)
merci bien

Reply

Marsh Posté le 13-11-2005 à 23:06:37

pb de link, poste plutot l'erreur en entier

Reply

Marsh Posté le 13-11-2005 à 23:12:09

je l'ai poste sur un sujet a part, je me dis que ca pourrait reservir a quelqu'un
http://forum.hardware.fr/hardwaref [...] 0093-1.htm

Reply

Marsh Posté le

Reply

Erreur flagrante? Serveur (sockets + threads)

Sujets relatifs:

Leave a Replay