Socket:multiple reads and writes, how to avoid overflow?

I've been serching on this topic for a while, both on google and here. I've read "Beej's Guide to Network Programming" as someone here sugested and the FAQ section about sockets where Hammer talks about overflow and my code is like his, and I still get rubish when reading and writing.

Here's the part of the code that matters:

Code:

---


#define BUF_CLIENT 50

SERVER SIDE

void* handle_cliente(int client_fd){
        int nread = 0,nwrite;
        char find[BUF_CLIENT];
        char buffer[BUF_CLIENT];
        nread = read(client_fd, buffer, BUF_CLIENT-1);
        buffer[nread-1] = '\0';
        printf("nread: %d",nread);      //just so I can see
        strcpy(find,host_lookup(buffer));      //don't bother with this
        printf("\n.%s.\n",buffer);      //just so I can see
        printf("tam:%d",strlen(find));
        find[strlen(find)]='\0';
        printf("\n.%s.\n",find);      //just so I can see
        nwrite=write(client_fd,(void*)find, strlen(find));
        printf("\nwrite:%d\n",nwrite);      //just so I can see
        close(client_fd);
        return NULL;
}

CLIENT SIDE
...
        char ip[BUF_CLIENT];
        write(fd, argv[3], strlen(argv[3])+1);
        nread = read(fd, ip, BUF_CLIENT-1);
        ip[nread]='\0';
        printf("nread:%d",nread);      //just so I can see
        printf("IP: %s.\n",ip);
        close(fd);
        return 0;
...

---

I've been trying to solve this problem all afternoon and would apreciate a quick fix as I believe this is easy to solve :p

Thanks ;)

EDIT: Using bzero() on the arrays before storing data into them is going in the right direction? Still not working..

One of you has to know the answer...

Are you sure that find have enought room for host_lookup(buffer), you could use strncpy.

Code:

---

        strcpy(find,host_lookup(buffer));      //don't bother with this

---

I think strlen looks for a terminating zero to determine the length. I assume that find don't have a terminating zero since you do this.

Code:

---

        find[strlen(find)]='\0';

---

I don't know I would look at these places at least.

host_lookup() returns a string containing an ip address.. its max length will be 15. find's size is 50. there's enough space.

concerning the strlen() stuff, I have already zeroed the find string with bzero() and/or a for cycle. Any other suggestions? Do you guys prefeer i post the entire code?

Thanks.

Quote:

---

Originally Posted by CYBERRRR

host_lookup() returns a string containing an ip address.. its max length will be 15. find's size is 50. there's enough space.

---

Well, somewhere something is larger than you expect, hence the overflow.

server.c

Code:

---

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <pthread.h>
#include <time.h>
#include <netdb.h>
#include <netinet/in.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <unistd.h>
#include <sys/wait.h>
#include <semaphore.h>
#include <fcntl.h>

#define CLIENT_PORT 8007
#define ADMIN_PORT 8008
#define BUF_CLIENT 50
#define BUF_ADMIN 1024

#define MENU "\nAdmin Menu\n1-Ver Lista de Dominios\n2-Adicionar \
Dominio\n3-Apagar Dominio\n4-Apagar Lista de Dominios\n5-Ver Log\n\
6-Apagar Log\n"

typedef struct dados* list;
typedef struct dados{
        char ip[50];
        char nome[50];
        char data[50];
        list next;
} dns_list;

typedef struct log_data* log;
typedef struct log_data{
        char user[9];
        char query[10];
        char objecto[50];
        char l_time[50];
        log next;
} log_list;

void* cliente_connection();
void* admin_connection();
void* handle_cliente();
void* handle_admin();
char* host_lookup(char*);
list read_dns();
void erro(char*);
void write_log(char* user,char* query, char* objecto);

list lista;
sem_t MUTEX;

int main(){
        lista=(list)malloc(sizeof(dns_list));
        if(lista!=NULL) lista->next=NULL;
        lista=read_dns(lista);
        sem_init(&MUTEX,0,1);
        pthread_t cliente_t;
        pthread_t admin_t;
        pthread_create(&cliente_t, NULL, cliente_connection, NULL);
        pthread_create(&admin_t, NULL, admin_connection, NULL);
        pthread_join(cliente_t,NULL);
        pthread_join(admin_t,NULL);
        return 0;
}

void* cliente_connection(){
        pthread_t cliente_handle_t;
        int hc, client;
        struct sockaddr_in c_addr, client_addr;
        socklen_t client_addr_size;
        bzero((void *) &c_addr, sizeof(c_addr));
        c_addr.sin_family = AF_INET;
        c_addr.sin_addr.s_addr = htonl(INADDR_ANY);
        c_addr.sin_port = htons(CLIENT_PORT);
        if ( (hc = socket(AF_INET, SOCK_STREAM, 0)) < 0)
          erro("na funcao socket");
        if ( bind(hc,(struct sockaddr*)&c_addr,sizeof(c_addr)) < 0)
          erro("na funcao bind");
        if( listen(hc, 5) < 0)
          erro("na funcao listen");
        printf("\nThread CLIENTE criada e a espera de ligacoes\n");
        while (1) {
                client_addr_size = sizeof(client_addr);
                client = accept(hc,(struct sockaddr*) &client_addr,&client_addr_size);
                if (client > 0) {
                        if (pthread_create(&cliente_handle_t, NULL, handle_cliente, (void*)client) != 0) {
                                erro("falhou a criacao da thread client_handle_t");
                        }
                close(client);
                }
        }
       
}

void* handle_cliente(int client_fd){
        int nread,nwrite;
        char find[BUF_CLIENT];
    char buffer[BUF_CLIENT];
    bzero(buffer,BUF_CLIENT);
    bzero(find,BUF_CLIENT);
    nread = read(client_fd, buffer, BUF_CLIENT-1);
        buffer[nread-1] = '\0';
        printf("nread: %d",nread);
        strcpy(find,host_lookup(buffer));
        printf("\n.%s.\n",buffer);
        printf("tam:%d",strlen(find));
        find[strlen(find)]='\0';
        printf("\n.%s.\n",find);
        nwrite=write(client_fd,(void*)find, strlen(find));
        printf("\nnwrite:%d\n",nwrite);
        close(client_fd);
        return NULL;
}

char* host_lookup(char* name){
        list aux;
        aux=lista->next;
        while(aux!=NULL){
                if(strcmp(aux->nome,name)==0){
                        //write_log("CLIENTE","pesquisou",name);
                        return aux->ip;
                       
                }
                else{
                        aux=aux->next;
                }
        }
        //write_log("CLIENTE","pesquisou",name);
       
        return "0.0.0.0";
}

void* admin_connection(){
        pthread_t admin_handle_t;
        int ha, admin;
        struct sockaddr_in a_addr, admin_addr;
        socklen_t admin_addr_size;
        bzero((void *) &a_addr, sizeof(a_addr));
        a_addr.sin_family = AF_INET;
        a_addr.sin_addr.s_addr = htonl(INADDR_ANY);
        a_addr.sin_port = htons(ADMIN_PORT);
        if ( (ha = socket(AF_INET, SOCK_STREAM, 0)) < 0)
          erro("na funcao socket");
        if ( bind(ha,(struct sockaddr*)&a_addr,sizeof(a_addr)) < 0)
          erro("na funcao bind");
        if( listen(ha, 1) < 0)
          erro("na funcao listen");
        printf("\nThread ADMIN criada e a espera de ligacoes\n");
        while (1) {
                admin_addr_size = sizeof(admin_addr);
                admin = accept(ha,(struct sockaddr*) &admin_addr,&admin_addr_size);
                if (admin > 0) {
                        if (pthread_create(&admin_handle_t, NULL, handle_admin, NULL) == 0){
                                close(ha);
                        }
               
                close(admin);
                }
        }
}

void erro(char *msg){
        printf("Erro: %s\n", msg);
    exit(-1);
}

void* handle_admin(){
        return NULL;
}

list read_dns(){
        list novo,aux;
        aux=(list)malloc(sizeof(dns_list));
        if(aux!=NULL) aux->next=NULL;
        FILE* fd=fopen("dns.dat","r");
        if(fd==NULL){
                erro("ao abrir o ficheiro dns.dat");
                return NULL;
        }
        while(1){                               
                novo=(list)malloc(sizeof(dns_list));
                if(fread((void*)novo,sizeof(dns_list),1,fd)!=1) break;
                novo->next=aux->next;
                aux->next=novo;
        }
        fclose(fd);
        return aux;
}

void write_log(char* user,char* query, char* objecto){
        sem_wait(&MUTEX);
        log log_dados;
        log_dados=(log)malloc(sizeof(log_list));
        struct tm *local;
        time_t t;
        t=time(NULL);
        local = localtime(&t);
        FILE* fl=fopen("log.dat","a");
        fprintf(fl,"%s\n",user);
        fprintf(fl,"%s\n",query);
        fprintf(fl,"%s\n",objecto);
        fprintf(fl,"%s\n",asctime(local));
        fclose(fl);
        sem_post(&MUTEX);
}

---

client.c

Code:

---

#include <stdio.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <netdb.h>

#define BUF_CLIENT 50

void erro(char *msg);

int main(int argc, char *argv[]) {
  char endServer[100];
  char ip[BUF_CLIENT];
  int fd;
  int nread;
  struct sockaddr_in addr;
  struct hostent *hostPtr;
  if (argc != 4) {
        printf("cliente <host> <port> <string>\n");
        exit(-1);
  }
  strcpy(endServer, argv[1]);
  if ((hostPtr = gethostbyname(endServer)) == 0)
        erro("Nao consegui obter endereco");
  bzero((void *) &addr, sizeof(addr));
  addr.sin_family = AF_INET;
  addr.sin_addr.s_addr = ((struct in_addr *)(hostPtr->h_addr))->s_addr;
  addr.sin_port = htons((short) atoi(argv[2]));
  if((fd = socket(AF_INET,SOCK_STREAM,0)) == -1)
        erro("socket");
  if( connect(fd,(struct sockaddr *)&addr,sizeof (addr)) < 0)
        erro("Connect");
  write(fd, argv[3], strlen(argv[3])+1);
  bzero(ip,BUF_CLIENT);
  nread = read(fd, ip, BUF_CLIENT-1);
  ip[nread]='\0';
  printf("IP: %s\n",ip);
  close(fd);
  return 0;
}
void erro(char *msg)
{
        printf("Erro: %s\n", msg);
        exit(-1);
}

---

Glad if you could help.

Why do you assume read worked? Check your return values before you start assuming they have something valid. You seem to have a bad habit of not checking your return values, and just plugging away. You assume your files open, you assume your reads work, etc. That's a bad thing to do, especially since you're trying to hunt down odd issues.

Maybe if you had done all those things first, you wouldn't be where you are now.



Quzah.

I haven't finished the program yet, I usually put that stuff at the final stages. I am checking the read values when reading from the buffer, didn't you see?
Yes, when it fails sometimes I get "-1" and other times I get "-1368939". The question is why?
The example code mine is based on is very similar and it works fine. The server ignores when the read value is < 0 but it never fails, I can keep sending data and it displays it.
The question here is why, after I have written stuff to the socket, the read returns a negative number.

I can't make it ignore the negative reads because then the client would get no response.

I don't suppose you're actually checking the error state either whenever you get a negative number that's odd?

Quote:

---

Originally Posted by guy with lots of problems

I haven't finished the program yet, I usually put that stuff at the final stages.

---

That's a lousy idea.

Quote:

---

Originally Posted by same guy

I am checking the read values when reading from the buffer, didn't you see?

---

Not everywhere:

Code:

---

void* handle_cliente(int client_fd){
        int nread,nwrite;
        char find[BUF_CLIENT];
    char buffer[BUF_CLIENT];
    bzero(buffer,BUF_CLIENT);
    bzero(find,BUF_CLIENT);
    nread = read(client_fd, buffer, BUF_CLIENT-1);
        buffer[nread-1] = '\0';
        printf("nread: %d",nread);
        strcpy(find,host_lookup(buffer));
        printf("\n.%s.\n",buffer);
        printf("tam:%d",strlen(find));
        find[strlen(find)]='\0';
        printf("\n.%s.\n",find);
        nwrite=write(client_fd,(void*)find, strlen(find));
        printf("\nnwrite:%d\n",nwrite);
        close(client_fd);
        return NULL;
}

---

You just assume that worked, and assume it's non-negative, and immediately try to access the buffer there without checking to see that you're actually accessing something valid.


Quzah.

The line after those two in red checks it :p

The error is bad file descriptor, I'm guessing something related to closing the socket is wrong, but I don't see where else I should put it..


A friend of mine corrected the problem for me, remove the close(client) from the cliente_connection() function.

Thanks anyway.

Quote:

---

Originally Posted by CYBERRRR

The line after those two in red checks it :p

---

You don't seem to understand the concept of checking things before you start playing around with their values. If read returns -13999 or whatever you said earlier, your line immediately after the read says:

Code:

---

foo[ -14000 ] = '\0';

---

Which is just begging for a crash.


Quzah.

when I said check I clearly meant I was making a printf to see the value, not checking "if ()".. Thanks