Code reviewers, I have changed code whatever be suggested by code reviewer and I want you to give me some feedback on the basis of my new code.
I have written TCP server client code to send data of any size (1 byte - 200KB) in a loop.
The client (A) sends data to the server (B). B then sends the same data back to A. A matches the sent data to the data received. If the data does not match, control flow will break from the while-loop. If the data matches, the loop will continue to 200K bytes and then break successfully.
This question is a follow-up question on Send and Receive more than 64KiB over TCP/IP socket
Server code (server.c):
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <sys/types.h>
#include <unistd.h>
#include <sys/select.h>
#include <errno.h>
#include <sys/socket.h>
#include <arpa/inet.h>
#include <netinet/tcp.h>
#include <stdbool.h>
#include "init.h"
#define MAX_CLIENT 20000
#define SIZE 220000
int server_listen(int argc, char **argv);
int main(int argc, char *argv[])
{
int ret, accept_desc, n, ssd;
unsigned int bytes = 1;
unsigned char *buf;
fd_set rfd;
ssd = server_listen(argc, argv); // --- return fd from listen()
n = ssd;
FD_ZERO(&rfd); // --- Clears a set
FD_SET(ssd, &rfd); // --- Add a given file descriptor from a set
while (true) {
FD_ZERO(&rfd);
FD_SET(n, &rfd);
ret = select(n + 1, &rfd, NULL, NULL, NULL); // --- Waiting until one or more of the file descriptors become "ready"
if (ret < 0) {
perror("Error in select\n");
close(n);
exit(EXIT_FAILURE);
}
if (FD_ISSET(ssd, &rfd)) { // --- Tests to see if a file descriptor is part of the set
accept_desc = server_accept(ssd); // ---Accept connection from client
fprintf(stderr, "client %d is connected\n", accept_desc);
if (n < accept_desc)
n = accept_desc;
}
if (FD_ISSET(accept_desc, &rfd)) {
bytes++;
buf = receive(accept_desc, bytes); // --- Receive buffer from client
if(buf == NULL) {
fprintf(stderr, "Memory not allocated\n");
close(accept_desc);
exit(EXIT_FAILURE);
}
send_str(accept_desc, buf, bytes); // --- Send received buffer to client
fprintf(stderr, "sent %d\n", bytes);
free(buf); // --- Free the memory
}
}
}
int server_listen(int argc, char **argv)
{
int ssd, flag = 1, ret, portno;
unsigned int addrlen;
struct sockaddr_in serv_addr;
portno = get_option(argc, argv); // --- Get portno from user using getopt
ssd = socket(AF_INET, SOCK_STREAM, 0); // --- socket creates an endpoint for communication
if (ssd < 0) {
perror("Error in socket");
exit(EXIT_FAILURE);
}
ret = setsockopt(ssd, IPPROTO_TCP, TCP_NODELAY,
(char *) &flag, sizeof(int)); // --- TCP_NODELAY used to disable nagle's algorithm
if (ret != 0) {
perror("Erro in tcp_nodelay");
close(ssd);
exit(EXIT_FAILURE);
}
ret = setsockopt(ssd, SOL_SOCKET, SO_REUSEADDR,
(char *) &flag, sizeof(int)); // --- SO_REUSEADDR to reuse same port
if (ret != 0) {
perror("Error in reuseAddr");
close(ssd);
exit(EXIT_FAILURE);
}
serv_addr.sin_family = AF_INET;
serv_addr.sin_addr.s_addr = INADDR_ANY;
serv_addr.sin_port = htons((uint16_t)portno);
bzero(&serv_addr.sin_zero, sizeof(serv_addr.sin_zero));
addrlen = sizeof(serv_addr);
ret = bind(ssd, (struct sockaddr *)&serv_addr, addrlen); // --- Bind assigns the address specified by addr to socket
if (ret < 0) {
perror("Error in binding");
close(ssd);
exit(EXIT_FAILURE);
}
fprintf(stderr, "Bind completed\n");
//Listen
ret = listen(ssd, MAX_CLIENT); // --- ready to accept incoming connection requests using accept()
if (ret < 0) {
perror("Error in listen");
close(ssd);
exit(EXIT_FAILURE);
}
fprintf(stderr, "listen\n");
return ssd;
}
Client code (client.c):
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/socket.h>
#include <netdb.h>
#include <errno.h>
#include <sys/select.h>
#include <unistd.h>
#include <time.h>
#include <sys/types.h>
#include <netinet/tcp.h>
#include "init.h"
#define KB 1024
#define NO_EXIT_OR_ERROR 1
#define STATE_SEND 0
#define STATE_RECEIVE 1
unsigned int bytes;
struct hostent *server;
int client_connect(int argc, char **argv);
unsigned char *fill_n_send(int fd);
void receive_n_compare(int fd, unsigned char *buf_send);
int main(int argc, char *argv[])
{
int fd, state = STATE_SEND, ret;
struct timeval tv;
unsigned char *buf_send;
fd_set rfd;
fd = client_connect(argc, argv); // --- Return fd from connect()
tv.tv_sec = 0;
tv.tv_usec = 100;
FD_ZERO(&rfd); // --- Clears a set
FD_SET(fd, &rfd);
while(NO_EXIT_OR_ERROR) {
FD_ZERO(&rfd);
FD_SET(fd, &rfd); // --- Add a given file descriptor from a set
ret = select(fd + 1, &rfd, NULL, NULL, &tv);
if(ret < 0) { // --- Waiting until one or more of the file descriptors become "ready"
perror("Select error");
close(fd);
exit(EXIT_FAILURE);
}
if(state == STATE_SEND) {
buf_send = fill_n_send(fd);
state = STATE_RECEIVE;
}
if (FD_ISSET(fd, &rfd)) { // --- Tests to see if a file descriptor is part of the set
receive_n_compare(fd, buf_send);
state = STATE_SEND;
}
}
}
unsigned char *fill_n_send(int fd)
{
unsigned int start = 0;
unsigned char *buf_send;
buf_send = malloc(bytes + 2);
if(buf_send == NULL) {
fprintf(stderr, "Memory not allocated\n");
close(fd);
exit(EXIT_FAILURE);
}
while(start <= bytes){ // --- fill send buf like A, AA, AAA, AAAA ..... upto 200KB size and send one by one like first A then AA then AAA ...
buf_send[start] = 'A';
start++;
}
buf_send[++bytes] = 0;
if(bytes >= 200*KB) {
free(buf_send);
close(fd);
exit(EXIT_SUCCESS);
}
send_str(fd, buf_send, bytes + 1);
return buf_send;
}
void receive_n_compare(int fd, unsigned char *buf_send)
{
unsigned char *buf_recv;
buf_recv = receive(fd, bytes + 1);
if(buf_recv == NULL) {
fprintf(stderr, "Memory not allocated\n");
free(buf_send);
close(fd);
exit(EXIT_FAILURE);
}
if(strcmp((const char*)buf_send, (const char*)buf_recv) == 0){
fprintf(stderr, "Match\n");
} else {
fprintf(stderr, "Unmatch %ld %ld\n", strlen((const char*)buf_send), strlen((const char*)buf_recv));
close(fd);
exit(EXIT_FAILURE);
}
free(buf_send);
free(buf_recv);
}
int client_connect(int argc, char **argv)
{
int fd1, ret, flag = 1, portno;
struct sockaddr_in serv_addr;
portno = get_option(argc, argv); // --- Get portno from user using getopt
if(server == NULL) { // --- Check whether receive ip address of server
fprintf(stderr, "Assign IP\n");
exit(EXIT_FAILURE);
}
fd1 = socket(AF_INET, SOCK_STREAM, 0); // --- socket creates an endpoint for communication
if (fd1 == -1) {
perror("ERROR on creating Socket\n");
exit(EXIT_FAILURE);
}
fprintf(stderr, "%d\n", fd1);
fprintf(stderr, "socket created successfully.\n");
bzero((char *) &serv_addr, sizeof(serv_addr));
serv_addr.sin_family = AF_INET;
bcopy((char *) server->h_addr, (char *)&serv_addr.sin_addr.s_addr,
(long unsigned int)server->h_length);
serv_addr.sin_port = htons((uint16_t)portno);
ret = setsockopt(fd1, IPPROTO_TCP, TCP_NODELAY,
(char *) &flag, sizeof(int)); // --- TCP_NODELAY used to disable nagle's algorithm
if (ret != 0) {
perror("Error in setsockopt");
close(fd1);
exit(EXIT_FAILURE);
}
if (connect(fd1, (struct sockaddr *)&serv_addr, sizeof(serv_addr)) == -1) { // --- Connect the socket to the server
perror("ERROR in Connecting");
close(fd1);
exit(EXIT_FAILURE);
}
printf("Client connected Successfully\n");
return fd1;
}
Initial definition (init.c):
#include "init.h"
#include <stdio.h>
#include <sys/socket.h>
#include <sys/types.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <errno.h>
#include <netdb.h>
#include <stdbool.h>
#define SIZE 220000
struct hostent *server;
unsigned char *receive(int csd, unsigned int size)
{
long int ret;
unsigned char *buf = malloc(size), *buf1 = NULL;
if(buf != NULL) {
memset(buf, 0,size);
} else {
fprintf(stderr, "Memory not allocated\n");
close(csd);
exit(EXIT_FAILURE);
}
while(true) {
ret = recv(csd, buf, size, 0);
if(buf[ret-1] > 0) {
buf1 = malloc(size);
if(buf1 != NULL) {
memset(buf1, 0,size);
} else {
fprintf(stderr, "Memory not allocated\n");
free(buf);
close(csd);
exit(EXIT_FAILURE);
}
strcpy((char*)buf1, (const char*)buf);
memset(buf, 0, size);
continue;
}
if (ret < 0) {
perror("Error in receive\n");
close(csd);
exit(EXIT_FAILURE);
} else if (ret == 0) {
fprintf(stderr, "client disconnected\n");
close(csd);
} else {
if(buf1 != NULL) {
strcat((char*)buf1, (const char*)buf);
free(buf);
return buf1;
}
return buf;
}
}
}
void send_str(int csd, unsigned char *buf, unsigned int size)
{
long int ret;
ret = send(csd, buf, size, 0);
if (ret < 0) {
perror("Error in send\n");
close(csd);
exit(EXIT_FAILURE);
} else if(ret == 0) {
fprintf(stderr, "Nothing to send\n");
close(csd);
}
}
int server_accept(int ssd)
{
int accept_desc;
struct sockaddr_in cli_addr;
socklen_t cli_addr_len;
accept_desc = accept(ssd, (struct sockaddr *)&cli_addr,
(socklen_t *)&cli_addr_len);
if (accept_desc < 0) {
perror("Error in accept\n");
close(ssd);
exit(EXIT_FAILURE);
}
return accept_desc;
}
int get_option(int argc, char **argv)
{
int opt, portno = 0;
while ((opt = getopt(argc, argv, "p:i:")) != -1) {
switch (opt) {
case 'p':
portno = atoi(optarg);
memset(&optarg, 0, strlen(optarg));
break;
case 'i':
server = gethostbyname(optarg);
memset(&optarg, 0, strlen(optarg));
break;
default: /* '?' */
fprintf(stderr, "Usage: %s\n", argv[0]);
exit(EXIT_FAILURE);;
}
}
if (portno == 0) {
fprintf(stderr, "Assign port number\n");
exit(EXIT_FAILURE);
}
printf("portno = %d optind = %d\n", portno, optind);
return portno;
}
Initial declaration (init.h):
#ifndef INIT_H
#define INIT_H
int get_option(int argc, char *argv[]);
unsigned char *receive(int csd, unsigned int size);
void send_str(int csd, unsigned char *buf, unsigned int size);
int server_accept(int ssd);
#endif // INIT_H
Makefile:
.PHONY: all server1 client1
all: server1 client1
server1: server.c init.c
gcc -Wall -Wextra -Wconversion -pedantic -std=gnu11 server.c init.c -o s
client1: client.c init.c
gcc -Wall -Wextra -Wconversion -pedantic -std=gnu11 client.c init.c -o c
server:
./s -p 13000
client:
./c -p 13000 -i 127.0.0.1