Few problems with my program

I am (trying) to write a bit of a webserver and I have made progress but although it is basicaly working I am getting a load of wierd problems.

When I open a connection to the server in telnet and type "testp" and then "test" it will print "testp" both times, but this does not happen reliably, it only seems to happen when going from a >4 character word to a 4 character word, it doesn't happen with 1 2 3 letter words etc, very odd!

The "free(html_command);" with the BOB printf's around it is sometimes returning a message like this "free(): invalid pointer 0x804b030!" which I don't understand. Its not constant, when I connect to the program via telnet and type things it produces the free error sometimes but not most of the time and I see no pattern in it. I also can't see the problem in the code as that is the only place where that variable is freed so surely if its not NULL there shouldn't be a problem, and I check that its not NULL!

When I start spamming the thing by typing alot (which I guess is like recieving lots of tiny messages) it seg faults. Unfortunately gdb doesn't seem to say anything handy:
"Program received signal SIGSEGV, Segmentation fault.
0x4008f346 in mallopt () from /lib/tls/libc.so.6"

When I point my web-browser at the program it recievs the first line fine but then replaces the other lines with the Ø symbol.

I suspect the problem is with the code that copys the recieved stuff between the buffer, the sockets buffer and the html_command but I am not really sure so I will paste the whole lot incase I miss off somthing important.

Code:

---

#include <stdio.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <stdbool.h>

#define PORT 1337
#define BACKLOG 10

/* Function delarations */
int main(int argc, char *argv[]);
int init();
void get_input();

/* Global variables */
int sin_size;
int fdmax; /* The largest file descriptor number being used */
struct sockaddr_in my_address; /* Server address */
struct sockaddr_in their_address; /* Client address */
int listener_fd, new_fd; /* Listening socket and newly made sockets */
fd_set master_fd_set;    /* Master file descriptor list */
fd_set read_fd_set;      /* Temp list for select() */

struct Ssocket
{
  int fd; /* File descriptor */
  char *buffer;
  int buffer_length;
  struct Ssocket *next;
  struct Ssocket *prev;
};

struct Ssocket *first_socket;

char *html_command;
int html_command_size;

int main(int argc, char *argv[])
{
  bool running = false;
  int i;
 
  /* Initialize stuff */
  if (init() == -1)
    {
        printf("Failed to init.\n");
        exit(1);
    }
  else
    running = true;
 
  /* Listen for connections */
  if (listen(listener_fd, BACKLOG) == -1)
    {
        printf("Problem listening for connections\n");
        exit(1);
    }
 
  FD_SET(listener_fd, &master_fd_set); /* Add the new socket to the set */
  fdmax = listener_fd; /* set the biggest fd as the current one */
 
  while (running == true)
        get_input(); 
       
  /* Tidy up before we go */
  close(new_fd);
  for (i = 0; i <= fdmax; i++)
    close(i);
  FD_ZERO(&master_fd_set);
  FD_ZERO(&read_fd_set);
  close(listener_fd);
}

int init()
{ 
  /* Start the socket file descriptor */
  if ((listener_fd = socket(AF_INET, SOCK_STREAM, 0)) == -1)
    {
        printf("Couldn't open the socket!\n");
        return(-1);
    }
 
  /* Assign the stuff */
  my_address.sin_family = AF_INET;
  my_address.sin_port = htons(PORT);    /* Set the port to the one defined */
  my_address.sin_addr.s_addr = htonl(INADDR_ANY);  /* Set the sockets IP to local */
  memset(&(my_address.sin_zero), '\0', 8);    /* Zero the rest */
 
  /* Bind the socket to the port */
  if (bind(listener_fd, (struct sockaddr *)&my_address, sizeof(struct sockaddr)) == -1)
  {
        printf("Couldn't bind the socket's port!\n");
        return(-1);
  }
 
  /* Figure this out to save complication later */
  sin_size = sizeof(struct sockaddr_in);
 
  /* Zero the socket sets */
  FD_ZERO(&master_fd_set); /* Clear the */
  FD_ZERO(&read_fd_set);  /* sets      */
 
  /* Reset the html command */
  html_command = NULL;
  html_command_size = 0;
 
  return 0;
}

void get_input()
{
  char buffer[100];
  int i, j, k, addrlen, nbytes;
  struct Ssocket *current_socket;
  struct Ssocket *new_socket;
  struct Ssocket *socket;
 
  read_fd_set = master_fd_set; /* Copy it */
 
  if (select(fdmax + 1, &read_fd_set, NULL, NULL, NULL) == -1)
    {
        printf("Problem with the socket set select watsit\n");
        exit(1);
    }
 
  for (i = 0; i <= fdmax; i++)
    if (FD_ISSET(i, &read_fd_set)) /* loop through each socket */
      if (i == listener_fd) /* Its a new connection */
        {
            addrlen = sizeof(their_address);
            if ((new_fd = accept(listener_fd, (struct sockaddr *)&their_address, &addrlen)) == -1)
              {
                printf("Problem accepting the connection");
                exit(1);
              }
            else
              {
              FD_SET(new_fd, &master_fd_set);
                if (new_fd > fdmax)
                  fdmax = new_fd; /* Keep the max the max */
               
                /* Add socket's structure to the linked list */
                new_socket = malloc(sizeof(struct Ssocket));
                new_socket->fd = new_fd;
                new_socket->buffer = NULL;
                new_socket->buffer_length = 0;
               
                new_socket->next = first_socket;
                if (first_socket != NULL)
                  first_socket->prev = new_socket;
                first_socket = new_socket;
               
                printf("New connecton (%d) from %s\n", new_fd, inet_ntoa(their_address.sin_addr));
              }   
        }
  else /* Its data from an already connected client */
    {
        nbytes = recv(i, buffer, sizeof(buffer), 0);
       
        /* Get the pointer of the socket structure */
        current_socket = first_socket;
        while (current_socket != NULL)
          {
            if (current_socket->fd == i)
              {
                  socket = current_socket;
                  break;
              }
           
            current_socket = current_socket->next;
          }
       
        /* Reset the html command size */
        html_command_size = 0;
       
        switch (nbytes)
          {
          case 0:
            printf("Socket quit (%d)\n", i);
            close(i);
            FD_CLR(i, &master_fd_set);
           
            /* Clean up the socket structure */
            if (socket->next != NULL)
              socket->next->prev = socket->prev;
            if (socket->prev != NULL)
              socket->prev->next = socket->next;
            if (socket == first_socket)
              first_socket = first_socket->next;
            if (socket->buffer != NULL)
              free(socket->buffer);
            free(socket);
            break;
          case -1:
            /* Could maybe just ignore instead of kicking */
            printf("Error recieving so kicking client\n");
            close(i);
            FD_CLR(i, &master_fd_set);
           
            /* Clean up the socket structure */
            if (socket->next != NULL)
              socket->next->prev = socket->prev;
            if (socket->prev != NULL)
              socket->prev->next = socket->next;
            if (socket == first_socket)
              first_socket = first_socket->next;
            if (socket->buffer != NULL)
              free(socket->buffer);
            free(socket);
            break;         
          default:
            /* Allocate space in the sockets buffer */
            if (socket->buffer_length <= 0)
              socket->buffer = malloc(nbytes + 1);
            else
              socket->buffer = realloc(socket->buffer, socket->buffer_length + nbytes + 1);
           
            /* Put the recieved guff in the sockets buffer */
            for (j = socket->buffer_length; j < socket->buffer_length + nbytes; j++)
              socket->buffer[j] = buffer[j - socket->buffer_length];
            socket->buffer[j + 1] = '\0';
           
            socket->buffer_length += nbytes;
           
            /* printf("%s\n",socket->buffer); */
           
            for (j = 0; j < socket->buffer_length; j++)
              {
                  if (socket->buffer[j] == '\n' || socket->buffer[j] == '\r')
                    {
                      /* Get the html command buffer the right size */
                      html_command_size = j;
                      printf("BOB\n");
                      if (html_command != NULL)
                        free(html_command);
                      printf("BOBB\n");
                      html_command = malloc(j + 1);
                     
                      /* Copy the stuff into the html command bufer */
                      for (k = 0; k < j; k++)
                        html_command[k] = socket->buffer[k];
                      html_command[k + 1] = '\0';
                     
                      /* Remove the dealt with stuff from the sockets buffer */
                      socket->buffer_length -= (j + 2); /* + 2 to get rid of the '\n' and stuff */
                     
                      if (socket->buffer_length <= 0)
                        {
                            free(socket->buffer);
                            socket->buffer = NULL;
                        }
                      else
                        {
                            for (k = 0; k < socket->buffer_length; k++)
                              socket->buffer[k] = socket->buffer[k+j];
                            socket->buffer[k+1] = '\0';
                            socket->buffer = realloc(socket->buffer, socket->buffer_length);
                        }
             
                      printf(".%s.\n", html_command);
                      break;
                    }
              }
            break;
          }
    }
}

---

Thanks

I don't know what's going on, but the free() error message sounds like you're not allocating a pointer that you're trying to free.

Quote:

---

Originally Posted by dwks

I don't know what's going on, but the free() error message sounds like you're not allocating a pointer that you're trying to free.

---

I thought it sounded somthing like that but I don't see how i am :(

> new_socket->next = first_socket;
Yeah, and ->prev is left as garbage - not good when you come around next time.

Also, your linked list delete code assumes WAY too much - the ends of the list need particular attention when you start doing stuff like this.
socket->prev->next = socket->next;

Create separate linked list functions to
- allocate
- insert
- remove
which you test separately and make sure they do the right thing in all cases.

> socket->buffer = realloc(socket->buffer, socket->buffer_length + nbytes + 1);
Classic realloc bug
If the allocation fails, you lose the memory you had anyway.
Use a temp variable which you then test, and only if it's != NULL do you assign it to your proper variable.

Again - code pollution.
You should be wrapping your self-expanding string up in some functions which do some of the work for you rather than piece-meal inlining it in the code.

Ok I have tried to fix the things you pointed out saleem. Although I don't see the problem with my code to remove items from the linked list.

Im happier with the code now but it still has all the odd problems that I decribed before :(

Anyway here it is

edit: The find_html_command function is returning a blank string now :(.

Code:

---

#include <stdio.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <stdbool.h>

#define PORT 1337
#define BACKLOG 10

/* Structure declarations */
struct Ssocket
{
  int fd; /* File descriptor */
  char *buffer;
  int buffer_length;
  struct Ssocket *next;
  struct Ssocket *prev;
};

/* Function delarations */
int main(int argc, char *argv[]);
int init();
void get_input();
struct Ssocket* add_socket(int fd);
void remove_socket(struct Ssocket *socket);
int update_buffer(struct Ssocket *socket, char *buffer, int nbytes);
char* find_html_command(struct Ssocket *socket);

/* Global variables */
int sin_size;
int fdmax; /* The largest file descriptor number being used */
struct sockaddr_in my_address; /* Server address */
struct sockaddr_in their_address; /* Client address */
int listener_fd; /* Listening  */
fd_set master_fd_set;    /* Master file descriptor list */
fd_set read_fd_set;      /* Temp list for select() */
struct Ssocket *first_socket; /* Pointer to first socket in the linked list */

int main(int argc, char *argv[])
{
  bool running = false;
  int i;
  char *html_command;
  struct Ssocket *current_socket;
 
  /* Initialize stuff */
  if (init() == -1)
    {
        printf("Failed to init.\n");
        exit(1);
    }
  else
    running = true;
 
  /* Listen for connections */
  if (listen(listener_fd, BACKLOG) == -1)
    {
        printf("Problem listening for connections\n");
        exit(1);
    }
 
  FD_SET(listener_fd, &master_fd_set); /* Add the new socket to the set */
  fdmax = listener_fd; /* set the biggest fd as the current one */
 
  while (running == true)
    {
        /* Check the socket set for input */
        get_input(); 
       
        /* Check the socket's buffers for whole commands */
        current_socket = first_socket;
        while (current_socket != NULL)
          {
            while ((html_command = find_html_command(current_socket)) != NULL)
              {
                  printf("q%s\n",html_command);
                  free(html_command);
              }
           
            current_socket = current_socket->next;
          }
    }
 
  /* Tidy up before we go */
  /* close(new_fd); */
  for (i = 0; i <= fdmax; i++)
    close(i);
  FD_ZERO(&master_fd_set);
  FD_ZERO(&read_fd_set);
  close(listener_fd);
}

int init()
{
  /* Start the socket file descriptor */
  if ((listener_fd = socket(AF_INET, SOCK_STREAM, 0)) == -1)
    {
        printf("Couldn't open the socket!\n");
        return(-1);
    }
 
  /* Assign the stuff */
  my_address.sin_family = AF_INET;
  my_address.sin_port = htons(PORT);    /* Set the port to the one defined */
  my_address.sin_addr.s_addr = htonl(INADDR_ANY);  /* Set the sockets IP to local */
  memset(&(my_address.sin_zero), '\0', 8);    /* Zero the rest */
 
  /* Bind the socket to the port */
  if (bind(listener_fd, (struct sockaddr *)&my_address, sizeof(struct sockaddr)) == -1)
    {
        printf("Couldn't bind the socket's port!\n");
        return(-1);
    }
 
  /* Figure this out to save complication later */
  sin_size = sizeof(struct sockaddr_in);
 
  /* Zero the socket sets */
  FD_ZERO(&master_fd_set); /* Clear the */
  FD_ZERO(&read_fd_set);  /* sets      */
 
  return 0;
}

void get_input()
{
  char buffer[100];
  int i, addrlen, nbytes, new_fd;
  struct Ssocket *current_socket;
  struct Ssocket *new_socket;
  struct Ssocket *socket;
 
  read_fd_set = master_fd_set; /* Copy it */
 
  if (select(fdmax + 1, &read_fd_set, NULL, NULL, NULL) == -1)
    {
        printf("Problem with the socket set select watsit\n");
        exit(1);
    }
 
  for (i = 0; i <= fdmax; i++)
    if (FD_ISSET(i, &read_fd_set)) /* loop through each socket */
      if (i == listener_fd) /* Its a new connection */
        {
            addrlen = sizeof(their_address);
            if ((new_fd = accept(listener_fd, (struct sockaddr *)&their_address, &addrlen)) == -1)
              {
                printf("Problem accepting the connection");
                exit(1);
              }
            else
              {
                /* Add the socket */
                if ((new_socket = add_socket(new_fd)) == NULL)
                  exit(1);
                else
                  printf("New connecton (%d) from %s\n", new_fd, inet_ntoa(their_address.sin_addr));
              }   
        }
  else /* Its data from an already connected client */
    {
        nbytes = recv(i, buffer, sizeof(buffer), 0);
       
        /* Get the pointer of the socket structure */
        current_socket = first_socket;
        while (current_socket != NULL)
          {
            if (current_socket->fd == i)
              {
                  socket = current_socket;
                  break;
              }
           
            current_socket = current_socket->next;
          }
       
        switch (nbytes)
          {
          case 0:  /* Client quit*/
            remove_socket(socket);
            printf("Socket quit (%d)\n", i);
            break;
          case -1:  /* Error recieving */
            remove_socket(socket);
            printf("Error recieving so kicking (%d)\n", i);
            break;         
          default:  /* Incoming stuff */
            /* Add the incoming stuff to the sockets buffer */
            if (update_buffer(socket, buffer, nbytes) == -1)
              exit(1);
            break;
          }
    }
}

struct Ssocket* add_socket(int fd)
{
  struct Ssocket *new_socket;
 
  /* Sort out the actual socket */
  FD_SET(fd, &master_fd_set);
  if (fd > fdmax)
    fdmax = fd; /* Keep the max the max */
 
  /* Allocate memory */
  if ((new_socket = malloc(sizeof(struct Ssocket))) == NULL)
    {
        printf("Could not allocate memory for new socket structure\n");
        return NULL;
    }
 
  /* Assign the data */
  new_socket->fd = fd;
  new_socket->buffer = NULL;
  new_socket->buffer_length = 0;
 
  /* Sort out the linked list pointers */
  new_socket->next = first_socket;
  new_socket->prev = NULL;
  if (first_socket != NULL)
    first_socket->prev = new_socket;
  first_socket = new_socket;
 
  return new_socket;
}

void remove_socket(struct Ssocket *socket)
{
  /* Close the socket and remove it from the set */
  close(socket->fd);
  FD_CLR(socket->fd, &master_fd_set);
 
  /* Sort out the linked list pointers */
  if (socket->next != NULL)
    socket->next->prev = socket->prev;
  if (socket->prev != NULL)
    socket->prev->next = socket->next;
  if (socket == first_socket)
    first_socket = first_socket->next;
 
  /* Free up the stuff */
  if (socket->buffer != NULL)
    free(socket->buffer);
  free(socket);
}

int update_buffer(struct Ssocket *socket, char *buffer, int nbytes)
{
  int i;
  char *temp;
 
  /* Allocate space in the sockets buffer */
  if (socket->buffer_length <= 0)
    {
        if ((socket->buffer = malloc(nbytes+1)) == NULL)
          {
            printf("Error allocating space for socket's buffer\n");
            return(-1);
          }
        else
          {
            if ((realloc(socket->buffer, socket->buffer_length + nbytes + 1)) == NULL)
              {
                  printf("Error reallocating the sockets buffer\n");
                  return(-1);
              }
            else
              socket->buffer = temp;
          }
    }
 
  /* Put the recieved guff in the sockets buffer */
  for (i = socket->buffer_length; i < socket->buffer_length + nbytes; i++)
    socket->buffer[i] = buffer[i - socket->buffer_length];
  socket->buffer[i + 1] = '\0';
 
  socket->buffer_length += nbytes;
 
  return 0;
}

char* find_html_command(struct Ssocket *socket)
{
  char *html_command, *temp;
  int i, html_command_size;
 
  html_command_size = -1;

  /* Find the first new line */
  for (i = 0; i < socket->buffer_length; i++)
    {
        if (socket->buffer[i] == '\n' || socket->buffer[i] == '\r')
          {
            html_command_size = i; 
            break;
          }
    }
 
  /* If no new lines found return NULL, otherwise deal with it */
  if (html_command_size == -1)
    return NULL;
 
  /* Get the html command buffer the right size */
  if ((html_command = malloc(html_command_size + 1)) == NULL)
    return NULL;
 
  /* Copy the stuff into the html command bufer */
  for (i = 0; i < html_command_size; i++)
    {
        printf(".%c\n", socket->buffer[i]);
        html_command[i] = socket->buffer[i];
    } 
  html_command[i + 1] = '\0';
  printf(",%s,/n", html_command);
  /* Remove the dealt with stuff from the sockets buffer */
  socket->buffer_length -= (html_command_size + 2); /* + 2 to get rid of the '\n' and stuff */
 
  if (socket->buffer_length <= 0)
    {
        free(socket->buffer);
        socket->buffer = NULL;
    }
  else
    {
        for (i = 0; i < socket->buffer_length; i++)
          socket->buffer[i] = socket->buffer[i+html_command_size];
        socket->buffer[i+1] = '\0';
       
        if ((realloc(socket->buffer, socket->buffer_length)) == NULL)
          {
            printf("Error reallocating the sockets buffer\n");
            exit(1);
          }
        else
          socket->buffer = temp;
    }
  return html_command;
}

---

Quote:

---

The find_html_command function is returning a blank string now .

---

I would think that socket->buffer_length is 0.

nah I fixed that problem. (the temp's wernt actualy allocated when i call realloc!) I tried to edit the code on this post but it didn't give me the option for some reason :o

I have sorted out the structure of the code a bit more so it makes more sence too. Still isn't bug free though :( . The free warning has gone but all the other bugs I described to start with are still there.

anyhoo this post is already spammed enough with code so I guess the best thing to do is just link to it: uoircbot.c