is this code reliable

Question

1.00/5 (1 vote)

See more:

Read function some times cause time out if time 1 or time 1000 although the server work well and get data in small time in command line
is the read function get all data in all cases or not
(unix aix)

C++

#include"TCPClient.h"
#include<fcntl.h>
#include<string>
using namespace std;
TCPClient::TCPClient()
{
signal(SIGPIPE,SIG_IGN);
  Socket = -1;
}
TCPClient::~TCPClient()
{
printf("\n tcp destroy \n");
 // close(Socket);

}
int TCPClient::Init()
{
 // Disconnect();
 
   printf("\n init \n ");
 
   	 Socket = socket(AF_INET,SOCK_STREAM,0);
         int flag = 1;
  
                          //SOL_SOCKET           //SO_RCVLOWAT SO_REUSEADDR|
	
	
	 if(Socket < 0)
	 {
	      printf(" \n fails create socket \n");
		// perror("\n fail to create Socket\n");
		 return -1;
	 }

    //zero(&data_socket,sizeof(data_socket));
     memset(&data_socket,0,sizeof(data_socket));
	return 1;
}
int TCPClient::Read(char* data,int len ,int timeout , char * delem,int delm_len,int &r)
{
 int l;
     timeval t_out;

         fd_set set;
   
      if(timeout<20)
       t_out.tv_sec = timeout;
      else 
      t_out.tv_sec = 35;

       t_out.tv_usec = 0;
       FD_ZERO(&set);   
       FD_SET(Socket,&set);
      
        l = select(Socket +1, &set, NULL, NULL, &t_out);
      
         setsockopt(Socket,SOL_SOCKET|SO_LINGER,SO_RCVTIMEO,(char*)&t_out,sizeof(struct timeval));

       if(l==0)
         {
          printf("\n time out %i \n",timeout);
          return -10;
         }
       
         else if(len<0)
          {
         printf("\n error in select 2 TCPClient \n");
       
           return -5;
          }
         
     char a[1] = {0};
    
       memset(data,0,sizeof(data));
      // strcpy(data," ");   
     ssize_t l1;
     int i = 0;
     int cycle = 0;
     while(true)
     {
     
          l1 = recv(Socket,a,1,0);
          if(l1==0)
          {
          printf("\n closed \n");
          i = 0;
            break;
          }
          data =  stpcpy(data,a);
          if(l1 == -1)
          {
            cycle++;
            if(r==5 && cycle<6)
             {
                sleep(1);
                printf("wait");
                    continue;
             }
             
            break;
          }
          i +=l1;
         
          if(delm_len>1 && strstr(data,delem)!=NULL)
          {
            printf("\n hit strstr \n  ");
            break;
          }
            else if(delm_len==1 && delem[0]==a[0])
            { 
            r = 1;
                break;
            }
            if(i>=len && r==1)
            break;
            
      }//while(i< len) ;    
      // printf("\n data recv %i  r = %i ",i,r);
       //printf("\n block data return %s \n",data);
 	return i; 
}


int TCPClient::Connect(char*address ,int port)
{
  Init();
  Bind();
  
    data_socket.sin_addr.s_addr = inet_addr( address );
    
    data_socket.sin_family = AF_INET;
  
    data_socket.sin_port = htons(port);
   
  int io_f = 0;
  ////
  timeval t_out;
  t_out.tv_sec = 3;
  t_out.tv_usec = 0;
  setsockopt(Socket,SOL_SOCKET,TCP_NODELAY|SO_REUSEADDR,(char*)&t_out,sizeof(struct timeval));

              //ioctl(Socket, FIONBIO , &io_f);
                ioctl(Socket, FIONREAD, &io_f);            
		printf("\n Icotol \n");
		 // fcntl(Socket,F_SETFL,O_NONBLOCK);
  
   if(connect(Socket,(struct sockaddr*)&data_socket,sizeof(data_socket))<0)
		   {

	             printf("Can't connect to %s ",address);
	             //perror("error to connect");
	             return -1;
		   }		   		
   return 1;
}

int TCPClient::Bind()
{

		   server.sin_addr.s_addr = INADDR_ANY;
		   server.sin_family = AF_INET;
		   server.sin_port  = htons(0);
		   int flag = 1;
		   setsockopt(this->Socket,IPPROTO_TCP,SO_REUSEADDR,(char*)&flag,sizeof(int));
	
		   if(bind(Socket,(struct sockaddr*)&server,sizeof(server))!=0)
		   {
			   printf("%s\n","failed bind Socket");

			   return -1;
		   }
		   sockaddr_in localaddress;
		  unsigned  long addrlen = sizeof(localaddress);
	int rc = getsockname(Socket,(struct sockaddr *)&localaddress, &addrlen);
	
		   
return 1;
}
void TCPClient::Disconnect()
{
  printf(" \n Disconnect() \n");
  shutdown(Socket,SHUT_RDWR);
  close(Socket);

}

Posted 6-Jun-15 11:46am

Mahmoud_Gamal

Updated 6-Jun-15 22:42pm

v2

Add a Solution

3 solutions

Solution 2

It doesn't look "reliable" to me. If I see things like:

C++

int TCPClient::Read(char* data,int len ,int timeout , 
        char * delem,int delm_len,int &r)
{

    memset(data,0,sizeof(data));
    ...

that doesn't look like the author knew what he was doing. Or another example is the code sequence:

C++

if(timeout<20)
 t_out.tv_sec = timeout;
else
t_out.tv_sec = 35;

That looks pretty much like experimental code and I would not include it in one of my programs before giving it a major re-write.

Posted 6-Jun-15 21:32pm

nv3

Comments

Mahmoud_Gamal 7-Jun-15 4:41am

ok but max wait is 35
and it work correctly for almost server expect some one
so i ask is the read function get all data in all cases or not
----
memset i use it dure to under it i read all data for single if memset remove
may be add return response of previous command + current command

nv3 7-Jun-15 5:54am

If you call your function with timeout=30 the timeout will be set to 35. That doesn't make sense. This is not the cause for the problem you describe, but it indicates the general sloppiness the code is written in.

As for the memset: It clears 4 or 8 bytes (depending on compilation settings). Remember that data is char* and sizeof(data) does not reflect the size of the array passed! This is also an indication that this code was written by someone without a clear understanding of C++.

Your question was: "Is this code reliable" and from what I have seen my answer is: No.

If your question is: Why does the read request sometimes timeout? I have no idea. It will probably take some debugging to find out.

Frankie-C 7-Jun-15 6:29am

5ed!

Mahmoud_Gamal 7-Jun-15 7:45am

thanks for that this problem occur where server restart and reset its configuration where problem are you think

Mahmoud_Gamal 7-Jun-15 6:31am

thanks alot
but i change code as you say
and the time out occur for some server although when use telnet command line it return data

Andreas Gieriet 7-Jun-15 15:35pm

My 5!
Cheers
Andi

nv3 7-Jun-15 16:54pm

Thanks, Andreas!

Solution 1

Ask yourself what your understanding of "reliably" is.
Once you distilled your answer, check your code yourself if it fulfills your expectations on reliable code.

You will see that you end up discussing the architecture and design of the solution as well as the implementation.

You will also end up running the code, i.e. to write test cases to execute all the control flow path you specify as crucial for the intended function of the code. Positive and negative tests, tests on robustness, through-put and (over-)load, etc.

Cheers
Andi

Posted 6-Jun-15 13:07pm

Andreas Gieriet

Comments

Mahmoud_Gamal 7-Jun-15 4:42am

it work correctly for almost server expect some one
so i ask is the read function get all data in all cases or not

Andreas Gieriet 7-Jun-15 16:02pm

Reading data from a network connection is in principle very straight forward: read the amount of data you expect to arrive.
The tricky part is the real world does not behave like this: you have many possible hiccups on the network connection that needs decent analysis and concept of handling deviations from the straight forward function.
E.g. you have to decide what to do if the network connection becomes "slow", or what if you are connected over wireless connection that may result in some "dropouts", or what if your server is heavily loaded and only responds in unpredictable intervals, or the server is (goes?) down while you are receiving data, etc.
All of these hiccups manifest themselves as "timeout".
It's your application/service that defines how you want to handle such timeouts - thus, my advise from above: you have to decide what "reliably" means in the context of your service.
E.g. you may give up receiving with the first timeout, or you define that each timeout event results in N retries after random intervals of T seconds, or you may decide to progressively increase the accepted timeout M times and only then give up.
In any case, you have to describe your approach and define test cases for your chosen approach. Admittedly, this is most likely implemented outside your read function. But nonetheless, you need to understand that network connections can be very instable and you need to be ready to handle these situations according to your expected quality of service.

I understand from other solutions and your replies that this is your very own read function. It seems that your code has several flaws that renders it not so usable (see other solutions)...

*No* read function will always get all data since you have the undelaying network not under control (see my incomplete list of possible hiccups above). So, you have to define what to do if the read function times out (which is a perfectly legal and common situation).

Cheers
Andi

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Jochen Arndt · Accepted Answer · 2015-06-07T00:17:00

Solution 3

There are multiple errors in the receive code:

C++

// A single character buffer.
// So just 'char a' can be used.
// But see below.
char a[1] = {0};
// ...
while(true)
{
    // This receives one byte
    l1 = recv(Socket,a,1,0);
    // stpcpy is a string function that expects null terminated strings.
    // But your buffer may be not null terminated.
    // It is just luck when a[1] is zero 
    // (due to alignment the compiler will not use the next 3/7 bytes)
    // Additionally, this will not work with binary data that contain
    //  null bytes.
    data =  stpcpy(data,a);

    // You are checking for an error after copying data and continue
    //  under specific circumstances.
    // But with errors, nothing has been stored in 'a' and you alredy have appended
    //  the previously received char.
    if(l1 == -1)
    {
        // ...
        continue;
    }
}

So you should at least ensure that your a buffer is null terminated by using a size of two chars and initialize it with null bytes:

C++

char a[2] = "\0";

In case of errors you should also get the error code errno and optionally print the error message (e.g. using strerror) to know what happened.

Posted 7-Jun-15 0:17am

Jochen Arndt

Comments

Mahmoud_Gamal 7-Jun-15 6:43am

ok thanks for this tips
but why it working for some server but other not
but when i use telnet command i work well i work on unix platform
when i reboot system this problem occur before thant there is nooo problem

Jochen Arndt 7-Jun-15 7:00am

This can't be really answered here.
Imagine that we don't have your servers and don't know about the type of data that are transferred.

But we can give you tips on how to solve the problem. That's what I and the others have tried.

So you should go on with what you have tried already:
Print all data that are received together with some states.
Print also the error codes and messages.

While writing this I just recognised another problem:
You are not using your fd_set variable. It is usually used with FD_ISSET to check if data are available. So your code may call recv() when ther are no data.

Mahmoud_Gamal 7-Jun-15 7:19am

is there configuration on aix solve this problem of timewait

Jochen Arndt 7-Jun-15 7:24am

I don't know AIX (only Linux). But there should be no difference because it looks more like a problem with your code than with the OS.

To solve such problems you must debug your code (in the simplest way by printing out your program states).

Mahmoud_Gamal 7-Jun-15 7:34am

i cant debug due to on unix i write on text and run code with display message it connect then send first command and send second then replay
when send third command may be connection loss or time out but when telnet normally it return data well
this problem suck me
thanks for response

Jochen Arndt 7-Jun-15 7:44am

A working Telnet indicates that the problem is in your code.
Printing out states is a simple form of debugging. It took some more time but can be used to find problems.

If you are still stuck, you may use example code from one of the many socket tutorials to check the receiving. If that is working, modify the code to implement your specific data handling.