Предисловие
Как-то раз откликнулся на вакансию С++ разработчика с хорошей вилкой от сорока до сто восьмидесяти тысяч в своем регионе. До этого не имел опыта коммерческой разработки и мне в ответ прислали тестовое задание. Там было три задачи из которых надо было решить две. Как всегда моя любимая рубрика – решение тестовых задач. Сегодня разберем одну из них.
1. Задача
Добрый день.
Решение желательно разместить в репозитории Github и прислать ссылки. Но можно прислать исходные коды.
Тестовая задача 1. Язык С++. Среда разработки Visual Studio 2019.
Задача. Написать 2 консольных приложения Client.exe и Server.exe под Windows, обменивающихся файлами через UPD сокет с подтверждением по TCP.
1.Сервер. Server.exe
Первый параметр – IP, второй номер порта, третий – каталог для хранения файлов.
Сервер начинает прослушивать по IP адресу порт и ждет подключения клиента по TCP.
Пример:
Server.exe 127.0.0.1 5555 temp
1. Клиент. Client.exe
Старт с 5 параметрами.
1 и 2 параметры – IP адрес и порт для подключения к серверу. Третий – порт для отправки UDP пакетов. Четвертый – путь к файлу. Пятый – таймаут на подтверждение UDP пакетов в миллисекундах.
Пример
Client.exe 127.0.0.1 5555 6000 test.txt 500
3.Взаимодействие сервера и клиента.
При старте сервер открывает TCP сокет с IP адресом и портом из стартовых параметров(1 и 2 параметры), берет его на прослушивание и ожидает подключений от клиента.
Клиент при старте пытается подключиться к серверу по TCP на IP адрес и порт из стартовых параметров (1 и 2 параметры ) пока не будет установлено соединение. После установления соединения клиент загружает в память файл (3 параметр, размер файла не более 10 Мб.) и отправляет по TCP соединению имя файл и порт UDP на сервер.
Далее клиент начинает отправлять файл блоками (Каждый блок данных в udp пакетах должен содержать свой id) udp датаграммами и получать по TCP подтверждения о приеме сервером.
В случае если в течении таймаута (5 параметр) не было получено подтверждение на пакет, то пакет по udp отправляется повторно. В случае когда все пакеты подтверждены клиент уведомляет сервер TCP об окончании передачи файла,
закрывает TCP соединение, завершает работу.
Сервер после установления подключения клиента по ТСP, получения имя файла и порта udp открывает udp сокет и начинает принимать udp пакеты исходящие с IP адреса клиента и порта переданного клиентом.
На каждый полученный udp пакет сервер отправляет подтверждение через tcp сокет на клиент. Полученные пакеты хранятся в памяти. После получения от клиента уведомления об окончании передачи (все пакеты подтверждены на клиенте) сервер сохраняет файл в каталог (3 параметр).
Формат посылок по TCP (имя файла, порт и подтверждения пакетов) - на ваше усмотрение
формат и нумерация блоков для udp пакетов - на ваше усмотрение
2. Решение
Что касается задачи – то она хорошо расписана – без всяких там «белых пятен». Давайте сразу перейдем к коду решения.
Код клиента
#include <iostream>
#include "Connection.h"//Класс в который обернуты сокеты
#include <fstream>
#include <vector>
#include <string>
int main(int argc,char*argv[])
{
if (argc == 1)
{
std::cout << "no flags argc!" << std::endl;
std::cout << "Usage: Client.exe <ip> <portTCP> <portUDP> <Filename> <Timeout>" << std::endl;
exit(1);
}
Connection* con = new Connection(argv[5]);//создаем соединение и передаем в него таймаут
con->InitClient(argv[1], atoi(argv[2]),atoi(argv[3]));//подключаемся к серверу(передаем ip адрес,tcp порт и udp порт
std::vector<std::string> vecline;
std::string line;
std::ifstream in(argv[4]); // открываем файл для чтения
if (in.is_open())
{
int ch=0;
while((ch=in.get())!=EOF)
{
line += ch;
if (line.size() == 242)
{
vecline.push_back(line);
line = "";
}
}
if(line!="")
vecline.push_back(line);
}
in.close(); // закрываем файл
std::string path = argv[4];
std::string filename = path.substr(path.rfind('\\') + 1);//выделяем имя файла из полного пути к файлу
con->Send(argv[3]);//отправляем на сервер udp порт через tcp
con->Send(filename);//передаем на сервер имя файла
con->Send(std::to_string(vecline.size()));//передаем на сервер кол-во строк
std::cout << "Starting transmission" << std::endl;//начинаем передачу
std::string id = "";
for (int i = 0; i < vecline.size(); i++)
{
id = "Begin";
if (i / 10.0 < 1)
id += "00000";
else if((i/100.0<1) && (i/10.0>=1))
id += "0000";
else if ((i/1000.0<1) && (i / 100.0 >= 1))
id += "000";
else if ((i / 10000.0 < 1) && (i / 1000.0 >= 1))
id += "00";
else if ((i / 100000.0 < 1) && (i / 10000.0 >= 1))
id += "00";
else if ((i / 1000000.0 < 1) && (i / 100000.0 >= 1))
id += "00";
else
id += "0";
id += std::to_string(i) + "End";
id += vecline[i]; //обертываем udp пакет служебными данными id формата Begin000001EndData
con->SendUDP(id);//отправляем udp пакет на сервер
//Sleep(1000);
con->Receive();//получаем подтверждение от сервера через tcp
std::cout << "i" << i << std::endl;
if ((i!=0) && (i != atoi(con->GetBuffer())))//в случае если пакеты
i--;//потерялись по дороге то уменьшаем i и по циклу состоится
id = "";//повторная передача
}
con->ClientClose();
con->~Connection();
std::cout << "Done!\n";
}Клиент считывает файл(текстового формата) посимвольно - пока не достигнет длины 242 символа. 14 символов зарезервировано для udp заголовка . В сумме будет 256=buflen.
Код сервера
#include <iostream>
#include "Connection.h"
#include <fstream>
#include <string>
int main(int argc, char* argv[])
{
system("Del \"C:\\temp\\test.txt\"");//удаляем наш тестовый файл если он есть
if (argc == 1)
{
std::cout << "no flags argc!" << std::endl;
std::cout << "Usage: Server.exe <ip> <port> <Folder>" << std::endl;
exit(1);
}
std::string timeout_ = "500";//задаем таймаут
Connection* con = new Connection(timeout_.c_str());
con->InitServer(argv[1], atoi(argv[2]));//стартует сервер с TCP сокетом
con->ReceiveServer();//получаем данные с портом udp от клиента
int portudp = atoi(con->GetBuffer());
con->InitServerUDP(argv[1], portudp); //стартует сервер с UDP сокетом
con->ReceiveServer();//Получаем имя файла
std::string FileName = con->GetBuffer();
if (FileName.size() > FILENAME_MAX)
std::cout << "File name is so long" << std::endl;
con->ReceiveServer();//получаем кол-во строк в файле
int j = atoi(con->GetBuffer());
int i = 0,jj;
std::string id = "";
std::cout << "Starting transmission" << std::endl;//передача
while (i < j)
{
jj = 0;
con->ReceiveServerUDP();
while (con->GetVec()[i][jj++] != 'n')
;
while (con->GetVec()[i][jj] != 'E')
id += con->GetVec()[i][jj++];//парсим строку
con->SendServer(id);//выделяем id пакета и отправляем его клиенту
std::cout << "i="<<i << std::endl;
if (atoi(id.c_str()) == i) );//сравниваем id пакета и номер строки
{
i++;
}
id = "";
}
std::cout << "Transmission is end" << std::endl;
std::ofstream out; // поток для записи
std::string path = argv[3];
system("mkdir \"C:\\temp\\\"");//создаем директорию
path += '\\' + FileName;
if (path.size() > FILENAME_MAX)
std::cout << "File name is so long" << std::endl;
std::cout << "Creating file" << std::endl;
out.open(path); // открываем файл для записи
if (out.is_open())
{
std::string strfile = "";
int jj;
for (int i = 0; i < con->GetVec().size(); i++)
{
strfile = "";
jj = 0;
while (con->GetVec()[i][jj++] != 'd')
;
for (jj; jj < con->GetVec()[i].size(); jj++)
strfile += con->GetVec()[i][jj];//Вырезаем заголовки udp
out <<strfile;// и пишем в файл
}
out.close();
}
con->ServerClose();
con->~Connection();
std::cout << "Done!\n";
}Схема передачи данных:
В самом же классе Connection в принципе описаны системные сокеты, единственное что хочется упомянуть – это вот данные фрагменты:
Таймауты:
struct timeval timeout;
timeout.tv_sec = 0;
if(timeout_!="")
timeout.tv_usec = atoi(timeout_);
else
timeout.tv_usec = 500;И части кода которые вызывается после всех инициализаций(непосредственно перед приемом или передачей – один раз):
if (setsockopt(newsockfd, SOL_SOCKET, SO_SNDTIMEO, (char*)&timeout, sizeof timeout) < 0)
error("setsockopt failed\n");Для серверного tcp сокета передачи
if (setsockopt(sockfd, SOL_SOCKET, SO_RCVTIMEO, (char*)&timeout, sizeof timeout) < 0)
error("setsockopt failed");Для клиентского tcp сокета приема
P.S. Считаю что данный пост будет не полным, если я не добавлю класс Connection с привычными для глаза системными сетевыми вызовами функций.
Заголовочный файл Connection.h
#pragma once
#include <iostream>
#include <stdlib.h>
#include <string.h>
#include <sys/types.h>
#include <WinSock.h>
#include <fcntl.h>
#pragma comment (lib,"WS2_32.lib")
#pragma warning(disable : 4996)
#include <vector>
class Connection
{
public:
Connection(const char* timeout);
int InitServer(const char* address, int port);
int ServerClose();
int Send(std::string line);
int Receive();
int SendUDP(std::string line);
int ReceiveUDP();
int InitClient(const char* address, int port,int udpport);
int ClientClose();
void bzero(char* buf, int l);
void error(const char* msg);
int SendServer(std::string line);
int ReceiveServer();
int ReceiveServerUDP();
int SendServerUDP();
char* GetBuffer();
std::vector<std::string> GetVec();
int Block(bool block);
int BlockServer(bool block);
int InitServerUDP(const char* address, int port);
virtual ~Connection();
protected:
private:
int sockfd, newsockfd,udpsocket;
int buflen;
char* buffer;
struct sockaddr_in serv_addr, cli_addr,serv_addr_udp,cli_addr_udp;
int clilen;
int servlen;
int n;
std::string strFile,strCli;
std::vector<std::string> vecline,veccli;
struct timeval timeout;
};
Соответственно у сервера три сокета: sockfd слушает tcp порт, newsockfd для передачи tcp пакетов клиенту и udpsocket - для приема udp.
У клиента sockfd - для приема tcp пакетов и udpsocket - для передачи udp пакетов.
Сам класс Connection.cpp
#include "Connection.h"
void Connection::bzero(char* buf, int l)
{
memset(buf, 0, l);
}
void Connection::error(const char* msg)
{
int err = WSAGetLastError();
perror(msg);
std::cout<<err<<std::endl;
WSACleanup();
std::cin.ignore();
exit(1);
}
Connection::Connection(const char*timeout_)
{
buflen = 256;
buffer = new char[buflen];
strFile = "";
strCli = "";
timeout.tv_sec = 0;
if(timeout_!="")
timeout.tv_usec = atoi(timeout_);
else
timeout.tv_usec = 500;
}
int Connection::InitServer(const char* address,int port_)
{
WSADATA ws = { 0 };
if (WSAStartup(MAKEWORD(2, 2), &ws) == 0)
{
sockfd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
if (sockfd < 0)
error("ERROR opening socket");
bzero((char*)&cli_addr, sizeof(cli_addr));
memset(serv_addr.sin_zero, 0, sizeof(serv_addr.sin_zero));
serv_addr.sin_family = AF_INET;
serv_addr.sin_port = htons(port_);
std::string str = "";
int i = 0;
while (address[i] != '.')
str += address[i++];
i++;
//std::cout << str<<std::endl;
serv_addr.sin_addr.S_un.S_un_b.s_b1 = atoi(str.c_str());
str = "";
while (address[i] != '.')
str += address[i++];
i++;
serv_addr.sin_addr.S_un.S_un_b.s_b2 = atoi(str.c_str());
//std::cout << str << std::endl;
str = "";
while (address[i] != '.')
str += address[i++];
i++;
serv_addr.sin_addr.S_un.S_un_b.s_b3 = atoi(str.c_str());
//std::cout << str << std::endl;
str = "";
while (i!=strlen(address))
str += address[i++];
// std::cout << str << std::endl;
serv_addr.sin_addr.S_un.S_un_b.s_b4 = atoi(str.c_str());
if (bind(sockfd, (struct sockaddr*)&serv_addr, sizeof(serv_addr)) < 0)
error("ERROR on binding");
if (listen(sockfd, SOMAXCONN) < 0)
error("ERROR on listen");
std::cout << "Waiting client" << std::endl;
clilen = sizeof(cli_addr);
newsockfd = accept(sockfd, (struct sockaddr*)&cli_addr, &clilen);
std::cout << "Client connected" << std::endl;
if (newsockfd < 0)
error("ERROR on accept");
//if (setsockopt(newsockfd, SOL_SOCKET, SO_RCVTIMEO, (char*)&timeout, sizeof timeout) < 0)
// error("setsockopt failed\n");
if (setsockopt(newsockfd, SOL_SOCKET, SO_SNDTIMEO, (char*)&timeout, sizeof timeout) < 0)
error("setsockopt failed\n");
}
else
WSAGetLastError();
return 0;
}
int Connection::InitServerUDP(const char* address,int port)
{
udpsocket = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
if (udpsocket < 0)
error("ERROR opening socket");
serv_addr_udp.sin_family = AF_INET;
serv_addr_udp.sin_port = htons(port);
std::string str = "";
int i = 0;
while (address[i] != '.')
str += address[i++];
i++;
serv_addr_udp.sin_addr.S_un.S_un_b.s_b1 = atoi(str.c_str());//задаем октеты адреса
str = "";
while (address[i] != '.')
str += address[i++];
i++;
serv_addr_udp.sin_addr.S_un.S_un_b.s_b2 = atoi(str.c_str());//задаем октеты адреса
str = "";
while (address[i] != '.')
str += address[i++];
i++;
serv_addr_udp.sin_addr.S_un.S_un_b.s_b3 = atoi(str.c_str());//задаем октеты адреса
str = "";
while (i != strlen(address))
str += address[i++];
serv_addr_udp.sin_addr.S_un.S_un_b.s_b4 = atoi(str.c_str());//задаем октеты адреса
if (bind(udpsocket, (struct sockaddr*)&serv_addr_udp, sizeof(serv_addr_udp)) < 0)
error("ERROR on binding");
std::cout << "Udp Ready" << std::endl;
return 0;
}
char* Connection::GetBuffer()
{
return buffer;
}
std::vector<std::string> Connection::GetVec()
{
return vecline;
}
int Connection::ServerClose()
{
shutdown(newsockfd, 0);
shutdown(sockfd, 0);
shutdown(udpsocket, 0);
closesocket(newsockfd);
closesocket(sockfd);
closesocket(udpsocket);
WSACleanup();
return 0;
}
int Connection::SendServer(std::string line)
{
memset(buffer, 0, buflen);
n = send(newsockfd, line.c_str(), buflen, 0);
//std::cout << line << std::endl;
if (n < 0)
error("ERROR on send");
return n;
}
int Connection::Send(std::string line)
{
n = send(sockfd, line.c_str(), buflen, 0);
if (n < 0)
error("ERROR on send");
return n;
}
int Connection::SendUDP(std::string line)
{
n = sendto(udpsocket, line.c_str(), buflen, 0, (struct sockaddr*)&serv_addr_udp, sizeof serv_addr_udp);
if (n < 0)
error("ERROR on send udp");
//std::cout << "send" << std::endl;
return n;
}
int Connection::ReceiveServer()
{
memset(buffer, 0, buflen);
n = recv(newsockfd, buffer, buflen, 0);
if (n < 0)
error("ERROR on read");
return n;
}
int Connection::ReceiveServerUDP()
{
memset(buffer, 0, buflen);
int slen = sizeof(sockaddr_in);
n = recvfrom(udpsocket, buffer, buflen, 0, (struct sockaddr*)&cli_addr_udp, &slen);
//std::cout << "n=" << n << std::endl;
if (n < 0)
error("ERROR on read udp");
for (int i = 0; i < strlen(buffer); i++)
strFile += buffer[i];
if (vecline.size()!=0 && strFile == vecline[vecline.size() - 1])
return n;//проверяем не повторилась ли передача udp пакета
vecline.push_back(strFile);
//std::cout << strFile <<" buf len"<< strlen(buffer)<< std::endl;
strFile = "";
return n;
}
int Connection::Receive()
{
memset(buffer, 0, buflen);
n = recv(sockfd, buffer, buflen, 0);
//std::cout << buffer << std::endl;
if (n < 0)
error("ERROR on read");
for (int i = 0; i < strlen(buffer); i++)
strCli += buffer[i];
if (veccli.size() !=0 && strCli == veccli[veccli.size() - 1])
return n;//проверяем не повторилась ли передача tcp пакета
veccli.push_back(strCli);
strCli = "";
return n;
}
int Connection::Block(bool block)
{
u_long argp = block ? 1 : 0;
ioctlsocket(sockfd, FIONBIO, &argp);
return n;
}
int Connection::BlockServer(bool block)
{
u_long argp = block ? 1 : 0;
ioctlsocket(newsockfd, FIONBIO, &argp);
return n;
}
Connection::~Connection()
{
delete[] buffer;
}
int Connection::InitClient(const char* address_, int port_,int udpport)
{
WSADATA ws = { 0 };
if (WSAStartup(MAKEWORD(2, 2), &ws) == 0)
{
sockfd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
if (sockfd < 0)
error("ERROR opening socket");
bzero((char*)&serv_addr, sizeof(serv_addr));
bzero((char*)&cli_addr, sizeof(cli_addr));
serv_addr.sin_family = AF_INET;
serv_addr.sin_addr.s_addr = inet_addr(address_);
serv_addr.sin_port = htons(port_);
servlen = sizeof(serv_addr);
n = connect(sockfd, (struct sockaddr*)&serv_addr, servlen);
if (n < 0)
error("ERROR on connect");
if (setsockopt(sockfd, SOL_SOCKET, SO_RCVTIMEO, (char*)&timeout, sizeof timeout) < 0)
error("setsockopt failed");
udpsocket = socket(AF_INET, SOCK_DGRAM, 0);
if (udpsocket < 0)
error("ERROR opening udp socket");
memset((char*)&serv_addr_udp, 0, sizeof(serv_addr_udp));
serv_addr_udp.sin_family = AF_INET;
serv_addr_udp.sin_port = htons(udpport);
serv_addr_udp.sin_addr.S_un.S_addr = inet_addr(address_);
//if (setsockopt(sockfd, SOL_SOCKET, SO_SNDTIMEO, (char*)&timeout, sizeof timeout) < 0)
// error("setsockopt failed");
}
else
WSAGetLastError();
return 0;
}
int Connection::ClientClose()
{
shutdown(udpsocket, 0);
shutdown(sockfd, 0);
closesocket(udpsocket);
closesocket(sockfd);
WSACleanup();
return 0;
}
Заключение и выводы
Вот такая задача – могу сказать, что научился применять передачу данных в такой вот связке и таймаутам, конечно. Единственный минус – кодировка русских букв в файле после приема меняется и не читаемая.
Полный код смотреть здесь:
https://gitlab.com/Gremlin_Rage/cpp/-/tree/master/Client
https://gitlab.com/Gremlin_Rage/cpp/-/tree/master/Server
Класс Connection общий для обоих проектов и находится в проекте с сервером.
По задаче в ней не указано, что делать с таймаутом на сервере – по мне его бы тоже передать на сервер вначале не помешало.
Комментарии (54)
SpiderEkb
01.09.2022 14:17+3Про poll/select не слышали ничего?
Единственный минус – кодировка русских букв в файле после приема меняется и не читаемая
Искажение данных. Задача не решена.
AllKnowerHou Автор
01.09.2022 14:42Про poll/select не слышали ничего?
Может в следующий раз - буду разбирать(ться)
Искажение данных. Задача не решена.
Может я не правильно выразился - после записи в файле - кодировка меняется
vesper-bot
01.09.2022 16:29+1Дык, у вас вывод реализован через << а это не бинарный, а текстовый вывод. А у вас данные по сути бесформатные, и реализовывать надо было это через массив байт, а я у вас там строки вижу.
AllKnowerHou Автор
01.09.2022 22:22Скорее в задаче делается упор на передачу данных, а не на ввод-вывод файлов в системе. По факту можно допилить и на коленке сделать передачу любых типов файлов, любого размера. Времени было мало просто...
amarao
01.09.2022 14:31+7Очень плотный грязный код. Делите на функции, давайте функциям названия. Нет тестов.
Kotofay
01.09.2022 14:34+8con->~Connection();Никогда больше так не делайте.
n = send(newsockfd, line.c_str(), buflen, 0);Стандартная ошибка начинающего, send не передаёт buflen байт за один вызов, и возвращает n >= 0 || n <= buflen в случае успешной передачи.
Вы не обрабатываете ситуацию, если передано меньше чем buflen, а нужно вернуться и передавать остаток пока не будет передана вся строка.
В задании не написано что файл текстовый и что файл будет один.
Задача не выполнена.
AllKnowerHou Автор
01.09.2022 14:53В задании не написано что файл текстовый и что файл будет один.
Вы наполовину правы что не только текстовый, а в задании написано что:
...Четвертый – путь к файлу....
Не к файлам, а к файлу.
Ну, а как решить спросите Вы?
Читать в бинарном режиме и передавать
Kotofay
01.09.2022 15:041.Сервер. Server.exe
Первый параметр – IP, второй номер порта, третий – каталог для хранения файлов.
"Файлов", вы же видите.
AllKnowerHou Автор
01.09.2022 15:11Ну сервер может хранить в том каталоге сколько угодно файлов, но клиент передает один файл за один запуск программы
Kotofay
01.09.2022 16:33+2Ваш Сервер может принять только один файл за один запуск, а в задании написано -- сервер хранит "файлы".
eptr
01.09.2022 20:35А как вы в четвертом параметре для клиента передадите несколько имён файлов?
Kotofay
01.09.2022 20:42А при чём тут клиент? Речь идёт о сервере. Клиент можно и много раз запустить с разных хостов.
Сервер будете каждый раз перезапускать для приёма одного файла?
В задании написано -- "хранить файлы".
Да и с параметром никаких проблем, напишите через разделитель {;,+} а не пробел.
AllKnowerHou Автор
01.09.2022 22:17-1Что-то не особо понимаю ваш акцент на словах "хранит файлы", файлы сами по себе, программы сами по себе. Ну лежат они там и пусть лежат себе, что такого?
Kotofay
01.09.2022 22:45+2Так можно про всё задание сказать.
Не понимаете, так и хорошо, я вам завидую, у вас ещё много интересного впереди.
eptr
01.09.2022 22:49А при чём тут клиент? Речь идёт о сервере. Клиент можно и много раз запустить с разных хостов.
Сервер будете каждый раз перезапускать для приёма одного файла?
В задании написано -- "хранить файлы".
Вы имеете ввиду, что файл, отправляемый на сервер с таким именем на сервере в том каталоге уже может быть, и нужно этот случай обработать?
Или что сервер не должен завершаться после приёма одного файла?
Не проще ли ясно выражать свои мысли, что именно вы имеете ввиду?
Да и с параметром никаких проблем, напишите через разделитель {;,+} а не пробел.
Нет, проблема есть -- клиент тогда должен разобрать этот параметр.
Но в задании ничего такого нет.
vesper-bot
01.09.2022 16:57Сервер не должен завершаться после работы с клиентом, а у вас он именно что завершается.
AllKnowerHou Автор
01.09.2022 22:37Это нигде не сказано
eptr
01.09.2022 23:20+2Однако, это -- общепринятая практика, поэтому специально об этом и не говорят.
На самом деле, сказано:
При старте сервер открывает TCP сокет с IP адресом и портом из стартовых параметров(1 и 2 параметры), берет его на прослушивание и ожидает подключений от клиента.
Написано "подключений", во множественном числе.
При этом, из описания алгоритма работы клиента ясно, что клиент открывает только одно TCP/IP-соединение.Значит, после того, как клиент закроет соединение и завершится, сервером будет обработано только одно подключение от клиента, поэтому, раз в условии написано "подключений", сервер должен продолжить, чтобы принять ещё хотя бы одно подключение.
Также, по поводу клиента явно сказано, что он завершает работу, а по поводу сервера -- нет.
AllKnowerHou Автор
02.09.2022 16:32Мне кажется по тексту - там немного проблема с тем, как оно сформулировано - в некоторых местах приходиться самому додумывать, либо искать логические ошибки - как спор о количестве клиентов и завершении работы сервера
SpiderEkb
02.09.2022 17:39Так суть тестового не в том, чтобы конкретную задачу решить, а в том, чтобы посмотреть как человек подходит к решению. Паттерны проектирования, обработки ошибок. Думаю, что даже уточняющие вопросы, правильно заданные, могут сыграть в плюс кандидату (я бы точно записал бы в плюс правильно заданные вопросы).
Далеко не всегда ТЗ бывает хорошим. Там тоже могут быть недочеты. И вопрос в том - будет разработчик бездумно кодировать по ТЗ, или скажет аналитику - "так это не работает, лучше вот тут сделать вот так".
У нас некоторые аналитики иногда сразу говорят - "мне на эту задачу нужен думающий разработчик, чтобы я ему объяснил суть проблемы, а он предложил как это лучше реализовать".
AllKnowerHou Автор
01.09.2022 15:23-1Стандартная ошибка начинающего, send не передаёт buflen байт за один вызов, и
возвращает n >= 0 || n <= buflen в случае успешной передачи.
Вы не обрабатываете ситуацию, если передано меньше чем buflen, а нужно вернуться и передавать остаток пока не будет передана вся строка.
Это заблуждение связанное с неполным знанием работы TCP/IP. Да сетевая карта может отправить один send на одну, две или более частей. В зависимости от ее загрузки другими данными - других приложений. Но на транспортном уровне данного приложения они соберутся в одну строку в recv и правильно обработаются. То есть программа будет ждать пока весь buflen не соберется в один пакет - строку.
Kotofay
01.09.2022 16:26+4Это вы с кем сейчас спорите? Читайте документацию по send.
https://www.opennet.ru/docs/RUS/linux_base/node250.html
https://docs.microsoft.com/ru-RU/windows/win32/api/winsock2/nf-winsock2-send
Return valueIf no error occurs, send returns the total number of bytes sent, which can be less than the number requested to be sent in the len parameter. Otherwise, a value of SOCKET_ERROR is returned, and a specific error code can be retrieved by calling WSAGetLastError.
То что они на приёмном конце соберутся нет никаких сомнений, но вот передающая часть может быть очень занята и буфер передачи TCP переполнен, тогда send просто вернёт размер того что ему удалось передать в данный момент. Попозже буфер передачи будет освобождён и передачу нужно продолжить.Такая же ситуация с функцией recv -- она может принять меньше чем вы ей указали и вернёт количество принятых байт.
И вообще, с таким кодом как у вас, лучше ни с кем не спорить. Просто впитывайте то что вам говорят, пока ещё без подколок и издевательств.
Задание не выполнено.AllKnowerHou Автор
01.09.2022 22:27Как бы то ни было, данный алгоритм либо перешлëт заново udp пакет, либо tcp пакет
Kotofay
01.09.2022 22:49+1Вы определённо не понимаете логики возвращаемого значения send.
Ваш алгоритм не решит проблему неполной передачи одной строки или дейтаграммы. При более-менее медленном соединении или через интернет вы начнёте терять хвосты строк и/или служебных данных.
#ifdef _Windows long int lsend(SOCKET f, const char *p, const long int s, int flags){ #else long int lsend(int f, const char *p, const long int s, int flags){ #endif long int All, SendUnit;/* Всего передано, передано блоков */ char *tmp = (char*)p;/* указатель на текущий буфер размером = (s - All) */ for(All = SendUnit = 0;All < s;){ /* Передаём блок */ SendUnit = send(f, tmp, s - All, flags); #ifdef _Windows if((SendUnit == SOCKET_ERROR) || (SendUnit == 0)) #else if((SendUnit == -1) || (SendUnit == 0)) #endif return SendUnit;/* Ошибка или ничего не передано */ tmp += SendUnit;/* Смещаем на сколько передано */ All += SendUnit;/* Сколько передано всего */ }/* for */ return All; }/* lsend */Ориентировочно так должен выглядеть блокирующий вывод без потерь через send. Этому коду 25 лет, с ходу может не собраться.
eptr
01.09.2022 23:59char *tmp = (char*)p;Для чего здесь приведение к
(char *)?
Или это демонстрация вашего пониманияconst25-летней давности?
AllKnowerHou Автор
02.09.2022 16:35Вы определённо не понимаете логики возвращаемого значения send.
Если бы я не понимал то - программа не работала бы. В случае если клиент отправил бы часть данных, либо сервер прочитал не все - программы аварийно завершаться
eptr
01.09.2022 23:04-2Это вы с кем сейчас спорите?
Разве это имеет хоть какое-то значение?
Авторитетов не существует.
Существуют только аргументы.Читайте документацию по send.
Вот это -- уже совсем другое дело.
Разве там сказано, что может быть записано меньше, чем затребовано?
Функция возвращает число записанных в сокет байтов ( в нормальном случае должно быть равно значению параметра len ) или -1 в случае ошибки.
"Ненормальный" случай исчерпывающе и недвусмысленно не описан.
Можно предположить, что под "ненормальным" случаем понимается возврат-1.
Правда, не сказано, что "ненормальный" случай этим и исчерпывается.
Явно про что-то, отличное отlenи-1не сказано.И это -- какая-то "вторичная" документация, она может быть искажённой и поэтому --ошибочной. Смотреть следует первичную.
К тому же, оказывается, что есть различия в поведении для UNIX'ов и для Windows, поэтому ссылаться на UNIX-документацию здесь не следует.
https://docs.microsoft.com/ru-RU/windows/win32/api/winsock2/nf-winsock2-send
Хорошо.
Return value
If no error occurs, send returns the total number of bytes sent, which can be less than the number requested to be sent in the len parameter. Otherwise, a value of SOCKET_ERROR is returned, and a specific error code can be retrieved by calling WSAGetLastError.
Это -- не вся правда.
Там есть ещё кое-что:If no buffer space is available within the transport system to hold the data to be transmitted, send will block unless the socket has been placed in nonblocking mode. On nonblocking stream oriented sockets, the number of bytes written can be between 1 and the requested length, depending on buffer availability on both the client and server computers.
Видите, в секции для return value не стали уточнять, что which can be less than the number requested to be sent in the len parameter относится к сокету, который has been placed in nonblocking mode.
В приведённом же коде сокеты не переводятся в nonblocking mode.
Поэтому там не требуется проверять, весь ли затребованный блок был отправлен.Вам на практике удавалось создать/наблюдать ситуацию, когда для блокирующих сокетов под Windows реально возникает эффект возврата из
sendменьшего количества, чем затребовано?И вообще, с таким кодом как у вас, лучше ни с кем не спорить. Просто впитывайте то что вам говорят, пока ещё без подколок и издевательств.
Почему человек должен вам верить на слово, что ему именно так и следует поступать?
Авторитетов не существует.
Или это акт -- "устрашения", учитывая упоминание подколок и издевательств?Задание не выполнено.
Вы точно не готовы review'ить чужой код.
Kotofay
02.09.2022 10:19+1Вам на практике удавалось создать/наблюдать ситуацию, когда для блокирующих сокетов под Windows реально возникает эффект возврата из
sendменьшего количества, чем затребовано?Когда то да.
Видите, в секции для return value не стали уточнять
Ваши интерпретации примечаний никого не интересуют. Гарантий того что написано в описании возвращаемого значения примечания не дают.
Тем более если по вашему коду не установлен неблокирующий вывод то это не значит что ваш код корректен, тем более что у вас есть и функция установки неблокирующего флага. Однако вы не обрабатываете эту ситуацию и для случая неблокирующего вызова у вас обработка не сделана.
Вы точно не готовы review'ить чужой код.
Ваши оценки тоже никого не интересуют.
Задание не выполнено.
eptr
02.09.2022 15:29Когда то да.
А сейчас?
На промежутке в 25 лет вполне имеет смысл учитывать явление фантазма.Ваши интерпретации примечаний никого не интересуют.
Зато они интересуют тех, кто хочет понять, как всё происходит на самом деле.
Мы же в публичном поле общаемся.Гарантий того что написано в описании возвращаемого значения примечания не дают.
Да, это проблема большинства документаций: остаются двусмысленности и не отвеченные вопросы.
Тем более если по вашему коду не установлен неблокирующий вывод то это не значит что ваш код корректен, тем более что у вас есть и функция установки неблокирующего флага.
Вообще-то, код -- не мой, если вы ещё до сих пор не заметили.
Функция есть, но не используется, а в реальности используется блокирующий ввод/вывод, и обсуждается именно он.
Однако вы не обрабатываете эту ситуацию и для случая неблокирующего вызова у вас обработка не сделана.
Для случая неблокирующего вызова обработка, очевидно, не сделана, потому что данный случай в текущем варианте кода не используется.
Ваши оценки тоже никого не интересуют.
Это, потому что вы не можете их принять.
Цель review заключается не в том, чтобы reviewer своими резкими высказываниями с намеренными недосказанностями пытался создавать ложное впечатление одновременно и собственного мнимого "величия", и мнимой ничтожности review'ируемого, а в том, чтобы объяснить проблемные места и задать направление, что следует изучить, дабы review'ируемый мог эффективно подтянуть свой уровень именно в том, что требуется для исправления проблемных мест, и в следующий раз выдавать уже код, близкий к нормальному.
Задание не выполнено.
Вы так упорно продолжаете это повторять, хотя все уже неоднократно это видели. Или вы в этом всё ещё не уверены?
Dacad
01.09.2022 15:09Почему при обработке Вас интересует именно полное отсутствие аргументов, а не их достаточное количество?
Вы предпочитаете уронить приложение путём возврата кода возврата, нежели обрабатывать исключения? К тому же вывод ошибок в stdout вместо stderr.
Выделение класса Connection, конечно хорошо, но пихать туда почти всю логику явно перебор. Стоило разбить сущности.
AllKnowerHou Автор
01.09.2022 15:48-1Почему при обработке Вас интересует именно полное отсутствие аргументов, а не их достаточное количество?
Ну можно написать например вначале
if ( argc<5 ) { ... }Вы предпочитаете уронить приложение путём возврата кода возврата, нежели обрабатывать исключения? К тому же вывод ошибок в stdout вместо stderr.
Дельное замечание - времени было мало, чтобы продумать данные варианты. Потому что там могут быть всякие разные ошибки в аргументах и их надо отлавливать.
Выделение класса Connection, конечно хорошо, но пихать туда почти всю логику явно перебор. Стоило разбить сущности.
Бритва Оккама son...
dyadyaSerezha
01.09.2022 20:24+2Зачем создаётся поинтер на Connection?? И уже если используете new, то заканчивать надо через delete, а не прямым вызовом деструктора. Но поинтера вообще не надо, а надо создать локальный объект на стеке. И в деструкторе проверять и закрывать соединение, если не закрыто.
Kotofay
01.09.2022 20:46+1Сложилось ощущение, что это пост сгенерирован ИИ а не написан реальным человеком.
Т.к. отвечает не по теме вопросов, код наляпан по школьному, отвечает с вызовом и пренебрежением, репозиторий годичной давности.
AllKnowerHou Автор
02.09.2022 16:36да delete забыл, а какая разница поинтер или просто переменная? Работать будет, что с pointer - что без
dyadyaSerezha
03.09.2022 10:56+1Разница такая, что размещение объекта на стеке существенно быстрее, чем в куче, и не надо помнить вызвать delete, так как деструктор вызовется автоматически при достижении конца области определения объекта. Это методологически неверно использовать поинтеры там, где прекрасно работают локальные объекты. Большой минус в глазах любого проверяющего.
assad77
01.09.2022 21:31+4обычно когда предлагают написать тестовую задачу, то хотят оценить кандидата с разных сторон: решение задачи, проектирование, стиль, понимание технологий с которыми предстоит работать, умение решать задачи эффективно, умение писать безопасный код.
какие я бы отметил проблемы:
Проектирование: основная проблема здесь, что существует один класс бог, который выполняем всё. с одной стороны этот класс не универсален, а заточен на конкретную ситуацию, с другой не решает конкретную задачу. кроме того, ответственность за работу с данными размазана между клиентом и сервером. лучшим решением было бы создать два разных класса client и server. работу с блоками данных лучше сделать на стороне клиента, чтобы не переусложнять. делать ли надстройку над сокетами -- это уже на усмотрение автора. но это упростит восприятие кода.
Протокол передачи. обычно при передаче через udp/tcp пакеты формируют в следующем виде: заголовок (4 байт), размер пакета (4 байт), crc32 (4 байт), служебные данные(например id пакета) (4 байт), данные (переменной длины). даже если часть данных передается в текстовом формате (json например) такой подход сохраняется. такой подход позволяет выявлять ошибки, получить уверенность что пакет пришел тот что надо. И это проще и для клиента и для сервера, тк протокол простой и заголовок всегда имеет стандартный размер. кроме того, так можно проверить что блок дошел полностью. здесь сходу бросается в глаза, что генерация id и заголовка переусложнена.
стилистика и мелкие ошибки проектирования: переменные называются i,j ... это очень неудобно. иногда даже опытные разработчики могут называть переменную i, но в этом случае область применения будет очень локальной типа небольшого цикла, здесь же, переменная j даже непонятно где объявлена, а тем более за что она отвечает. В c++ не принято создать объект, а потом еще раз его проинициализировать. Это приводит к тому, что объект получает возможность быть в невалидном состоянии. Лучше этого не допускать. Лишние члены класса коннекшен. sockaddr, buffer, vecline это все можно вынести в клиента и использовать один раз. Диагностика в в std::cout в классе connection и другие мелочи типа объявление переменных без инициализации. ну и то о чем уже писали. создание переменной connection в динамической памяти, вызов деструктора руками. да и WsaStratup я бы тоже в main вытащил.
решение задачи: часть проблем уже описывали: принимается только один файл и даже не идет речи о запуске двух клиентов параллельно (чему, кстати мешает приемный буфер - член класса не в последнюю очередь), работа только с текстовым файлом. Возвращаемое значение recv нужно учитывать всегда, вызов системных функций с захардкожеными путями temp.
производительность, самая главная проблема здесь -- ожидание клиентом ответа сервера. так передача будет работать очень медленно. я бы сделал клиенте упорядоченный по оффсетам файла vector, содержащий пару оффсет (текущее смещение в файле) и время. и при очередном отправлении пакета проверял не прошел ли таймаут у первого, и если прошел, то его можно перепослать. на клиенте чтобы учесть потерю пакетов забивал бы нулями незаполненную часть. конечно этот совет можно проигнорировать, но передача данных в этой задаче -- основная цель, те в продуктовом коде будут подобные задачи, и нужно сразу показать, то что решение будет эффективным. ну и конечно, нужно пользовать select/poll. это особенно важно становится если клиентов много.
безопасный код. Ошибки в большинстве случаев не проверяются. нужно либо пользовать исключениями, или в каждой строчке с io проверять результат выполнения функции. при этом exit(1) в классе решение плохое. завершать приложение класс не должен. за это должен отвечать main. про проверку количества параметров писали, можно также сказать про контроль размеров буфера. если хочется пользоваться c функциями, совместно с вектором, то желательно не использовать член класса bufsize а пользоваться buf.size(), это снизит вероятность ошибок с выходом за область массива. с этим нужно очень аккуратно работать. нужно везде на это внимание обратить.
по поводу тестов. я не думаю, что они здесь нужны. конечно некоторые их ожидают увидеть, тк видят в них серебряную пулю, поэтому не поставят галочку. но здесь код слишком простой и можно протестировать вручную. консольные утилиты сами по себе являются тестов. особенно с учетом того, что хочется сделать быстро. главное понимать как проверить какие-то краевые условия. Но, если делать оптимизацию по скорости, то на этот алгоритм можно и сделать отдельный тест. это время съэкономит. если же тестировать все подряд, то это сильно усложнит код и работу только замедлит.
Tujh
02.09.2022 13:10нужно пользовать select/poll. это особенно важно становится если клиентов много.
Если память не подводит для select в WinAPI существует ограничение в 64 сокета, больше он обработать не сможет. Поэтому или WSAEvent, но вновь с ограничениями, или IOCP, но это слишком сложно для тестовой задачи, хотя если бы было - дало бы огромный плюс.
AllKnowerHou Автор
02.09.2022 16:49...один класс бог... класс не универсален...ответственность за работу с данными размазана между клиентом и сервером...
Просто по фактам
...crc32...
не помешал бы
переменные называются i,j...
да косяк
BombaBomba
02.09.2022 09:20+1Искать ошибки в чужом коде это занятие куда более увлекательное чем...)
AllKnowerHou Автор
02.09.2022 16:52они короче люди с фантазией и опытом - додумывают полезный и оверюзлесс функционал но забывают - что времени мало всего день на это ушло и что я ни дня в коммерческой разработке не участвовал и если и имел опыт с с++ - то в других областях - всего понемногу
atd
02.09.2022 10:49+2Почему все критикуют решение автора (вполне заслуженно), но не обращают на мудацкую постановку задачи?
Почему вообще мы должны делать ACK того, что отправили в UDP по другому сокету, ещё и TCP? Лучше было бы обратно в UDP и ответить.
Почему клиент сам выбирает, в какой порт сервера будет отправлять данные? А если придут два клиента и скажут что хотят слать в порт 6000?
Как решается проблема легитимного изменения адреса клиента? (например переподключение с провода на WiFi или вкл выкл впна)
Как проверяется то, что данные приехали от нужного клиента? (src-ip легко можно поставить левый, особенно в UDP)
Зачем вообще переизобретать flow-control и другие фишки TCP? Если с ним какие-то проблемы, то они решаются тюнингом, а не переизобретением велосипеда. В крайнем случае использованием готовых решений типа QUIC
P.S.: посмотрел код, боже ж мой. Если кандидата после этого приняли на работу, то это просто идеальное соответствие квалификации кодера и архитектора. Просто в яблочко, они нашли друг друга!
AllKnowerHou Автор
02.09.2022 16:43Почему клиент сам выбирает, в какой порт сервера будет отправлять данные? А если придут два клиента и скажут что хотят слать в порт 6000?
Вот и логическая ошибка русского языка - в одном месте читается один клиент - в других местах несколько - я о ней выше говорил.
По остальным замечаниям и пунктам - времени мало - некогда все обдумать - думал лишь о функционале программы - меньше об ошибках, тестах и прочем. Да и если бы обдумал то код явно увеличился в разы.
Хотя с другой стороны два клиента или более на один порт - возможно, но придеться добавлять в пакеты данных информацию что от кого и парсить ее, а серверу массив newsockfd[i], buffer[i], cliaddr[i] многопоточность для них - я как ни пытался не смог понять poll/select
Tujh
02.09.2022 12:30К другим комментариям добавлю, что есть принципиальная ошибка в логике клиента.
Если данные пересылаются по UDP, а подтверждаются по TCP, то зачем ждать подтверждения каждого пакета? Это конечно не прописано прямо в задании, но подразумевается, что UDP используется для скорости, тот же TFTP, а значит нужно слать UDP-дейтаграммы непрерывным потоком и через таймаут повторять те пакеты, которые не подтверждены сервером. Как следствие, протокол обмена должен уметь подтверждать множество пакетов в одном сообщении, а клиент - уметь парсить это.
Уже на основании только этого - задание не пройдено.
А так, ещё куча мелочей
if (WSAStartup(MAKEWORD(2, 2), &ws) == 0)
...bzero((char)&serv_addr, sizeof(serv_addr));
мешанина API, если это для WinAPI, то зачем bzero() и почему не ZeroMemory()
udpsocket = socket(AF_INET, SOCK_DGRAM, 0);
if (udpsocket < 0)В WinAPI этот код принципиально ошибочен, так как, в отличии от unix систем, SOCKET целочисленная переменная, и INVALID_SOCKET имеет нулевое значение. То есть проверка "<0" просто не работает для Windows.
If no error occurs, socket returns a descriptor referencing the new socket. Otherwise, a value of INVALID_SOCKET is returned, and a specific error code can be retrieved by calling
https://docs.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-socket
Тест окончательно провален.
AllKnowerHou Автор
02.09.2022 16:40bzero((char)&serv_addr, sizeof(serv_addr));
Это от проекта кроссплатформенного - в линуксе есть реализация bzero, в винде нет. Там короче можно было переписать через memset и убрать этот метод совсем.
if (udpsocket < 0)Я не сталкивался с такой ошибкой ни разу, дельный совет
Тест окончательно провален.
Какой тест?
Tujh
02.09.2022 17:28Я не сталкивался с такой ошибкой ни разу, дельный совет
а надо было лишь посмотреть warning-и компилятора на максимальном уровене.
typedef unsigned int UINT_PTR, *PUINT_PTR; ... typedef UINT_PTR SOCKET;поэтому if ( <unsigned int> < 0 ) не имеет ни какого смысла и ни когда не сработает. Но и я ошибся за давностью лет...там не нулевое значение.
#define INVALID_SOCKET (SOCKET)(~0)Это от проекта кроссплатформенного - в линуксе есть реализация bzero, в винде нет.
Поэтому я и говорю - смесь API, более того, сами себе противоречите
Задача была в visual studio - значит в виндовс.
Ну и вот ещё
По задаче в ней не указано, что делать с таймаутом на сервере – по мне его бы тоже передать на сервер вначале не помешало.
Нет, не требуется, задача сервера только принять данные и отрапортовать что принято, задача клиента - передать всё, серверу не нужно знать таймаут клиента, дублирующиеся данные сервер должен просто отбрасывать.
Какой тест?
Который "за один день". Не думаю, что с таким решением вы пройдёте дальше, ну или на позицию не сильно выше джуниора.
AllKnowerHou Автор
02.09.2022 17:56Не думаю, что с таким решением вы пройдёте дальше, ну или на позицию не сильно выше джуниора.
А мне большего пока и не надо
AllKnowerHou Автор
02.09.2022 21:28-1Поэтому я и говорю - смесь API, более того, сами себе противоречите
Где я сам себе противоречю?
Раньше писал и под винду и под линукс, этот bzero остался из-за этого, давно было, теперь онли виндовс - потому что глюкнутые иде в линуксе выносят мозг
screwer
Код не смотрел.
Что бросилось в глаза:
не оберток над самими сокетами. Из-за этого обилие close/shutdown
плаформозависимый код (WSAxxxx) стоило бы вынести наружу. В линуксе он не нужен. Да и в Винде нужен один раз на процесс, не факт что именно в вашей подсистеме
закомментированный отладочный вывод. Лучше чтобы он отключался. Хотя бы и макросами, и навроде dprint_errot/dptint_info/etc., Но не оставлять закомментированного кода.
AllKnowerHou Автор
Жаль.
Задача была в visual studio - значит в виндовс. Плюс я больше не разрабатываю программы в линуксе - разлюбил его.