Really strange problem.

[rai32]

Hello people,

i am really sorry to post two new threads in such a short time, but i have a problem that's driving me nuts. I think i know where my program fails, but i don't know the reason why. Let me explain:

I am making a chat program. I have one server, and then a client program. An undetermined number of clients (a.k.a users) can join the chat. I'm using the UDP protocol instead of TCP.

Any message that the clients send to the server has this structure:

"computer_name:user_name:message"

Well, to keep track of the "connected" users (as i'm using UDP, they're not exactly connected), i need to make a list of nodes, being each client a node (note that various clients can connect from the same computer). The structures are as follows:

// Estructuras de datos
struct nodo {

  char *nom_usuario;  //user_name
  struct sockaddr_in IP;
  struct nodo *siguiente;  //next

};

  struct lista {
    struct nodo *primero; //first
    struct nodo *ultimo;  //last

  };

The main mission of the server is to re-send the received messages to every client except for the one that sent the original message. Nothing too fancy.

So, my approach to the server was that firstly i needed to make a function in order to create a new node everytime a new client sends a message.

The code is as follows:

int insertar(char* buffer2, struct lista* maestra,int count, char* IP); 
int insertar(char* buffer2, struct lista* maestra,int count, char* IP)
   {
	

     	if(count == 1) //This only applies if the list is empty
	{
		
		struct nodo *nodo1 = (struct nodo *)malloc(50);	
                nodo1->IP.sin_addr.s_addr=inet_addr(IP);
		
		nodo1->nom_usuario=buffer2;
		nodo1->siguiente=NULL;
		maestra->primero=nodo1;
		maestra->ultimo=nodo1;
		count++;

        }// Fin count=1
       
       else 
	{
	
	struct nodo *aux;
	struct nodo *aux2;
	
	for(aux=maestra->primero;aux!=NULL;aux=aux->siguiente){
	
	if(strcmp(aux->nom_usuario,buffer2)==0) //If the user already exists in the list,we don't create the new node
	return 0;
	aux2=aux;
	

	}
	
	//If the function reaches this point is because the user didn't exist, so we create the new node.
	struct nodo *nodo2 = (struct nodo *)malloc(50);	
        nodo2->IP.sin_addr.s_addr=inet_addr(IP);
	nodo2->nom_usuario=buffer2;
	nodo2->siguiente=NULL;	
	aux2->siguiente=nodo2;
	maestra->ultimo=nodo2;

	count++;
	

	}// Fin else

	

	return 1;
   }

This is where problems begin to happen. I'll try to make it as clear as possible:

In order to check whether this function works ok or not, i decided to make a few test users (inside main), before trying it with real remote users, with these lines:

char *IP;
IP="192.168.3.1";
char msj[]1="pcname:user1:whatever";
n=insertar(msj1,&maestra,1,IP); //"maestra" is the masterlist,previously created.

I made 4 of these fake users, changing the message written, the IP, and the count value. I even checked whether the program discards the already existing users. And the result was....a total success. It worked perfectly.

If i make this loop in main:

for(aux=maestra.primero;aux!=NULL;aux=aux->siguiente){
	printf("%s \n",aux->nom_usuario);
	}

the output is:

user1
user2
user3
user4

So, what's the problem? That for some surrealistic reason, if i try this very same thing with real clients, it doesn't work. If i have for instance 5 computers connected to my server, instead of having X users, it only keeps rewriting the first node. This is the server infinite loop:

while(1)

	{
	   clilen=sizeof(cli_addr);
           
           n = recvfrom(sockfd, msj, MAXLINE, 0, (struct sockaddr *) &cli_addr, &clilen);
	   msj[n]='\0';
	   printf("%s",msj);
	   IP=inet_ntoa(cli_addr.sin_addr);
	   buffer=strchr(msj,':') +1;  //	This way, i separate
	   buffer2=strtok(buffer,":"); //       the username from the rest                      of the message.
	   z = 	insertar(buffer2,&maestra,count,IP);
	   count=count+1;
	   
	}

It seems that everytime a client says something, the nom_usuario of the first (and only) node gets overwritten. My point is why it only happens here and not with the fake clients.

I am totally aware that this is a long thread and can be a real pain to try a check it out, i'll understand if i have no replies. Anyways, thanks a lot in advance.

[mitrmkar]

I believe your 'username' pointers ( nom_usuario ) end up pointing to the same index of the one and only buffer you are using (i.e. buffer indirectly via buffer2 ).

Try changing the code to malloc() + strcpy() the username data instead, like so..

int insertar(char* buffer2, struct lista* maestra,int count, char* IP)
{
  if(count == 1) //This only applies if the list is empty
  {
    /* Note: sizeof(*nodo1) instead of '50' */
    struct nodo *nodo1 = malloc(sizeof(*nodo1));
    nodo1->IP.sin_addr.s_addr=inet_addr(IP);
    
    /* Note: 1 extra byte for '\0' */
    nodo1->nom_usuario = malloc(1 + strlen(buffer2));
    strcpy(nodo1->nom_usuario, buffer2);

    /* .. remember to free() the memory when done with it */

    nodo1->siguiente=NULL;
    maestra->primero=nodo1;
    maestra->ultimo=nodo1;
...

Does that make a difference?

[EDIT]
Also note that malloc() can return a NULL pointer! So you might make sure that it does not happen.

PS. Naturally this approach applies throughout the program (i.e., every nom_usuario is to be handled this way).

[rai32]

Well, it seems that the problem is solved! Awesome job mitrmkar! Thanks a lot!