From: Michael K. O'Neill on
"Vinoj" <Vinoj(a)discussions.microsoft.com> wrote in message
news:21D3AC8A-8AB8-4051-BF14-8ED32922E5DF(a)microsoft.com...
> I'm using TCP/IP. I'm also sending/receivingbinary data (file transfer).
>
> The sending side sends an 'f' char immediately before a file is sent.
When
> the 'f' is received, the receive file routine is called (which receives
the
> file name - 256 chars, the file size, and then the file data). After
that,
> occasionally there will be no more data (expected), however, sometimes
there
> more data on the socket. The receive file routine follows (adapted from
Mike
> O'Niell's CSocket file transfer tutorial). Thanks.
>
> Vinoj

The code you posted does not execute an "asynchronous" file transfer.
Rather, the code is ocmpletely synchronous, since it executes, seriatim,
reception of file name, followed by size, followed by data; and it does not
complete until all tasks are also complete.

An asynchronous file transfer would perform a single call to Receive() in
response to an FD_READ notification (i.e., inside your
CAsyncSocket::OnReceive() override), and then exit after updating your state
machine, to await again a new FD_READ notification. The serial tasks shown
in your function (below) would be "cobbled together" after multiple calls to
OnReceive(), based on logic in your state machine.

The various calls to Sleep() were probably needed to make the code "work",
in the sense that a file was successfully received without a
WSAGetLastError() of WSAEWOULDBLOCK. This alone should be enough to
demonstrate to you that the code is not asynchronous, but rather very
synchronous and sequential.

As for "garbage being sent after data", if you are using
http://www.codeproject.com/internet/SocketFileTransfer.asp , then nothing at
all is being sent after the data, since the sending code specifically sends
less than a full buffer at the end of a file (it only sends the final part
of the file, which usually is less than a full buffer). On the receiving
side, at the end of the file, the code is designed to write to the output
file only the number of bytes remaining in the file, and not the entire
buffer (i.e., the code writes to the file based on iiRecd (which is the
number of bytes received) and not based on RECV_BUFFER_SIZE (which is the
size of the buffer)).

So, the only source of this behavior is if you are sending multiple files,
and you do not correctly handle the point of demarcation between sending of
the current file and that of a prior file.

See specific notes on your code, below.

Mike


>
> BOOL AsyncReceiveFile(CString FilePath){
>
> BOOL bRet = TRUE; // return value
> int dataLength, cbBytesRet, cbLeftToReceive, loop = 0;
> BYTE* recdData = NULL;
>
> CFile destFile;
> char buff[256];
> BOOL bFileIsOpen = FALSE;
> Sleep(200);
> CString strFileName;
>
> //receive the filename
> memset(buff, 0, sizeof(buff));
> cbLeftToReceive = MAX_FILENAME_LENGTH;
> do{
> cbBytesRet = pAsyncClient->Receive(buff, cbLeftToReceive, 0);


This code is wrong, since it always loads the most-recently-received part of
the file name to the beginning of the buffer. It should be something like
this:

cbBytesRet = pAsyncClient->Receive(buff+MAX_FILENAME_LENGTH-cbLeftToReceive,
cbLeftToReceive, 0);



> if(cbBytesRet > 0)
> cbLeftToReceive -= cbBytesRet;
> else if(++loop > 1000)
> return FALSE;
> }
> while(cbLeftToReceive >0);


This test makes sense only if the file name is always exactly
MAX_FILENAME_LENGTH in length, and only if both sides agree on the value of
MAX_FILENAME_LENGTH. As a better alternative, why not prefix the file name
with the byte-count of the nae itself, much like you do when sending the
file's data.



>
> strFileName = buff;
> // open/create target file that receives the transferred data
>
> if(!(bFileIsOpen = destFile.Open((LPCTSTR)strFileName,
> CFile::modeCreate|CFile::modeWrite|CFile::typeBinary))){
> MessageBeep(MB_ICONEXCLAMATION);
>
> // Open Error Log File and Record Problem
> }
> return FALSE;
> }
>
> // get the file's size
> Sleep(200);
> cbLeftToReceive = sizeof(dataLength);
> loop = 0;
>
> do{
> BYTE* bp = (BYTE*)(&dataLength) + sizeof(dataLength) -
> cbLeftToReceive;
> cbBytesRet = pAsyncClient->Receive( bp, cbLeftToReceive );
>
> // test for errors and get out if they occurred
> if(cbBytesRet == SOCKET_ERROR || cbBytesRet == 0){
> int iErr = ::GetLastError();
> if(iErr != WSAEWOULDBLOCK){
>
> MessageBeep(MB_ICONEXCLAMATION);
>
> // Open Error Log File and Record Problem }
>
> return FALSE;
> }
> cbLeftToReceive += cbBytesRet;
>
> //if data cannot be read
> if(++loop > 1000)
> return FALSE;
> }
>
> cbLeftToReceive -= cbBytesRet;
>
>
>
> }
> while (cbLeftToReceive > 0);
>
> if((dataLength = ntohl( dataLength )) < 0){
> MessageBeep(MB_ICONEXCLAMATION);
>
> // Open Error Log File and Record Problem
> }
> return FALSE;
> }
>
>
> // now get the file in RECV_BUFFER_SIZE chunks at a time
> Sleep(200);
> recdData = new byte[BUFFER_SIZE];
> cbLeftToReceive = dataLength;
> loop = 0;
> do{
> int iiGet, iiRecd;
>
> iiGet = (cbLeftToReceive<BUFFER_SIZE)?cbLeftToReceive:BUFFER_SIZE
;
> iiRecd = pAsyncClient->Receive( recdData, iiGet );
>
> // test for errors and get out if they occurred
> if (iiRecd == SOCKET_ERROR || iiRecd == 0){
> int iErr = ::GetLastError();
> if(iErr != WSAEWOULDBLOCK){
> //Record error
>
> delete [] recdData;
> return FALSE;
> }
> cbLeftToReceive += iiRecd;
>
> if(++loop > 1000)
> return FALSE;
> }
>
> // good data was retrieved, so accumulate
> // it with already-received data
> if(iiRecd > 0)
> destFile.Write( recdData, iiRecd); // Write it
> cbLeftToReceive -= iiRecd;
>
>
> }
> while ( cbLeftToReceive > 0 );
>
>
> delete[] recdData;
>
> if ( bFileIsOpen )
> destFile.Close();
>
> return bRet;
> }
>
> "Joseph M. Newcomer" wrote:
>
> > Are you using TCP/IP or UDP?
> >
> > Are you making sure that, if you are handling strings, that you insert a
terminating NUL
> > character after the characters received? For example, if you have a 1K
buffer, receive 4
> > bytes representing 4 8-bit characters, then you must put a NUL in [4] a
NUL character, or
> > whatever is seen there will be seen as part of the string.
> > joe
> >
> > On Tue, 10 Jan 2006 16:03:02 -0800, "Vinoj"
<Vinoj(a)discussions.microsoft.com> wrote:
> >
> > >Hello,
> > >
> > >I have two programs that use async sockets. Occasionally, there is
garbage
> > >(extra bytes) being sent after data, thus confusing my finite state
machine.
> > >Is there a way to track everytime the send() is called? Any ideas on
why
> > >this could be happening?
> > >
> > >I don't know if it matters, but the sending side of the program is
written
> > >in C and the receiving side is MFC. Thanks.
> > >
> > >Vinoj
> > Joseph M. Newcomer [MVP]
> > email: newcomer(a)flounder.com
> > Web: http://www.flounder.com
> > MVP Tips: http://www.flounder.com/mvp_tips.htm
> >


From: Michael K. O'Neill on

"Vinoj" <Vinoj(a)discussions.microsoft.com> wrote in message
news:B6236102-C952-409E-85C2-75DBCB2DDE42(a)microsoft.com...
> As far as I understand - if the return value for Receive is -1, there's an
> error, and if it's 0 or greater - that reflects the number of bytes read,
> correct?
>

Not exactly.

Receive returns: (1) SOCKET_ERROR (which is #defined as -1) for an error;
(2) 0 if the connection has been closed; and (3) the number of bytes copied
to your buffer if successful.


From: Scott McPhillips [MVP] on
Vinoj wrote:
> I'm using TCP/IP. I'm also sending/receivingbinary data (file transfer).
>
> The sending side sends an 'f' char immediately before a file is sent. When
> the 'f' is received, the receive file routine is called (which receives the
> file name - 256 chars, the file size, and then the file data). After that,
> occasionally there will be no more data (expected), however, sometimes there
> more data on the socket. The receive file routine follows (adapted from Mike
> O'Niell's CSocket file transfer tutorial). Thanks.
> <mucho code>

Your first call to Receive does not check for WSAEWOULDBLOCK. Your
second and third calls to Receive do check for it but they exit if it
occurs. This is wrong. WSAEWOULDBLOCK can happen in the normal course
of receiving (and sending) socket data. What do you expect to happen if
you call Receive before the desired data has been received?

Micheal's comments are also valid. I don't know why your result appears
to be extra data, and I did not analyze your code in detail, but the
design of your routine is not compatible with the design of
CAsyncSocket. (Your routine is compatible with the design of CSocket.)
There is a knowledgebase article that says there should be only one
call to CAsyncSocket::Receive() for each call to your OnReceive().

Your receive code is attempting to "pull" data from the socket, but
async sockets don't work that way. Instead, OnReceive notifies you when
some data is available, and you should Receive that data and then return
until the next call to OnReceive comes in. IOW, let the socket "push"
data to your code.

There is also a possibility that your extra data symptom is simply a
misunderstanding of debugger displays. If you display the data as if it
were a string you can see extra data because the buffer is not nul
terminated. I.e. it is not really a string.

--
Scott McPhillips [VC++ MVP]

From: Vinoj on
Thanks for the advice. I'll try taking it apart.

"Michael K. O'Neill" wrote:

> "Vinoj" <Vinoj(a)discussions.microsoft.com> wrote in message
> news:21D3AC8A-8AB8-4051-BF14-8ED32922E5DF(a)microsoft.com...
> > I'm using TCP/IP. I'm also sending/receivingbinary data (file transfer).
> >
> > The sending side sends an 'f' char immediately before a file is sent.
> When
> > the 'f' is received, the receive file routine is called (which receives
> the
> > file name - 256 chars, the file size, and then the file data). After
> that,
> > occasionally there will be no more data (expected), however, sometimes
> there
> > more data on the socket. The receive file routine follows (adapted from
> Mike
> > O'Niell's CSocket file transfer tutorial). Thanks.
> >
> > Vinoj
>
> The code you posted does not execute an "asynchronous" file transfer.
> Rather, the code is ocmpletely synchronous, since it executes, seriatim,
> reception of file name, followed by size, followed by data; and it does not
> complete until all tasks are also complete.
>
> An asynchronous file transfer would perform a single call to Receive() in
> response to an FD_READ notification (i.e., inside your
> CAsyncSocket::OnReceive() override), and then exit after updating your state
> machine, to await again a new FD_READ notification. The serial tasks shown
> in your function (below) would be "cobbled together" after multiple calls to
> OnReceive(), based on logic in your state machine.
>
> The various calls to Sleep() were probably needed to make the code "work",
> in the sense that a file was successfully received without a
> WSAGetLastError() of WSAEWOULDBLOCK. This alone should be enough to
> demonstrate to you that the code is not asynchronous, but rather very
> synchronous and sequential.
>
> As for "garbage being sent after data", if you are using
> http://www.codeproject.com/internet/SocketFileTransfer.asp , then nothing at
> all is being sent after the data, since the sending code specifically sends
> less than a full buffer at the end of a file (it only sends the final part
> of the file, which usually is less than a full buffer). On the receiving
> side, at the end of the file, the code is designed to write to the output
> file only the number of bytes remaining in the file, and not the entire
> buffer (i.e., the code writes to the file based on iiRecd (which is the
> number of bytes received) and not based on RECV_BUFFER_SIZE (which is the
> size of the buffer)).
>
> So, the only source of this behavior is if you are sending multiple files,
> and you do not correctly handle the point of demarcation between sending of
> the current file and that of a prior file.
>
> See specific notes on your code, below.
>
> Mike
>
>
> >
> > BOOL AsyncReceiveFile(CString FilePath){
> >
> > BOOL bRet = TRUE; // return value
> > int dataLength, cbBytesRet, cbLeftToReceive, loop = 0;
> > BYTE* recdData = NULL;
> >
> > CFile destFile;
> > char buff[256];
> > BOOL bFileIsOpen = FALSE;
> > Sleep(200);
> > CString strFileName;
> >
> > //receive the filename
> > memset(buff, 0, sizeof(buff));
> > cbLeftToReceive = MAX_FILENAME_LENGTH;
> > do{
> > cbBytesRet = pAsyncClient->Receive(buff, cbLeftToReceive, 0);
>
>
> This code is wrong, since it always loads the most-recently-received part of
> the file name to the beginning of the buffer. It should be something like
> this:
>
> cbBytesRet = pAsyncClient->Receive(buff+MAX_FILENAME_LENGTH-cbLeftToReceive,
> cbLeftToReceive, 0);
>
>
>
> > if(cbBytesRet > 0)
> > cbLeftToReceive -= cbBytesRet;
> > else if(++loop > 1000)
> > return FALSE;
> > }
> > while(cbLeftToReceive >0);
>
>
> This test makes sense only if the file name is always exactly
> MAX_FILENAME_LENGTH in length, and only if both sides agree on the value of
> MAX_FILENAME_LENGTH. As a better alternative, why not prefix the file name
> with the byte-count of the nae itself, much like you do when sending the
> file's data.
>
>
>
> >
> > strFileName = buff;
> > // open/create target file that receives the transferred data
> >
> > if(!(bFileIsOpen = destFile.Open((LPCTSTR)strFileName,
> > CFile::modeCreate|CFile::modeWrite|CFile::typeBinary))){
> > MessageBeep(MB_ICONEXCLAMATION);
> >
> > // Open Error Log File and Record Problem
> > }
> > return FALSE;
> > }
> >
> > // get the file's size
> > Sleep(200);
> > cbLeftToReceive = sizeof(dataLength);
> > loop = 0;
> >
> > do{
> > BYTE* bp = (BYTE*)(&dataLength) + sizeof(dataLength) -
> > cbLeftToReceive;
> > cbBytesRet = pAsyncClient->Receive( bp, cbLeftToReceive );
> >
> > // test for errors and get out if they occurred
> > if(cbBytesRet == SOCKET_ERROR || cbBytesRet == 0){
> > int iErr = ::GetLastError();
> > if(iErr != WSAEWOULDBLOCK){
> >
> > MessageBeep(MB_ICONEXCLAMATION);
> >
> > // Open Error Log File and Record Problem }
> >
> > return FALSE;
> > }
> > cbLeftToReceive += cbBytesRet;
> >
> > //if data cannot be read
> > if(++loop > 1000)
> > return FALSE;
> > }
> >
> > cbLeftToReceive -= cbBytesRet;
> >
> >
> >
> > }
> > while (cbLeftToReceive > 0);
> >
> > if((dataLength = ntohl( dataLength )) < 0){
> > MessageBeep(MB_ICONEXCLAMATION);
> >
> > // Open Error Log File and Record Problem
> > }
> > return FALSE;
> > }
> >
> >
> > // now get the file in RECV_BUFFER_SIZE chunks at a time
> > Sleep(200);
> > recdData = new byte[BUFFER_SIZE];
> > cbLeftToReceive = dataLength;
> > loop = 0;
> > do{
> > int iiGet, iiRecd;
> >
> > iiGet = (cbLeftToReceive<BUFFER_SIZE)?cbLeftToReceive:BUFFER_SIZE
> ;
> > iiRecd = pAsyncClient->Receive( recdData, iiGet );
> >
> > // test for errors and get out if they occurred
> > if (iiRecd == SOCKET_ERROR || iiRecd == 0){
> > int iErr = ::GetLastError();
> > if(iErr != WSAEWOULDBLOCK){
> > //Record error
> >
> > delete [] recdData;
> > return FALSE;
> > }
> > cbLeftToReceive += iiRecd;
> >
> > if(++loop > 1000)
> > return FALSE;
> > }
> >
> > // good data was retrieved, so accumulate
> > // it with already-received data
> > if(iiRecd > 0)
> > destFile.Write( recdData, iiRecd); // Write it
> > cbLeftToReceive -= iiRecd;
> >
> >
> > }
> > while ( cbLeftToReceive > 0 );
> >
> >
> > delete[] recdData;
> >
> > if ( bFileIsOpen )
> > destFile.Close();
> >
> > return bRet;
> > }
> >
> > "Joseph M. Newcomer" wrote:
> >
> > > Are you using TCP/IP or UDP?
> > >
> > > Are you making sure that, if you are handling strings, that you insert a
> terminating NUL
> > > character after the characters received? For example, if you have a 1K
> buffer, receive 4
> > > bytes representing 4 8-bit characters, then you must put a NUL in [4] a
> NUL character, or
> > > whatever is seen there will be seen as part of the string.
> > > joe
> > >
> > > On Tue, 10 Jan 2006 16:03:02 -0800, "Vinoj"
> <Vinoj(a)discussions.microsoft.com> wrote:
> > >
> > > >Hello,
> > > >
> > > >I have two programs that use async sockets. Occasionally, there is
> garbage
> > > >(extra bytes) being sent after data, thus confusing my finite state
> machine.
> > > >Is there a way to track everytime the send() is called? Any ideas on
> why
> > > >this could be happening?
> > > >
> > > >I don't know if it matters, but the sending side of the program is
> written
> > > >in C and the receiving side is MFC. Thanks.
> > > >
> > > >Vinoj
> > > Joseph M. Newcomer [MVP]
> > > email: newcomer(a)flounder.com
> > > Web: http://www.flounder.com
> > > MVP Tips: http://www.flounder.com/mvp_tips.htm
> > >
>
>
>
From: Vinoj on
Thanks. Back to the drawing board.

"Scott McPhillips [MVP]" wrote:

> Vinoj wrote:
> > I'm using TCP/IP. I'm also sending/receivingbinary data (file transfer).
> >
> > The sending side sends an 'f' char immediately before a file is sent. When
> > the 'f' is received, the receive file routine is called (which receives the
> > file name - 256 chars, the file size, and then the file data). After that,
> > occasionally there will be no more data (expected), however, sometimes there
> > more data on the socket. The receive file routine follows (adapted from Mike
> > O'Niell's CSocket file transfer tutorial). Thanks.
> > <mucho code>
>
> Your first call to Receive does not check for WSAEWOULDBLOCK. Your
> second and third calls to Receive do check for it but they exit if it
> occurs. This is wrong. WSAEWOULDBLOCK can happen in the normal course
> of receiving (and sending) socket data. What do you expect to happen if
> you call Receive before the desired data has been received?
>
> Micheal's comments are also valid. I don't know why your result appears
> to be extra data, and I did not analyze your code in detail, but the
> design of your routine is not compatible with the design of
> CAsyncSocket. (Your routine is compatible with the design of CSocket.)
> There is a knowledgebase article that says there should be only one
> call to CAsyncSocket::Receive() for each call to your OnReceive().
>
> Your receive code is attempting to "pull" data from the socket, but
> async sockets don't work that way. Instead, OnReceive notifies you when
> some data is available, and you should Receive that data and then return
> until the next call to OnReceive comes in. IOW, let the socket "push"
> data to your code.
>
> There is also a possibility that your extra data symptom is simply a
> misunderstanding of debugger displays. If you display the data as if it
> were a string you can see extra data because the buffer is not nul
> terminated. I.e. it is not really a string.
>
> --
> Scott McPhillips [VC++ MVP]
>
>