VPF::Хороший код или нет? - Форум программистов

SIRIUStar

Дата 23.7.2007, 18:44 (ссылка)

(нет голосов)

Загрузка ...

Шустрый

Профиль
Группа: Участник
Сообщений: 71
Регистрация: 20.7.2007
Где: Одинцово

Репутация: нет
Всего: нет

Здравствуйте люди! Вот попробовал себя в сетевом программировании) Прошу посмотреть что у меня вышло! Программа вполне работает и делат то что я от нее хотел) Но можно ли так делать в больших приложениях или нет и вообще есть ли здесь ошибки, огрехи, неточности? Если кто знает как сделать более совершеный код, пожалуйста, сообшите мне smile

я в сетевом программировании новичек, и хотелось бы получить несколько советов))

Клиент соединяется с сервером и кидает на него строки текста, на сервере же в консоль выводятся эти строки в виде ip:port<N клиента>Nick: строка

Как вы поняли возможно подключение нескольких клиентов))

Код Сервера

Код


//---------------------------------------------------------------------------

#pragma hdrstop
#include <windows.h>
#include <windowsx.h>
#include <winsock.h>
#include <iostream>
#include <conio.h>
#include <cstring>

//---------------------------------------------------------------------------
using namespace std;

struct __Client__
{
  int id;
  SOCKET cs;
  sockaddr_in cladrr;
};

DWORD WINAPI ThreadFunc(
  LPVOID lpParameter)
{
    char Stemp[255];
    __Client__ *clinfo = ( __Client__*)lpParameter;

    cout <<  inet_ntoa(clinfo->cladrr.sin_addr) <<
             ":" << ntohs(clinfo->cladrr.sin_port) << "<" <<
             clinfo->id << ">" << " Connected!" << endl;
    while(true)
    {
        if(recv(clinfo->cs, Stemp, 256, 0) == SOCKET_ERROR) //ôóíêöèÿ ïðè¸ìà ñîîáùåíèé
        {
           closesocket(clinfo->cs);
           cout <<  inet_ntoa(clinfo->cladrr.sin_addr) <<
                    ":" << ntohs(clinfo->cladrr.sin_port) << "<" <<
                    clinfo->id << ">" << " Disconnected..." << endl;
           break;
        }
        cout << inet_ntoa(clinfo->cladrr.sin_addr) <<
                ":" << ntohs(clinfo->cladrr.sin_port) << "<" <<
                clinfo->id << ">" << Stemp << endl;
    };
    delete clinfo;

    return 1;
}


#pragma argsused
int main(int argc, char* argv[])
{
  if(argc != 3) return 1;
  WSADATA wsadata;
  SOCKET s;
  int PORT;
  sockaddr_in servsin;
/*  cout << argc << endl;;
  cout << argv[1] << endl;
  cout << argv[2] << endl;
*/
  sscanf(argv[2], "%d", &PORT);

  ZeroMemory(&servsin, sizeof(servsin));
  servsin.sin_family = AF_INET;
  servsin.sin_addr.s_addr = inet_addr(argv[1]);
  servsin.sin_port = htons(PORT);

  if (WSAStartup(MAKEWORD(1,1), &wsadata) != 0) return(1);

  //ñîçäàíèå ñîåäèíåíèÿ äëÿ ïðè¸ìà äàííûõ
  s = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);

  //ïðèâÿçûâàíèå ñîçäàííîãî ñîåäèíåíèÿ ê ïîðòó è ê IP àäðåñó
  if (bind(s, (sockaddr*)&servsin, sizeof(servsin)) == SOCKET_ERROR)
   {
      closesocket(s);
      s = 0;
      cout << "Cannot bind the socket" << endl;
      WSACleanup();
      getch();
      return EXIT_FAILURE;
   }

  //èíèöèàëèçàöèÿ äëÿ ïðè¸ìà äàííûõ
  if (listen(s, 10) == SOCKET_ERROR)
   {
     closesocket(s);
     s = 0;
     cout << "Cannot listen socket" << endl;
     WSACleanup();
     getch();
     return EXIT_FAILURE;
   }

  sockaddr_in SIN;
  SOCKET S2 = -1;
  DWORD TreadId;
  __Client__ *clinfo;
  int i = 0;
  int size = sizeof(SIN);

  cout << "Server in " << argv[1] << ":" << argv[2]
       << " build: waiting connection..." << endl;
  while(true)
  {
    S2 = accept(s, (sockaddr*)&SIN, &size);

    i++;
    clinfo = new __Client__;

    clinfo->id = i;
    clinfo->cladrr = SIN;
    clinfo->cs = S2;
    CreateThread(NULL, 0, (LPTHREAD_START_ROUTINE)ThreadFunc,
                         (PVOID)clinfo, 0, &TreadId);
  };

  closesocket(s);
  WSACleanup();

  return 0;
}
//---------------------------------------------------------------------------

Код клиента

Код


//---------------------------------------------------------------------------
#include <windows.h>
#include <windowsx.h>
#include <winsock.h>
#include <iostream>
#include <conio.h>
#include <cstring>
#pragma hdrstop

//---------------------------------------------------------------------------
using namespace std;
#pragma argsused
bool CompStr(char *str1, char *str2)
{
  int i = 0;
  if(strlen(str1) != strlen(str2)) return false;

  while(str1[i])
  {
    if(str1[i] != str2[i])
      return false;
    i++;
  };
  return true;
};


int main(int argc, char* argv[])
{
  WSADATA wsadata;
  char IPaddres[20];
  char UserName[255];
  char InStr[255];
  int PORT;
  SOCKET s;
  sockaddr_in clsin;

  cout << "IP: ";
  cin >> IPaddres;
  cout << "Port: ";
  cin >> PORT;
  cout << "Name: ";
  cin >> UserName;
  cout << endl;

  ZeroMemory(&clsin, sizeof(clsin));
  clsin.sin_family = AF_INET;
  clsin.sin_addr.s_addr = inet_addr(IPaddres);
  clsin.sin_port = htons(PORT);

  if (WSAStartup(MAKEWORD(1,1), &wsadata) != 0) return(1);

  //ñîçäàíèå ñîåäèíåíèÿ
  s = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);

  cout << "Connecting to " << IPaddres << ":" << PORT << "...";
  if(connect(s, (sockaddr *)&clsin, sizeof(clsin)))
  {
    closesocket(s);
    WSACleanup();
    cout << "NO" << endl;
    getch();
    return EXIT_FAILURE;
  };
  strcat(UserName, ": ");

  cout << "OK" << endl;
  while(true)
  {
    char UserMessage[255] = "\0";
    cin >> InStr;
    strcat(UserMessage, UserName);
    strcat(UserMessage, InStr);
    if(CompStr(InStr, "quit"))
    {
       return 0;
    };

    if(send(s, UserMessage, 256, 0) == SOCKET_ERROR)
    {
      cout << "Server lost..." << endl;
      closesocket(s);
      WSACleanup();
      getch();
      return EXIT_FAILURE;
    };
  };

  closesocket(s);
  WSACleanup();

  return 0;
}
//---------------------------------------------------------------------------

Это сообщение отредактировал(а) SIRIUStar - 23.7.2007, 18:46

leniviy

Дата 27.7.2007, 15:31 (ссылка)

(нет голосов)

Загрузка ...

Опытный

Профиль
Группа: Участник
Сообщений: 552
Регистрация: 8.2.2003
Где: Спб

Репутация: нет
Всего: 5

Попробуй погонять сервер, при этом в таскменеджере следи за количеством дескрипторов. Оно будет нарастать, так как ты не вызываешь CloseHandle(hThread);

maxim1000

Дата 27.7.2007, 18:02 (ссылка)

(нет голосов)

Загрузка ...

Эксперт

Профиль
Группа: Участник
Сообщений: 3334
Регистрация: 11.1.2003
Где: Киев

Репутация: нет
Всего: 110

сервер не смотрел, посмотрел клиент:
1. CompStr: в общем-то можно было бы воспользоваться стандартной функцией сравнения, но если уж писать свои функции такого рода, то лучше сравнивать до нулевого символа, а то получается двойная работа (прохождение до 0 в strlen + прохождение до конца для сравнения)
2. если сокет не сможет соединиться, произойдёт return EXIT_FAILURE, а сам сокет не удалится
3. похожая ситуация с WSACleanup()
(хотя эти две ситуации учтены в обработке ошибки посылки)

т.к. используется C++, можно использовать следующий способ захвата/освобождения ресурсов:
описываем класс
в конструкторе выделяем ресурс
в деструкторе его освобождаем

это позволяет просто бросать исключения при ошибках, при этом все ресурсы, выделенные таким образом, будут нормально освобождены

пример:

Код


class CWSAInitializer
{
    public:
        CWSAInitializer()
        {
            WSADATA wsadata;
            if (WSAStartup(MAKEWORD(1,1), &wsadata) != 0)
                throw "всё пропало! :)";
        }
        ~CWSAInitializer()
        {
            WSACleanup();
        }
};
...
main
{
    try
    {
        WSAInitializer initializer;
        ...
        if(<ошибка срединения, например>)
          throw "ошибка соединения"; //здесь будет автоматически вызван деструктор WSAInitializer, который вызовет WSACleanup
        ....
    }
}

соответственно можно и закрытие сокета сделать

Это сообщение отредактировал(а) maxim1000 - 27.7.2007, 18:03

--------------------

qqq

SIRIUStar

Дата 28.7.2007, 19:18 (ссылка)

(нет голосов)

Загрузка ...

Шустрый

Профиль
Группа: Участник
Сообщений: 71
Регистрация: 20.7.2007
Где: Одинцово

Репутация: нет
Всего: нет

Спасибо за предложения! Хорошая идея с С++ кодом! smile

Один вопрос: что такое таскменеджер и где его найти в билдере?? smile

Это сообщение отредактировал(а) SIRIUStar - 28.7.2007, 19:19

JackYF

Дата 28.7.2007, 20:17 (ссылка)

(нет голосов)

Загрузка ...

полуавантюрист

Профиль
Группа: Участник
Сообщений: 5814
Регистрация: 28.8.2004
Где: страна тысячи озё р

Репутация: 1
Всего: 162

Цитата(SIRIUStar @ 28.7.2007, 19:18

)

акое таскменеджер и где его найти в билдере?? smile

таскменеджер - это приложение Винды (ну или другой ОС) и к Билдеру не имеет никакого отношения.
Нажми Ctrl+Alt+Del - и оно появится, если у тебя Винда.

--------------------

Пожаловаться на меня как модератора можно здесь.

ptr

Дата 29.7.2007, 06:42 (ссылка)

(нет голосов)

Загрузка ...

Опытный

Профиль
Группа: Участник
Сообщений: 661
Регистрация: 31.5.2006
Где: Новосибирск

Репутация: 9
Всего: 14

Цитата(SIRIUStar @ 23.7.2007, 21:44

)

servsin.sin_addr.s_addr = inet_addr(argv[1]);

Вместо конкретного адреса всегда лучше использовать INADDR_ANY.
2. При нормальном отсоединении другой стороны recv возвращает 0.
3. recv не завершает строку '\0'. Поэтому надо делать это самому:

Код


int retVal = recv(clinfo->cs, Stemp, 256, 0);
if(retVal==0){}
if(retVal==SOCKET_ERROR){}
Stemp[retVal]='\0';

4. recv и send не получают/отсылают всей строки, поэтому чтобы отослать всю строку надо вызывать send/recv в цикле.
5. Проверка возвращаемых значений socket, accept, CreateThread.
6.

Цитата(SIRIUStar @ 23.7.2007, 21:44

)

SOCKET S2 = -1;

Что такое -1 для SOCKET?
7.

Цитата(SIRIUStar @ 23.7.2007, 21:44

)

Но можно ли так делать в больших приложениях или нет

Не стоит. В больших приложениях применяются другие методы (часто комбинированные). У тебя, например нет контроля количества потоков, поэтому в некоторый момент потоков может не хватить.
Да и не особо эффективно создавать на каждого клиента по потоку.
8.

Цитата(SIRIUStar @ 23.7.2007, 21:44

)

if(send(s, UserMessage, 256, 0) == SOCKET_ERROR) {

Вместо 256 стоит поставить strlen(UserMessage)
9. Да, чуть не забыл. Переменные при объявлении стоит инициализировать.

Это сообщение отредактировал(а) ptr - 29.7.2007, 20:03

--------------------

Единственный способ определить границы возможного - это выйти за эти границы, в невозможное.
Артур Кларк.

SIRIUStar

Дата 11.9.2007, 14:21 (ссылка)

(нет голосов)

Загрузка ...

Шустрый

Профиль
Группа: Участник
Сообщений: 71
Регистрация: 20.7.2007
Где: Одинцово

Репутация: нет
Всего: нет

Спасибо все понял! спасибо за участие) Доброго времени! smile

0 Пользователей читают эту тему (0 Гостей и 0 Скрытых Пользователей)
0 Пользователей:
« Предыдущая тема \| C/C++: Сети \| Следующая тема »