patterncMinor
C Version of a Client/Server application
Viewed 0 times
versionclientapplicationserver
Problem
Starting to write an article about Socket Programing.
So I need a simple C version of a client/server app.
So here it is for review (Also on github)
A linked question is the C++ version
MakeFile
Common.h
common.cpp
```
#include "common.h"
#include
#include
#include
#include
char const* failedSocket = "Failed: socket()";
char const* failedBind = "Failed: bind()";
char const* failedListen = "Failed: listen()";
char const* failedConnect= "Failed: connect()";
char const* socketError = "None";
int serverSocket()
{
int socketId = socket(PF_INET, SOCK_STREAM, 0);
if (socketId == -1)
{
socketError = failedSocket;
return -1;
}
struct sockaddr_in serverAddr;
bzero((char*)&serverAddr, sizeof(serverAddr));
serverAddr.sin_family = AF_INET;
serverAddr.sin_port = htons(8080);
serverAddr.sin_addr.s_addr = INADDR_ANY;
if (bind(socketId, (struct sockaddr *) &serverAddr, sizeof(serverAddr)) != 0)
{
socketError = failedBind;
close(socketId);
return -1;
}
if(listen(socketId, 5) != 0)
{
socketError = failedListen;
close(socketId);
return -1;
}
return socketId;
}
int clientSocket(char const* host)
{
int socketId = socket(PF_INET, SOCK_STREAM, 0);
if (socketId == -1)
{
socketError = failedSocket;
return -1;
}
str
So I need a simple C version of a client/server app.
So here it is for review (Also on github)
A linked question is the C++ version
MakeFile
all: client server
.PHONY: all
.INTERMEDIATE: %.o
CFLAGS += -Wall -Wextra -Werror -pedantic
client: client.o common.o
server: server.o common.o
server.o: server.c common.h
client.o: client.c common.h
common.o: common.c common.hCommon.h
#ifndef COMMON_H
#define COMMON_H
#include
#define QUOTE_(X) #X
#define QUOTE(X) QUOTE_(X)
extern char const* socketError;
int serverSocket();
int clientSocket(char const* host);
int sendMessage(int socket, char const* buffer, size_t size);
int getMessage(int socket, char* buffer, size_t size);
#endifcommon.cpp
```
#include "common.h"
#include
#include
#include
#include
char const* failedSocket = "Failed: socket()";
char const* failedBind = "Failed: bind()";
char const* failedListen = "Failed: listen()";
char const* failedConnect= "Failed: connect()";
char const* socketError = "None";
int serverSocket()
{
int socketId = socket(PF_INET, SOCK_STREAM, 0);
if (socketId == -1)
{
socketError = failedSocket;
return -1;
}
struct sockaddr_in serverAddr;
bzero((char*)&serverAddr, sizeof(serverAddr));
serverAddr.sin_family = AF_INET;
serverAddr.sin_port = htons(8080);
serverAddr.sin_addr.s_addr = INADDR_ANY;
if (bind(socketId, (struct sockaddr *) &serverAddr, sizeof(serverAddr)) != 0)
{
socketError = failedBind;
close(socketId);
return -1;
}
if(listen(socketId, 5) != 0)
{
socketError = failedListen;
close(socketId);
return -1;
}
return socketId;
}
int clientSocket(char const* host)
{
int socketId = socket(PF_INET, SOCK_STREAM, 0);
if (socketId == -1)
{
socketError = failedSocket;
return -1;
}
str
Solution
General
Nice clean presentation.
-
A potential infinite loop. Consider some limitation on repetitive
-
Name space: The names of functions, variables and macros in
-
Rather than
-
-
Alternate initialization
-
Avoid naked magic numbers. Consider self-documenting code
-
I have a suspicion that usage of
Minor
-
Recommend using
-
Style: Prefer the explicit function signature
-
Style: The spacing used in the below adds little clarity, but adds typing. Suggest simplification.
Nice clean presentation.
-
A potential infinite loop. Consider some limitation on repetitive
EINTR conditions. I would expect a errno==0 before the write().while(sentSize != size) {
errno = 0; // add
size_t sent = write(socket, buffer + sentSize, size - sentSize);
if (sent == -1u && errno == EINTR) {
continue;
}-
Name space: The names of functions, variables and macros in
common.h share no cohesive naming scheme. This makes it unclear of their source when used. The use of QUOTE() and common.h is something that easily could collide with other libraries. Suggest something like CVersion.h with CVersion_serverSocket(),CVersion_QUOTE(), etc.-
Rather than
extern char const socketError;, make it a function: char const socketError(); to prevent user code from changing it.-
bzero() is not standard C, yet implementations I find use void bzero(void *s, size_t n);, thus the cast is not needed. Also parens not needed with sizeof variable// bzero((char*)&serverAddr, sizeof(serverAddr));
bzero(&serverAddr, sizeof serverAddr );-
Alternate initialization
//struct sockaddr_in serverAddr;
//bzero((char*)&serverAddr, sizeof(serverAddr));
struct sockaddr_in serverAddr = { 0} ;-
Avoid naked magic numbers. Consider self-documenting code
// htons(8080)
#define PORT_DUJOUR 8080
htons(PORT_DUJOUR);
// listen(socketId, 5)
#define LISTIN_BACKLOG 5
listen(socketId, LISTIN_BACKLOG)-
I have a suspicion that usage of
format = get != SERVER_BUFFER_SIZE ? endBufferFormat : fullBufferFormat; may result in the final getMessage() not printing a ’\n’. Minor
-
Recommend using
bool for Boolean like variables.// int finished = 0;
//while(!finished)
#include
bool finished = false;
while(!finished)-
Style: Prefer the explicit function signature
int serverSocket(void) vs int serverSocket().-
Style: The spacing used in the below adds little clarity, but adds typing. Suggest simplification.
// int get;
// char buffer[SERVER_BUFFER_SIZE];
int get;
char buffer[SERVER_BUFFER_SIZE];Code Snippets
while(sentSize != size) {
errno = 0; // add
size_t sent = write(socket, buffer + sentSize, size - sentSize);
if (sent == -1u && errno == EINTR) {
continue;
}// bzero((char*)&serverAddr, sizeof(serverAddr));
bzero(&serverAddr, sizeof serverAddr );//struct sockaddr_in serverAddr;
//bzero((char*)&serverAddr, sizeof(serverAddr));
struct sockaddr_in serverAddr = { 0} ;// htons(8080)
#define PORT_DUJOUR 8080
htons(PORT_DUJOUR);
// listen(socketId, 5)
#define LISTIN_BACKLOG 5
listen(socketId, LISTIN_BACKLOG)// int finished = 0;
//while(!finished)
#include <stdbool.h>
bool finished = false;
while(!finished)Context
StackExchange Code Review Q#125272, answer score: 3
Revisions (0)
No revisions yet.