pThread

Cornu Nicolas wrote:

D'abord un petit détail. Le langage C++ ne connaît ni les
threads ni les sockets. Dans la mésure que la plupart des
implémentations sur les machines généralistes les supportent, on
en parle ici, mais on se limite à des aspects génériques propre
à C++ (le « thread-safety », l'encapsulation des sockets, etc.).
Si tu as une question réelement propre à la gestion des sockets,
il vaut mieux la poser dans un forum propre au système
d'exploitation.

J'ai crée une classe ServeurSocket et une classe Serveur
composé d'une instance de la classe ServeurSocket

Lorque la méthode Accept est appelé depuis run() &ListenSocket
retourne 1 et j'obtient une erreure de lecture. Sinon
j'obtient un pointer.

Il y a plusieurs choses de suspect dan ton programme, surtout au
niveau des types. Mais d'abord, deux commentaires un peu
généraux :

-- En général, il vaut mieux éviter les noms tout en majuscule
sauf pour les commentaires. C'est une convension tellement
répandue que bien de programmeurs C++ voir automatiquement
un macro dès qu'il voit un symbole tout en majuscule (à
l'exception peut-être des symboles d'une seule lettre -- T
est assez consacré comme nom du paramètre type d'un
template).

-- AMHA, ce n'est pas une très bonne idée d'utiliser autant
d'espaces référentiels. J'ai déjà travaillé sur un projet où
on les utilisait un peu comme les noms de package en Java ;
c'était pénible. (En Java, au moins avec JDK, on y est
prèsque obligé, puisque le compilateur insiste que la
structure des répertoires reflète les hièrarchies des
packages. En C++, on a la liberté de faire mieux -- je
dirais même de ne pas faire cette bêtise.)

En gros, je n'utiliserais les espaces référentiels qu'à un
niveau plus élevé, pour des sous-systèmes entiers. Dans les
petits projets, je m'en passerais complètement (sauf pour
les espaces référentiels anonymes). Pour des bibliothèques
externes, qui sont développées à part, et servent dans
plusieurs applications, je mettrais toute la bibliothèque
dans un espace référentiel dont le nom contient la version,
avec ensuite quelque chose du genre :

namespace Gabi = Gabi_v200501 ;

Comme ça, si tu compiles avec les en-têtes d'une version, et
linke avec une autre, tu auras des erreurs lors de l'édition
des liens. (J'avoue que je suis très sensible à ce genre de
problème, ayant déjà perdu pas mal du temps à la récherche
des erreurs dues au fait qu'un des fichiers objet n'a pas
été récompilé quand il fallait. Normalement, c'est la
gestion du projet qui doit éviter de telles bavures, mais
dans la pratique, la gestion du projet est rarement
parfaite.)

Mais enfin, ce sont des avis personnels : à prendre ou à
laisser. Maintenant, pour les vrais problèmes :

----------------------------------------------------------------------------

------------------
void * SMSD::CommClient::run(void *server_socket)

Je suis assez méfiant de ces void*. Ça sent l'interface C. Mais
laquelle ?

{
time_t t1;
SOCKET* AcceptSocket = new SOCKET();

Est-ce qu'il y a une raison d'allouer sur la pile, plutôt que de
déclarer l'objet directement ? Si tu alloues sur la pile, il
faut impérativement que tu libères (explicitement) quelque part.
Si la durée de vie souhaitée correspond à une portée (ce qui est
assez souvent le cas), un simple :

SOCKET AcceptSocket ;

fait l'affaire, et est de loin préférable.

while (true)
{
((COMM::ServeurSocket*)server_socket)->Accept(AcceptSocket);

Et voilà. Pourquoi est-ce que run() ne prend pas carrément un
COMM::ServeurSocket* ? Ça ne peut pas être directement pour des
raisons de compatibilité C, parce que tu ne peux pas appeler
cette fonction depuis le C.

J'aimerais plus de détails ; quelque chose cloche, et je suis
prèsque sûr qu'il y a une erreur plus bas.

Aussi, dans le cas exceptionnel où il faut passer un void*, je
ferais l'affectation à un pointeur avec le bon type comme
première chose dans la fonction (et au moyen d'un
reinterpret_cast). C'est un peu une question de style, mais je
n'aime pas que les void* servent plus que nécessaire.

printf("Client connected.n");
printf("%d",AcceptSocket);

Aïe ! Ici, AcceptSocket est tout sauf un entier. Tu ne peux donc
pas le passer à un %d dans un printf. Pour éviter ce genre de
problème, et bien d'autres, il vaut mieux utiliser les iostream.
(Note que dans la mésure où la chaîne de formattage est une
chaîne littérale, un bon compilateur aurait quand même sorti un
avertissement. Mets le niveau d'avertissement au maximum, et
écrit du code pour qu'il compile sans avertissements. Puisque
c'est un projet d'étudiants, et que tu te sers de pthread, je
suppose Linux, et donc g++. Et g++ avertit bien dans ce cas-ci.)

Mais je me pose toujours la question : qu'est-ce que tu voulais
réelement faire ? AcceptSocket est un pointeur, mais
*AcceptSocket n'est pas un entier non plus.

}
pthread_exit(NULL);
return NULL;

Tu ne reviens pas de pthread_exit. Le return est donc superflu.

Il y en a, comme moi, qui n'aime pas utiliser pthread_exit (ni
exit) ; on préfère remonter à coup de return, voire au moyen
d'une exception, et puis faire un return dans la fonction donnée
à pthread_create (qui doit obligatoirement être « extern "C" »
-- ne l'oublie pas). D'autant plus qu'au moins sur certaines
plate-formes (Solaris sur Sparc), pthread_cancel (et donc
pthread_exit, dont la sémantique est définie en termes de
pthread_cancel) ne fonctionne pas correctement avec g++. (Mais
il faut se méfiait ici. La définition de « correctement » pour
pthread_cancel dans un programme C++ ne fait pas partie de la
norme Posix, et risque donc de varier d'une machine à l'autre.)

}

----------------------------------------------------------------------------

------------------
void SMSD::COMM::ServeurSocket::Accept(SOCKET* AcceptSocket)
{
*AcceptSocket = SOCKET_ERROR;

Tu déréférences sans avoir testé pour NULL ? Si tu n'acceptes
pas les pointeurs NULL, il vaut mieux utiliser une référence.
(AMHA -- c'est encore une question de style. Mais déréférencer
un pointeur sans avoir vérifier qu'il ne soit pas NULL est AMHA
une erreur.)

Et encore, quels sont les types ? Pour que ça a un sens, il faut
que SOCKET_ERROR ait le type SOCKET. Alors que je m'attendrais à
ce qu'un type qui s'appelle SOCKET modèle un socket -- en tant
que tel, il est probable qu'il ne supporte pas l'affectation.

Ici aussi, il nous manque assez d'information pour être sûr,
mais j'ai bien l'impression que tu t'es planté encore dans les
types (au moins que SOCKET soit un simple typedef pour int, mais
alors, pourquoi tous les pointeurs ?).

while( *AcceptSocket == SOCKET_ERROR ) {
printf(".%d,",AcceptSocket);

Encore, un pointeur ne passe pas à %d. Utiliser iostream, et tu
n'auras pas ce genre de problème.

printf("%d",&ListenSocket);

C'est quoi, ListenSocket ? La seule façon que ce serait légal,
c'est si c'est une classe avec l'operator& surchargé pour
renvoyé un int. Ce qui m'étonnerait (et decevra) beaucoup.

*AcceptSocket = accept( this->ListenSocket, NULL, NULL );

Dois-je supposer qu'accept, ici, est la fonction Posix du même
nom ?

Pour reprendre ta question au départ :

Lorque la méthode Accept est appelé depuis run() &ListenSocket
retourne 1 et j'obtient une erreure de lecture. Sinon
j'obtient un pointer.

Je ne comprends pas vraiment la question, mais ne sachant pas
plus sur les types, c'est difficile à dire plus. Une chose, en
revanche, est certain : le type d'une expression est figée lors
de la compilation, et ne dépend pas non plus du contexte (sauf
dans quelque cas vraiment rares). L'expression &ListenSocket
renvoie une adresse, sauf surcharge (probablement abusif) de
l'opérateur &. Si tu affiches un « 1 », c'est que l'expression a
évalué à l'adresse 1, ce qui veut dire prèsqu'à coup sûr qu'il y
a un comportement indéfini (probablement un pointeur mal
initialisé) ailleurs. Que vaut this ?

(Pour sortir un pointeur avec printf, la spécification de
conversion est "%p". Mais comme j'ai dis, il vaut mieux utiliser
std::cout, et ne pas se soucier du type.)

}
printf("Client connected.n");
//return AcceptSocket;
}

----------------------------------------------------------------------------

-------------------

Si j'ai bien compris ta fonction Accept, elle doit plutôt
ressembler à :

int
SMSD::COMM::ServeurSocket::Accept( int socketId )
{
int resultat = -1 ;
while ( resultat < 0 ) {
resultat = accept( socketId, NULL, NULL ) ;
}
return resultat ;
}

Sauf que dans la pratique, je mettrais un peu plus de gestion
des erreurs. (Au minimum, si accept échoue, je le signalerais
dans le log.)

--
James Kanze mailto: james.kanze@free.fr
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 pl. Pierre Sémard, 78210 St.-Cyr-l'École, France +33 (0)1 30 23 00 34

Je vous fait parvenir les sources.
J'ai commencer à modifier certaines choses. accept retourne un type SOCKET
sous windows

NC

Finalement je pense utiliser une librarie portable. Et je n'aurais plus
beosin des threads a ce niveau.
Je reste curieux de la solution.

NC

"Cornu Nicolas" <nicolas.cornu@club-internet.fr> a écrit dans le message de
news:42627eb1$0$15276$7a628cd7@news.club-internet.fr...

Je vous fait parvenir les sources.
J'ai commencer à modifier certaines choses. accept retourne un type SOCKET
sous windows

NC