ATL / WTL / STL

16-Sep-12 10:07

Following the OP, and per various tutorials I have found, I have created skeleton methods and virtual overrides everywhere I think there should be one I have also set breakpoints in all these methods. The results are not expected and quite interesting.

Note: The send time class is instantiated in the C_Server::Accept() method called by the Accept button in the dialog and passed into the Accept() method that I think is the one from the base class CAsyncSocket. Is that the correct action to perform?

After clicking the Connect for the Client class, the breakpoint is activated in C_Client::OnConnect( int nErrorCode)

The error code said the port was in use, but I am now thinking that maybe the error code should be expected and considered normal. Maybe I should write code in here for when the connect is successful. Per an example this method has code to call CAsyncsocket::OnConnect( int x ); I presume that is the base class. That function has no code. Should there be anything there?

Click Run in the debugger to see what happens and,…

The breakpoint in C_Server::OnAccept( int error_code ) is called. Thanks to an example I found, it calls CAsyncSocket::OnAccept( error_code ) which again is empty. I am now presuming that despite the earlier error code, the server code, the part within the Windows API that I don’t see, has indeed accepted the connection and it ready to send data to and receive data from the client. Is this indeed the case? If so, is there something that should be done in this method?

Clicking the debugger run one more time,
The breakpoint in C_Client::OnSend( int error_code ) is called.

Now this is puzzling. My code does not send anything. Is this telling me the client is expected to send something to the server? What should it send?

Thanks for your time

-- modified 16-Sep-12 16:45pm.

Richard Andrew x6416-Sep-12 11:53

Richard Andrew x64

16-Sep-12 11:53

bkelly13 wrote:
Then the buttons for the client are selected to Initialize, wsa error = 0, then Connect to the same IP address and port. It gets wsa error code 10048, WSAEADDRINUSE, address already in use. Hmmm. Yeah, it is in use, but if it is not in use by a server then the client cannot connect.

Show the code that does the Initialize and Connect.

The difficult we do right away...
...the impossible takes slightly longer.

pasztorpisti16-Sep-12 12:05

16-Sep-12 12:05

Well, I've done some TCP socket programming in the past but haven't used the CAsyncSocket class. What is interesting for me is that listen() returns EWOULDBLOCK for you. listen() should never return EWOULDBLOCK if you take a look at its documentation. listen() is always blocking but only for a very short time, it either returns failure or does its job (establises incoming connection request queue, the backlog) and returns with success.
So whats wrong with your code? Here is my guess: After starting the listen you unnecessarily issue some other request on your listen socket (for example an accept() call) and that results in a WSAEWOULDBLOCK message that is not still not the end of the world, but strange and you should check out the reason for this. About the WSAEADDRINUSE problem: If your listening starts as expected then it allocates and uses a port on your machine. When you connect the client side should issue a connect() call that results in the following events: your server side receives an event and as a result calls accept(). Both the server side accept() call and the client side connect() call returns a socket that has random ports assigned. HOWEVER: If your misterious CAsyncSocket class calls bind() before the connect() call, then it will fix the port of the returned socket by the connect() call and if that is the same as the port on which your server is listening then the socket can not be created on the client side! You should make sure that bind isn't called before connect() allowing the connect() call to assign a random port for the client side socket! bind() is usually used before connect when your firewall allows only specific ports for the software to communicate.
Another side note: WSAEADDRINUSE might return if you shut down your program during debugging. In this case some of your sockets might stay alive inside the network subsystem because of lingering. In this case sometimes you have to wait for minutes for that socket object to diminish from the network subsystem so as to release your ports.

bkelly1316-Sep-12 12:55

16-Sep-12 12:55

Oops. It is the Accept() that returns the would block WSA code, not Listen. My appologies.
Here are some code fragments:

Omitting the error checking and reporting, the server initialize does this

C++

bool C_Server::Class_Initialize()
{ ...
  m_winsock_status = AfxSocketInit();
...
  m_winsock_status = Create( m_port_number,  // value is 49000
                             SOCK_STREAM,
                             FD_READ | FD_WRITE | FD_OOB | FD_ACCEPT | FD_CONNECT | FD_CLOSE,
                             DEFAULT_IP_ADDRESS );  // value is 127.0.0.1
...}

the Listen() method does this:

C++

bool C_Server::Class_Listen()
{
   m_winsock_status = Listen();
... }

And the Accept() looks like this:

C++

bool C_Server::Class_Accept()
{
   mp_C_Server_Send_Time_Socket = new C_Server_Send_Time_Socket;
   m_winsock_status = Accept( *mp_C_Server_Send_Time_Socket );
...}

I have continued work and am partway through the Client code. The client initialize seems to be OK, the client Connect() gets WSA code 10035, socket in use. I am beginning to think that is to be expected.

Advice?

Thanks for your time
Edit, I keep messing with the formatting trying to get the code blocks to display correctly. Having some difficulties there. Preview looks good, but when I post it does not. I think I have it, disabled the checkbox to treat my content as plain text. But this darn edit box keeps jumping around in the text.

pasztorpisti16-Sep-12 13:09

16-Sep-12 13:09

You say you have problems with connecting and you copy paste everything here except the client code and connect. First go and check whether you (or your CAsyncSocket class) issue a bind() call to the client socket before the connect call.

bkelly1316-Sep-12 13:17

16-Sep-12 13:17

[QUOTE]First go and check whether you (or your CAsyncSocket class) issue a bind() call to the client socket before the connect call.[/QUOTE]

No it does not. So it should Call the Bind in the Client before the Connect?

Thanks for your time

Edit: What is the code to mark a quote?

Edit: according to an MSDN page, Create() calls socket and if sucessful calls Bind. If it calls Bind, why should I explicitly call bind.
http://msdn.microsoft.com/en-us/library/xz019029(v=vs.80).aspx[^]

pasztorpisti16-Sep-12 13:34

16-Sep-12 13:34

No it shouldnt call bind because that could cause WSAEADDRINUSE as I described previously.

bkelly1316-Sep-12 14:07

better version of strcpy() and printf()

16-Sep-12 14:07

Well then, I think I was doing things out of order. My dialog has all the pieces broken out so I performed the following steps:

Server::Initialize, wsa = 0
Client::Initialize, wsa = 0
Server::Listen, wsa = 0
Client::Connect wsa = 10035
Client::OnConnect() is called
Server::OnAccept() is called
Client::OnSend() is called

As noted earlier, I have all those methods defined but with no body and a breakpoint in each. Is this normal behavior? If so,...
Does Client::OnConnect() need to do anything?
Same for Server::OnAccept and Client::OnSend?

I am getting close. Next Step?

Thanks for your time

Edit again: I have this code in the function C_Server::Class_Accept() that is called via a button in the dialog.

C++

bool C_Server::Class_Accept()
{
   mp_C_Server_Send_Time_Socket = new C_Server_Send_Time_Socket;
   m_winsock_status = Accept( *mp_C_Server_Send_Time_Socket );
}

Now I that I see C_Server::OnAccept() called, that code should be moved into the OnAccept method and executed automatically after the client connects. Then I would NOT have a manually called Accept() method in the server class.

Does that sound right?

modified 16-Sep-12 20:16pm.

bkelly1315-Sep-12 10:02

15-Sep-12 10:02

There are numerous versions of the basic copy function, too many now. some of the new versions are declared safer because they incorporate a limiting count value. However, all the ones I have tried and inadvertenly tested just crash the application. I find that rather rude.

Are there any versions of ...cpy...() and ...printf...() that limit the count of characters copied to the space found in the destination string. Without an ungracefull app crash?

Thanks for your time

Paul M Watt15-Sep-12 12:33

Paul M Watt

15-Sep-12 12:33

bkelly13 wrote:
Are there any versions of ...cpy...() and ...printf...() that limit the count of
characters copied to the space found in the destination string.
Without an ungracefull app crash?

Unfortunately, No. Strcpy and printf are two of the most common functions that hackers use to exploit programming errors to "hack" into a program. When using these functions you need to pay extra attention to the size of your memory buffers and the actions you are performing on them.

You may have better luck with the C++ Standard Library.

The std::string class will manage your memory buffers, copies and concatenations.
std::cout doesn't have the program crash issues that you are running into.

All of my software is powered by a single Watt.

bkelly1315-Sep-12 15:34

15-Sep-12 15:34

In that case I wish to go on record as saying that we need a new set of these functions. These functions should be named ...strcpy_l(...) and ...printf_l(...), with the "l" (lower case L) standing for Limited. Under the covers, the function will examine the destination, determine the length of the destination, and quietly refuse to move more characters than the destination string can hold.

Thanks for your time

Paul M Watt15-Sep-12 15:45

Paul M Watt

15-Sep-12 15:45

I misunderstood part of your question.

There are these two functions:

strncpy(pDest, pSrc, size)

and

snprintf(pDest, count, pFormatStr, ...)

These will do what you are asking. Read the descriptions though, strncpy will not NULL terminate your string if it can't write all of the data to the buffer. This would open yourself up to another set of possibilities to crash your program.

All of my software is powered by a single Watt.

bkelly1315-Sep-12 16:23

15-Sep-12 16:23

Those sound like an improvement, but I claim both are defective. If they leave the string in an known improper state, then that is a serious problem.

Thanks for your time

pasztorpisti16-Sep-12 6:54

16-Sep-12 6:54

I think both strncpy() and snprintf() sucks. Once I searched for hours for a bug that was caused by strncpy(). Here is the problem: strncpy() is suboptimal because even if you copy a 1 byte string, it fills the whole remaining part of the destination buffer with zeros unnecessarily, and on the other hand it doesn't zero terminate the string if its length equals to or bigger than the size of the dest buffer. The missing zero terminator character in similar cases is true for snprintf() too. I highly discourage everyone from using these functions.

Richard MacCutchan16-Sep-12 0:10

16-Sep-12 0:10

bkelly13 wrote:
In that case I wish to go on record as saying that we need a new set of these functions.

It's no good making statements like this on CodeProject, you need to go and complain to the standards body.

One of these days I'm going to think of a really clever signature.

Richard Andrew x6415-Sep-12 17:01

Richard Andrew x64

15-Sep-12 17:01

You might find this article useful. It lists the unsafe string functions along with safe replacements:

http://msdn.microsoft.com/en-us/library/bb288454.aspx[^]

The difficult we do right away...
...the impossible takes slightly longer.

Richard MacCutchan16-Sep-12 0:09

16-Sep-12 0:09

I did make a suggestion as to how you may alleviate this in my answer to your previous question.

One of these days I'm going to think of a really clever signature.

pasztorpisti16-Sep-12 1:37

16-Sep-12 1:37

Here is my opinion: strcpy is totally useless and totally unsafe. Use a string class in C++ and assign one string to another instead of strcpy. You can even implement refcounting if you write your own string class and then assignment will be super fast. You can of course use std::string for your needs.

If you write you own string class or if you extend your own string class from std::string then you can put in a format and vformat function (similarly to the MFC CString class) and you can do formatting so that the buffer size will be managed automatically for you. Here is an example to that: How to Format a String[^]. I would mention only one thing regarding this formatting function in the link: The use of the std::vector is unnecessary, you could put the string formatting directly to the string. even in that case the string resizing unnecessarily fills the whole string with a fill character before formatting the string into its buffer. With a custom string you can write more optimal code for this task.

The type safety of format parameters with the vararg nature of format functions is still endangering your program. Using the bad parameter types make your program crash at runtime when the formatting code gets executed. Example to a buggy formatting statement: format("Name: %s\n", 5). Gcc has an __attribute__ format printf to defend against such mistakes at compile time. You can mark you format function with this attribute and then gcc will catch such errors, it would find out for exmple that "%s" and 5 are not compatible. Unfortunately visual C++ doesn't have such feature but our current project is crossplatform so we compile it with both VC++ and gcc and this gcc feature is priceless.

Unfortunately this kind of formatting is too widespread and comfortable without similarly comfortable alternatives so this wont be dropped easily from C++. However a feature like the one gcc has can make this stuff quite usable and safe.

get a number into static text control

Stephen Hewitt18-Sep-12 0:29

Stephen Hewitt

18-Sep-12 0:29

You could try something like Boost's format[^] library.

Steve

bkelly1313-Sep-12 17:05

13-Sep-12 17:05

Environment: Windows 7, Visual Studio 2008, MFC, CPP

Goal: start with an int, and put the character string equivilant into a text format such that a static control variable from a dialog will accept it. mp_C_Client is a pointer to a class that contains the TCP/IP code. The error code is returned as expected.l

C++

void C_MFC_Winsock_01_Dlg::OnBnClickedBtnClientInit()
{
    bool method_status = false;
    int wsa_error_code = 0;
    TCHAR wsa_error_text[ 16 ];

    mp_C_Client = new C_Client;
	
    if( mp_C_Client != NULL )
    {
       method_status = mp_C_Client->Class_Initialize();
       wsa_error_code = mp_C_Client->Class_Get_WSA_Error_Code();
       wprintf( wsa_error_text, "%d", wsa_error_code );           // <<<< this line
       m_client_wsa_error_text.SetWindowText( wsa_error_text );
    }

Intellisense shows the control variable argument as:

C++

m_client_wsa_error_text.SetWindowText( LPCTSTR lpszstring )

This compiles but after the wprintf the text in wsa_error_text is not as expected.
How should this be coded?

Thanks for your time

Edit: This post looked pretty good in the preview, but not good after I posted it.
And again: But now its okay. ????

Richard MacCutchan13-Sep-12 21:52

13-Sep-12 21:52

bkelly13 wrote:
wprintf( wsa_error_text, "%d", wsa_error_code ); // <<<< this line

You should be using _stprintf()[^] to format the string into a buffer.

One of these days I'm going to think of a really clever signature.

bkelly1314-Sep-12 15:00

14-Sep-12 15:00

That got me where I needed to go. I used this:

C++

int wsa_error_code;
TCHAR wsa_error_text[ 16 ];
...
_stprintf_s( wsa_error_text, 15,  L"%d", wsa_error_code );

Note the _s on the end and the L prefixing the "%d"
This nonsense with all the different character types is way out of hand. After I get over this hurdle of writing some async TCP/IP code, I think I should re-create the project and get rid of all the multibyte and unicode nonsense.

Thanks for your time
Edit: The post was nicely formated in the preview, but looks bad after being submitted.

Paul M Watt14-Sep-12 15:36

Paul M Watt

14-Sep-12 15:36

Unicode is the default compile type for Visual Studio projects.
If you would like to work in multi-byte only, you can change that property in your compiler settings on the General page.

The _stprintf_s function compiles using the character format based on that compiler setting.

If you don't want to mess with the compiler settings, you can use a char rather than a TCHAR, and the function sprintf_s,
or if you want to force the unicode type use the data type wchar_t, and the function wsprintf_s.

All of my software is powered by a single Watt.

Richard MacCutchan14-Sep-12 23:02