C beginner: searching in structs (dynamically allocated)

[Yourko]

Hello! I ame working on a hobby-project - a simple TCP/IP server. Its purpose (for now) is to act like a chat program: send all received data from one connected client to all others.

First step is to listen on port:

void server_init(void){
int main_socket;
main_socket = create_socket(port);


/* Two filedescriptor sets for use with select() */
fd_set before; // Before calling select()
fd_set after; // Modified by select()

FD_ZERO(&before);
FD_ZERO(&after);

/* Add main socket to the 'before' fd set */
FD_SET(main_socket, &before);

int fd;

listen(main_socket, 10); // Why 10? I dont know

while(1){
after = before;
if( select(FD_SETSIZE, &after, NULL, NULL, NULL) < 0){
perror("select");
exit(1);
}

for(fd = 0; fd < FD_SETSIZE; ++fd){ /* Is using here FD_SETSIZE a good idea? */
if(FD_ISSET(fd, &after)){
if(fd == main_socket){
/* new connection */
int new_fd = client_new(fd);
...

Next step is to accept connections.
In globals.h file i defined a structure named client (guys at comp.lang.c helped me with pointer stuff ) :

struct _client{
char * name;
int namelen;
int fd; // file descriptor
struct sockaddr_in sock_name;
} * * client;

and a client_count variable:

int client_count;

Here is function client_new():

int client_new(int filedes){ // file descriptor the client is connected on

if(client_count == 0){
/* first client connected */
client = calloc(1, sizeof(*client));
client[0] = calloc(1, sizeof(*client[0]));
client_count = 1;
}
else{
/* already have people connected */
++client_count;
struct _client * tmp;
if( (tmp = realloc(client, client_count * sizeof(*client[0])) ) == 0){
fprintf(stderr, "Can`t allocate memory for new client");
exit(1);
}
client = tmp;
free(tmp);

client[client_count - 1] = calloc(1, sizeof(*client[client_count - 1]));

}
int new_filedes;
int addrlen = sizeof(client[client_count - 1]->sock_name);
new_filedes = accept(filedes, (& client[client_count - 1]->sock_name), &addrlen);
if(new_filedes < 0){
perror("accept");
exit(1);
}
if(debug) printf("client[%d]->fd = %d\n", client_count - 1, new_filedes);


/* Give this client a name */

int namelen;
send(new_filedes, "server: login please: ", 24, 0);
namelen = recv(new_filedes, buffer, BUFSIZE, 0);
client[client_count - 1]->name = malloc(sizeof(char) * namelen + 1);
memcpy(client[client_count - 1]->name, buffer, namelen * sizeof(char));

if(client[client_count - 1]->name[namelen] != "\0"){
client[client_count - 1]->name[namelen] = "\0";
}

client[client_count - 1]->fd = new_filedes;
client[client_count - 1]->namelen = namelen;

return(new_filedes);
}

Next step is to deliver received data to connected people (except sender).
I want to add to received string name of the client-sender, but to do this i must know which client talks to me. This is client_search(), which takes client file descriptor as argument and returns client number:

int client_search(int filedes){
int i;
i = 0;
while(i < (client_count-1)){
if(client->fd == filedes) return(i); // Here my program crashes, Segmentation fault
++i;
}
}

I think i made a mistake in client_new() function on memory allocation, but can't find it...
Any suggestions?

 

[David Resnick]

Hello! I ame working on a hobby-project - a simple TCP/IP server. Its purpose (for now) is to act like a chat program: send all received data from one connected client to all others.

Lots of the stuff in your program is not really on topic here.
Fortunately your problem is.

client = tmp;
free(tmp);

client[client_count - 1] = calloc(1, sizeof(*client[client_count - 1]));

Look carefully at those lines of code and a problem may emerge.
Freeing something followed by using it is a bad idea. There may be
other problems,
but this popped out to me.

-David

 

[CBFalconer]

Hello! I ame working on a hobby-project - a simple TCP/IP server.
Its purpose (for now) is to act like a chat program: send all
received data from one connected client to all others.

.... snip ...

Since you diligently lurked here, read the FAQ, and became familiar
with netiquette in general, you must be aware that your post has
nothing to do with the standard C language, and that your lines
shockingly exceed the recommended maximum, etc. Thus you obviously
do not expect to ever receive a useful answer, and are simply
trying to annoy.

--
"If you want to post a followup via groups.google.com, don't use
the broken "Reply" link at the bottom of the article. Click on
"show options" at the top of the article, then click on the
"Reply" at the bottom of the article headers." - Keith Thompson
More details at: <http://cfaj.freeshell.org/google/>
Also see <http://www.safalra.com/special/googlegroupsreply/>

 

[pete]

Yourko wrote:

Next step is to accept connections.
In globals.h file i defined a structure named client
(guys at comp.lang.c helped me with pointer stuff ) :

struct _client{
char * name;
int namelen;
int fd; // file descriptor
struct sockaddr_in sock_name;
} * * client;

and a client_count variable:

int client_count;

I don't know how much of the following is in the h file,
but as for the previous,
it's bad style to define objects and or functions in h files,
because it's undefined behavior
if more than one C file includes that h file.

Here is function client_new():

int client_new(int filedes){ // file descriptor the client is connected on

if(client_count == 0){
/* first client connected */
client = calloc(1, sizeof(*client));

What happens if calloc returns a null pointer?

client[0] = calloc(1, sizeof(*client[0]));
client_count = 1;
}
else{
/* already have people connected */
++client_count;
struct _client * tmp;
if( (tmp = realloc(client, client_count * sizeof(*client[0])) ) == 0){
fprintf(stderr, "Can`t allocate memory for new client");
exit(1);
}
client = tmp;
free(tmp);

client[client_count - 1] = calloc(1, sizeof(*client[client_count - 1]));

David Resnick caught that one.
That was a good catch.