🚀 go-pugleaf

Alfred wrote:
> I am making a client/server application with
> TClientSocket/TServerSocket and i have a little problem.
> - when the server read I can have only a part of the full package
> (for lag, fragmentation... ) so my idea is to make my package I send
> from client to server like it : "1234xxxxxxxxx" so 1234 is the full
> lenght of the data the server need to riceve and xxxxx are the data.
> Here is the problem , for example :
> - server get only half package, if i read the "1234"
> (Socket->ReceiveBuf(buf,4);)  and i check Socket->ReceiveLength() I
> can see the package is not complete, but If I wait till next
> ClientSocketRead for a full package I cannot read again the length
> for another check before process it. This can not be a problem with a
> single client becouse i can save the lenght into a string somewhere
> but it is a problem with a high traffic server i think. I dont like
> indy, i like to make all by myself..
> Any idea ?

How about this:

In the header file, add this:
struct TDataStruct
{
    int Length;
    TMemoryStream *DataStream;
};

In your OnClientConnect write this:
{
    TDataStruct *Data = new TDataStruct;
    Data->Length = -1;
    Data->DataStream = new TMemoryStream;
    Socket->Data = Data;
}

Then in your OnClientRead
{
    int DataLength;
    TDataStruct *Data = static_cast<TDataStruct>( Socket->Data );

    if( Data->Length == -1 )
        Socket->ReceiveBuf( &(Data->Length), sizeof( int ) );

    DataLength = min( Socket->ReceiveLength(), Data->Length );
    if( DataLength > 0 )
    {
        char *Buffer = new char[ DataLength ];

        Socket->ReceiveBuf( Buffer, DataLength );
        Data->DataStream->Write( Buffer, DataLength );

        delete[] Buffer;
    }

    Data->Length -= DataLength;
    if( Data->Length == 0 )
    {
        // process your command here.
        // it is stored in Data->DataStream.
        // the stream is cleared once you've finished.

        Data->Length = -1;
        DataStream->Clear();
    }
}

In your OnClientDisconnect
{
    TDataStruct *Data = static_cast<TDataStruct>( Socket->Data );
    delete Data;

    Socket->Data = NULL;
}

Jonathan

Thank you for you answare.
I am trying to use it.. but i have a problem :
TDataStruct *Data = static_cast<TDataStruct>( Socket->Data ); make error (cannot convert 'void *' to 'TDataStruct'...

I dont understand very well how it work, can you help me ?
What happen to server when :
- I get half package, it store it in Data->DataStream, i get a second package it owerwrite datastream ? ( Data->DataStream->Write( Buffer, DataLength ); ) or it is written and the end of the olf Datastream ?

- I get half package, it store it in Data->Datastream, now i get a new half package by another client (another socket) . It is stored in a new datastream or it make confusione the the old one ?

thank you very much
Alfred

Jonathan Benedicto wrote:

 >     DataLength = min( Socket->ReceiveLength(), Data->Length );

If receivelength() > then Data->Length then your code will not read all
available bytes. Your code has to do that though otherwise your program
will hang.

>         char *Buffer = new char[ DataLength ];
> 
>         Socket->ReceiveBuf( Buffer, DataLength );
>         Data->DataStream->Write( Buffer, DataLength );
> 
>         delete[] Buffer;

You don't need to declare a temporary buffer. Just receive directly
in the TMemoryStream and care for a sufficient Size:

   int receivelength = Socket->ReceiveLength();

   if ( receivelength <= 0 )
       return;

   if ( MemoryStream->Position + receivelength >= MemoryStream->Size )
      MemoryStream->Size += receivelength + 1; // +1 is not needed I think

   int nread = Socket->ReceiveBuf (
	((char*)MemoryStream->Memory) + MemoryStream->Position, receivelength );


>     if( Data->Length == 0 )
>     {
>         // process your command here.
>         // it is stored in Data->DataStream.
>         // the stream is cleared once you've finished.

If you check for Data->Length == 0 then you are not allowing the sender
to send more than one message at a time. If the sender sends more
messages at once than the second message can have partly come in with
the first message. Better check if a complete message came in.
You then cannot automatically clear the stream also because of a next message
already partly present.

Hans.

Alfred wrote:
> I am trying to use it.. but i have a problem :

Sorry, that code was untested. Try this:

TDataStruct *Data = reinterpret_cast<TDataStruct>( Socket->Data );

> I dont understand very well how it work, can you help me ?
> What happen to server when :
> - I get half package, it store it in Data->DataStream, i get a second
> package it owerwrite datastream ? ( Data->DataStream->Write( Buffer,
> DataLength ); ) or it is written and the end of the olf Datastream ?

If you get a second packet, then it is written at the end of the stream.

> - I get half package, it store it in Data->Datastream, now i get a
> new half package by another client (another socket) . It is stored in
> a new datastream or it make confusione the the old one ?

By using the Socket->Data to store the TDataStruct's pointer, I make sure 
that every socket, or connection, has its own stream.

Jonathan

Hans Galema wrote:
> If receivelength() > then Data->Length then your code will not read
> all available bytes. Your code has to do that though otherwise your
> program will hang.

Why will it do that ?

> You don't need to declare a temporary buffer. Just receive directly
> in the TMemoryStream and care for a sufficient Size:

Thank you for that improvement.

> If you check for Data->Length == 0 then you are not allowing the
> sender to send more than one message at a time. If the sender sends more
> messages at once than the second message can have partly come in with
> the first message. Better check if a complete message came in.
> You then cannot automatically clear the stream also because of a next
> message already partly present.

Thank you, I hadn't thought of that. I'll fix it.

Jonathan

Alfred wrote:

> I am making a client/server application with TClientSocket/TServerSocket and i have a little problem.
> - when the server read I can have only a part of the full package (for lag, fragmentation... )

Please wrap your lines manually. Now it is difficult to reply selective.

Not only for the server. Also for a client. In this respect it's better to speak
of sender and receiver.

>  so my idea is to make my package I send from client to server like it :
 >  "1234xxxxxxxxx" so 1234 is the full lenght of the data the server need to
 > riceve and xxxxx are the data.

That looks like the length in ascii. Better declare an int size.

   int size = the_size_of_the_message;

   ->SendBuf ( &size, sizeof(size);
   ->SendBuf ( message, size );

check the return values.

> Here is the problem , for example :
> - server get only half package, if i read the "1234" (Socket->ReceiveBuf(buf,4);) 
 > and i check Socket->ReceiveLength() I can see the package is not complete,
 > but If I wait till next ClientSocketRead for a full package I cannot read
 > gain the length for another check before process it. This can not be a problem
 > with a single client becouse i can save the lenght into a string somewhere but
 > it is a problem with a high traffic server i think.

As Johan showed you:

   int size = 0;
   Socket->ReceiveBuf(&size,4);

Using a TMemoryStream is fine.

What you have to consider first is that if OnRead triggers that
you /have/ to read all bytes indicated by ReceiveLength().

That can be very easily handled by just reading /all/ into a TMemoryStream.
(Adjust Size first, And Position after).

Now after having put all in the memorystream check the first four bytes.
They indicate the size of the first message that is in it.

   int size = *(int*)MemoryStream->Memory;

You should then make a loop in which you check if there is a complete message
in the stream. If so handle it. If there are more (partial) in the stream
then the first message then move the following messages to the start
of the stream.

Hans.

Hans Galema wrote:
> If you check for Data->Length == 0 then you are not allowing the
> sender to send more than one message at a time. If the sender sends more
> messages at once than the second message can have partly come in with
> the first message. Better check if a complete message came in.
> You then cannot automatically clear the stream also because of a next
> message already partly present.

Please could you explain why ? I looked at my code again, and it never 
reads more than the message said its size was.

Jonathan

Jonathan Benedicto wrote:

> Please could you explain why ? I looked at my code again, and it never 
> reads more than the message said its size was.

Indeed. That is correct. But your code should read all bytes indicated by
ReceiveLength(). Not the minimum with the messagelength. So then first it
would go wrong.

If your code runs fine then you probably have senders that do not sent
more then one message at once. For instance a client that waits to
send a next command until the server responded.

Hans.

Hans Galema wrote:

> As Johan showed you:

Ai ai .. Sorry Jonathan!

Hans.

Hans Galema wrote:
> Ai ai .. Sorry Jonathan!

No problem <g> My name often gets changed. It is long.

Jonathan

Hans Galema wrote:
> Indeed. That is correct. But your code should read all bytes
> indicated by ReceiveLength(). Not the minimum with the messagelength.
> So then first it would go wrong.

I just tried this code, and the app didn't hang:

void __fastcall TForm1::Button1Click(TObject *Sender)
{
    AnsiString Msg = "This is a very long message.";

    for( int i = 0; i < 20; ++i )
    {
        ClientSocket1->Socket->SendBuf( Msg.c_str(), Msg.Length() );
        Label1->Caption = i;
        Label1->Repaint();
    }
}

void __fastcall TForm1::ServerSocket1ClientRead(TObject *Sender,
      TCustomWinSocket *Socket)
{
    int rl = Socket->ReceiveLength();

    rl /= 2;

    if( rl <= 0 )
       return;

    char *buffer = new char[ rl ];

    Socket->ReceiveBuf( buffer, rl );
    Memo1->Lines->Add( "recvd buf of " + IntToStr( rl ) + " bytes." );

    delete[] buffer;
}

Jonathan

Thank you for reply
>
>   int size = the_size_of_the_message;
>
>   ->SendBuf ( &size, sizeof(size);
>   ->SendBuf ( message, size );

Yes that is a better idea than mine (thank you Jonathan and you).

>Using a TMemoryStream is fine.
>What you have to consider first is that if OnRead triggers that
>you /have/ to read all bytes indicated by ReceiveLength().
>That can be very easily handled by just reading /all/ into a TMemoryStream.
>(Adjust Size first, And Position after).
>Now after having put all in the memorystream check the first four bytes.

ok i am trying to write it :
on clientsocketread :
        int Len = ClientSocket->Socket->ReceiveLength();

        char *Buffer = new char[Len];
        Socket->ReceiveBuf( Buffer, Len );

        TMemoryStream *Data = new TMemoryStream;
        Data->Size = Len;
        Data->Position = 0;
        Data->Write( Buffer, Len );

now my data are stored in it and like you written I can read the packet size with :
        int size = *(int*)Data->Memory;
here is my new problem , for example : the first time I get only the first part of a package and after few seconds I get the last part of this  package. How I can join both parts ? my code now works but I haven't a single message, i have a lot of splitted message.
thank you very much
Alfred

a litte confused for me, i am pretty new in c++ but It gived me a lot of info.. Thank you very much :)
Alfred

Jonathan Benedicto wrote:

> I just tried this code, and the app didn't hang:

But clicking the button a second time does nothing.

To see what is happening:

void __fastcall TForm1::Button1Click(TObject *Sender)
{
    AnsiString Msg = "This is a very long message.";

    int nsendtotal = 0;

     for( int i = 0; i < 20; ++i )
     {
         int nsend = ClientSocket1->Socket->SendBuf( Msg.c_str(), Msg.Length() );

         nsendtotal += nsend;

         Label1->Caption = IntToStr ( i ) + " - " + IntToStr ( nsendtotal );
         Label1->Repaint();
     }
}


void __fastcall TForm1::ServerSocket1ClientRead(TObject *Sender,
       TCustomWinSocket *Socket)
{
    int rl = Socket->ReceiveLength();

    int r2 = rl/ 2;

     if( r2 <= 0 )
        return;

     char *buffer = new char[ r2 ];

     int nreceived = Socket->ReceiveBuf( buffer, r2 );

     // private int receivedtotal;
     receivedtotal += nreceived;

     Memo1->Lines->Add( IntToStr ( r2 )
         + " recvd buf of " + IntToStr( nreceived )
         + " bytes. total: " + IntToStr ( receivedtotal ));

     delete[] buffer;
}

Hans.

Hans Galema wrote:
> But clicking the button a second time does nothing.

It seemed to work fine for me.

Jonathan

Alfred wrote:
> now my data are stored in it and like you written I can read the
>        packet size with : int size = *(int*)Data->Memory;
> here is my new problem , for example : the first time I get only the
> first part of a package and after few seconds I get the last part of
> this  package. How I can join both parts ? my code now works but I
> haven't a single message, i have a lot of splitted message. thank you
> very much

Like this: (untested)

Data->Position = 0;

while( true )
{
    int MsgSize, SizeRead;

    SizeRead = Data->Read( &MsgSize, sizeof( int ) );
    if( SizeRead < sizeof( int ) )
        break;

    if( MsgSize > 0 )
    {
        AnsiString Message = AnsiString( (char*)( Data->Memory + 
Data->Position ), MsgSize );
        // Process message here.

        Data->Seek( MsgSize, soFromCurrent );
    }
};

Jonathan

"Alfred" <alf@no.spam.not.valid.email> wrote in message
news:43662baf$1@newsgroups.borland.com...

> - server get only half package, if i read the "1234"
(Socket->ReceiveBuf(buf,4);)
> and i check Socket->ReceiveLength()

You should be calling ReceiveLength() before ReceiveBuf(), not after.  And
you should be buffering everything that is received and then process the
buffer only when enough bytes are available.

> I can see the package is not complete, but If I wait till next
> ClientSocketRead for a full package I cannot read again the
> length for another check before process it.

Yes, you can, if you buffer everything.

> This can not be a problem with a single client becouse i can save
> the lenght into a string somewhere but it is a problem with a
> high traffic server i think.

You need to store buffers on a per-connection basis.  You can use the
TCustomWinSocket::Data property for that.

For example:

    void __fastcall TForm1::ServerSocket1ClientConnect(TObject* Sender,
TCustomWinSocket* Socket)
    {
        Socket->Data = new TMemoryStream;
    }

    void __fastcall TForm1::ServerSocket1ClientDisconnect(TObject* Sender,
TCustomWinSocket* Socket)
    {
        delete (TMemoryStream*) Socket->Data;
        Socket->Data = NULL;
    }

    void __fastcall TForm1::ServerSocket1ClientRead(TObject* Sender,
TCustomWinSocket* Socket)
    {
        TMemoryStream *Buffer = (TMemoryStream*) Socket->Data;

        int Len = Socket->ReceiveLength();
        if( Len < 1 ) // error...
            return;

        // expand the Buffer ...
        int Old = Buffer->Size;
        Buffer->Size = ( Old + Len );

        // need to do this after changing the Size above in case a memory
        // allocation was performed that changes the address that the
        // Memory property points to...
        LPBYTE *BufPtr = (LPBYTE) Buffer->Memory;

        // read the new bytes into the expanded Buffer memory...
        int NumRead = Socket->ReceiveBuf(&BufPtr[Old], Len);
        if( NumRead < 1 ) // error...
        {
            // remove the expanded Buffer memory...
            Buffer->Size = Old;
            return;
        }

        // loop through the Buffer processing only complete packets...
        Buffer->Position = 0;
        while( (Buffer->Size - Buffer->Position) >= sizeof(int) )
        {
            // read the packet length...
            Buffer->Read(&Len, sizeof(int));

            // if not enough bytes for both the length and the data
            // then leave the length in the Buffer until later...
            if( (Buffer->Position + Len) >= Buffer->Size )
            {
                Buffer->Seek(-sizeof(int), soFromCurrent);
                break;
            }

            // process a complete packet...
            ProcessPacket(Socket, &BufPtr[Buffer->Position], Len);

            // skip past the complete packet...
            Buffer->Seek(Len, soFromCurrent);
        }

        // if any complete packets were processed then remove them from the
Buffer...
        if( Buffer->Position > 0 )
        {
            int NewSize = ( Buffer->Size - Buffer->Position );
            ::MoveMemory(BufPtr, &BufPtr[Position], NewSize);
            Buffer->Size = NewSize;
        }
    }

    void__fastcall TForm1::ProcessPacket(TCustomWinSocket *Socket, void
*Data, int DataLength)
    {
        // process Data as needed...
    }


> I dont like indy

Why not?


Gambit

"Jonathan Benedicto" <incorrect@no.server> wrote in message
news:4366425e$1@newsgroups.borland.com...

> Sorry, that code was untested. Try this:
>
> TDataStruct *Data = reinterpret_cast<TDataStruct>( Socket->Data );

That is still wrong.  You need to cast to a pointer type:

    TDataStruct *Data = reinterpret_cast<TDataStruct*>( Socket->Data );


Gambit

"Jonathan Benedicto" <incorrect@no.server> wrote in message
news:436642bd$1@newsgroups.borland.com...

> Why will it do that ?

If you do not remove ALL available bytes from the socket at the time of the
event, then the socket will fall out of sync with itself and not receive new
read notifications properly.


Gambit

"Alfred" <alf@no.spam.not.valid.email> wrote in message
news:4366672a$1@newsgroups.borland.com...

> ok i am trying to write it :
> on clientsocketread :
>         int Len = ClientSocket->Socket->ReceiveLength();

You are not taking into account the conditions where ReceiveLength() can
return a value < 1.  You need to do so.  You should also be accessing the
Socket parameter of the OnRead event handler, not the Socket property of the
TClientSocket.

>         TMemoryStream *Data = new TMemoryStream;
>         Data->Size = Len;
>         Data->Position = 0;
>         Data->Write( Buffer, Len );

You shold not be creating a new TMemoryStream on every event.  You need to
create a single TMemoryStream and then reuse it over and over.  That is the
only way you can preserve incomplete data between multiple events.

> the first time I get only the first part of a package and after few
> seconds I get the last part of this  package. How I can join both parts ?

By writing both pieces to the same TMemoryStream object, not to separate
streams.


Gambit

"Jonathan Benedicto" <incorrect@no.server> wrote in message
news:436669aa$1@newsgroups.borland.com...

>     SizeRead = Data->Read( &MsgSize, sizeof( int ) );
>     if( SizeRead < sizeof( int ) )
>         break;

You are removing the length from the stream.  If there are not enough bytes
for the data, you have now lost the length for the next event.  You should
be seeking the stream backwards sizeof(int) bytes so that the same length
can be re-read on the next loop.

>         AnsiString Message = AnsiString( (char*)( Data->Memory +
> Data->Position ), MsgSize );

The Memory property is declared as a void*.  You cannot add the Position to
the Memory as-is.  You have to cast the Memory first, such as to BYTE* or
char*.


Gambit

Remy Lebeau (TeamB) wrote:
> That is still wrong.  You need to cast to a pointer type:

Thank you.

Jonathan

Remy Lebeau (TeamB) wrote:
> If you do not remove ALL available bytes from the socket at the time
> of the event, then the socket will fall out of sync with itself and
> not receive new read notifications properly.

Thank you for this info. I didn't know that. Thankfully I haven't use code 
like that before.

Jonathan

Remy Lebeau (TeamB) wrote:
> You are removing the length from the stream.  If there are not enough
> bytes for the data, you have now lost the length for the next event.
> You should be seeking the stream backwards sizeof(int) bytes so that
> the same length can be re-read on the next loop.

I wasn't writing that code for the OnRead event. But thank you for pointing 
that out.

> The Memory property is declared as a void*.  You cannot add the
> Position to the Memory as-is.  You have to cast the Memory first,
> such as to BYTE* or char*.

Thank you for this info. The code was untested so I didn't see that error 
when compiled.

Jonathan

Alfred wrote:

> here is my new problem , for example : the first time I get only the first part of 
 > a package and after few seconds I get the last part of this  package. How I can
 > oin both parts ? my code now works but I haven't a single message, i have a lot of splitted message.

Please keep wrapping your lines als at the end of a message.

How to concatenate I already told in one of the first replies to Jonathan:

And you do not need an extra buffer. Put it in the stream directly.

   int receivelength = Socket->ReceiveLength();

   if ( receivelength <= 0 )
       return;

   if ( MemoryStream->Position + receivelength >= MemoryStream->Size )
      MemoryStream->Size += receivelength + 1; // +1 is not needed I think

   int nread = Socket->ReceiveBuf (
     ((char*)MemoryStream->Memory) + MemoryStream->Position, receivelength );

   MemoryStream->Position += nread;

Now check the stream for a complete message and so.

Hans.

Jonathan Benedicto wrote:

>>But clicking the button a second time does nothing.
> 
> It seemed to work fine for me.

Strange.

Could you post the contents of Memo1 after one buttonclick and then
after two (using the code with my changes) ?

Hans.

Great code, I learned a lot with it :)

>            if( (Buffer->Position + Len) >= Buffer->Size )
>            {
>                Buffer->Seek(-sizeof(int), soFromCurrent);
>                break;
>            }

Are it correct ? I have tryed it but it execute always the Buffer->Seek routine even if the message is complete, i have changed it in :

if( (Buffer->Position + Len) > Buffer->Size )

it work now. It is correct ?

I have tryed a ProcessPacket routine like :
void__fastcall TForm1::ProcessPacket(TCustomWinSocket *Socket, void *Data, int DataLength)
{
        // process Data as needed...
        char *a = new char[DataLength];
        a = (char*)Data;
        Memo->Lines->Add(AnsiString(a,DataLength));
        delete[] a;
}

It dont work.. why not ? Sender program send an example with :
        char *buf=new char[10];
        buf = "1234567890";
        int size=10;

        Socket->SendBuf ( &size, sizeof(int) );
        Socket->SendBuf ( buf , size );

and the receiver write only not ascii chars. What is wrong ?

>> I dont like indy
>
>Why not?
>

I like to program all by myself so I can learn as much as possible. If I used indy my code was already finished but I never knowed a lot of info I am learning right now.
Thank you very much all for the help
Alfred

>I have tryed a ProcessPacket routine like :
>void__fastcall TForm1::ProcessPacket(TCustomWinSocket *Socket, void *Data, int DataLength)
>{
>        // process Data as needed...
>        char *a = new char[DataLength];
>        a = (char*)Data;
>        Memo->Lines->Add(AnsiString(a,DataLength));
>        delete[] a;
>}
>
>It dont work.. why not ? Sender program send an example with :
>        char *buf=new char[10];
>        buf = "1234567890";
>        int size=10;
>
>        Socket->SendBuf ( &size, sizeof(int) );
>        Socket->SendBuf ( buf , size );
>
>and the receiver write only not ascii chars. What is wrong ?

Fixed it, it was : ProcessPacket(TCustomWinSocket *Socket, LPBYTE *Data, int DataLength) so it is LPBYTE not void* .
A new problem appairs now when i receive half package each time, i write in the Memo the first 5 chars correctly and the last 5 in a not ascii chars..

Here is the code used in the sender :
   char *buf=new char[10];
   buf = "12345";

   Memo->Lines->Add("invio 1");
   int size=10;
   Socket->SendBuf ( &size, sizeof(int));
   Socket->SendBuf(buf,5);

   Sleep(3000); // to be sure it send data in 2 different package
   buf = "67890";
   Memo->Lines->Add("invio 2");
   Socket->SendBuf(buf,5);

Any idea ?
Thank you very much, you all helped my a lot :)
Alfred

yes you right.
I am making a lot of confusion while programming.
I have one last error : when I get 2 half package , for example :
12345 and 67890 i have problems with the second package. the code :
void __fastcall TForm1::ProcessPacket(TCustomWinSocket *Socket, void *Data, int DataLength)
{
        // process Data as needed...
        Memo->Lines->Add(AnsiString((char*)Data, DataLength));
} 
write sometimes the first 5 chars correctly and the last 5 in not ascii code and sometimes all 10 chars in not ascii code.. why that ? I have only copied all you code with no modifications..

Thank you very much for your help
Alfred

"Hans Galema" <notused@notused.nl> wrote in message
news:43668460@newsgroups.borland.com...

>    if ( MemoryStream->Position + receivelength >= MemoryStream->Size )
>       MemoryStream->Size += receivelength + 1; // +1 is not needed I think

No, +1 is not needed.  Neither is checking the (Position + receivelength),
either.  TMemoryStream's Size property sets the Capacity property, which
already checks the new size before actually allocating memory.
TMemoryStream allocates its Memory in 8K chunks.


Gambit

"Alfred" <alf@no.spam.not.valid.email> wrote in message
news:4366ccd3$1@newsgroups.borland.com...

> I have tryed a ProcessPacket routine like :

That code is wrong for three reasons:

>        char *a = new char[DataLength];

1) You are allocating an extra buffer that is not actually being used for
anything.

>        a = (char*)Data;

2) you are then reassigning the pointer to point to the Data parameter
instead of the buffer you allocated.  You have now lost your pointer to the
buffer.  You can never free it, thus leaking the allocated memory.

>        delete[] a;

3) Since you are now pointing to the Data parameter, you are freeing memory
that is owned by the TMemoryStream.

The routine should be like this instead:

    void__fastcall TForm1::ProcessPacket(TCustomWinSocket *Socket, void
*Data, int DataLength)
    {
        // process Data as needed...
        Memo->Lines->Add(AnsiString((char*)Data, DataLength));
    }

> Sender program send an example with :

That code is wrong as well, for the same reasons as above.  Not only that,
but you are not checking the return value of SendBuf(), either.  You need to
do so, ie:

    bool __fastcall TForm1::SendData(TCustomWinSocket *Socket, void *Data,
int DataSize)
    {
        LPBYTE pBuf = (LPBYTE) Data;

        while( DataSize > 0 )
        {
            int Sent = Socket->SendBuf(pBuf, DataSize);
            if( Sent < 1 )
            {
                if( Sent == 0 )
                    return false; // disconnected

                if( WSAGetLastError() != WSAEWOULDBLOCK )
                    return false; // error

                Sleep(50); // keep waiting
            }
            else
            {
                pBuf += Sent;
                DataSize -= Sent;
            }
        }

        return true;
    }


    char *buf = "1234567890";
    int len = 10;

    if( SendData(Socket, &len, sizeof(int)) )
        SendData(Socket, buf, len);


Gambit

Hans Galema wrote:
> Could you post the contents of Memo1 after one buttonclick and then
> after two (using the code with my changes) ?

I had only changed one thing, this:
if( r2 <= 0 )
    return;

to:
if( r1 < 2 )
    r2 = r1;


After one button click:

280 recvd buf of 280 bytes. total: 280
140 recvd buf of 140 bytes. total: 420
70 recvd buf of 70 bytes. total: 490
35 recvd buf of 35 bytes. total: 525
17 recvd buf of 17 bytes. total: 542
9 recvd buf of 9 bytes. total: 551
4 recvd buf of 4 bytes. total: 555
2 recvd buf of 2 bytes. total: 557
1 recvd buf of 1 bytes. total: 558
1 recvd buf of 1 bytes. total: 559
1 recvd buf of 1 bytes. total: 560

After two:

280 recvd buf of 280 bytes. total: 280
140 recvd buf of 140 bytes. total: 420
70 recvd buf of 70 bytes. total: 490
35 recvd buf of 35 bytes. total: 525
17 recvd buf of 17 bytes. total: 542
9 recvd buf of 9 bytes. total: 551
4 recvd buf of 4 bytes. total: 555
2 recvd buf of 2 bytes. total: 557
1 recvd buf of 1 bytes. total: 558
1 recvd buf of 1 bytes. total: 559
1 recvd buf of 1 bytes. total: 560
280 recvd buf of 280 bytes. total: 840
140 recvd buf of 140 bytes. total: 980
70 recvd buf of 70 bytes. total: 1050
35 recvd buf of 35 bytes. total: 1085
17 recvd buf of 17 bytes. total: 1102
9 recvd buf of 9 bytes. total: 1111
4 recvd buf of 4 bytes. total: 1115
2 recvd buf of 2 bytes. total: 1117
1 recvd buf of 1 bytes. total: 1118
1 recvd buf of 1 bytes. total: 1119
1 recvd buf of 1 bytes. total: 1120

Jonathan

"Alfred" <alf@no.spam.not.valid.email> wrote in message 
news:4366deb1$1@newsgroups.borland.com...

> I have only copied all you code with no modifications..

Change these lines:

    LPBYTE *BufPtr = (LPBYTE) Buffer->Memory;

    if( (Buffer->Position + Len) >= Buffer->Size )

    ::MoveMemory(BufPtr, &BufPtr[Position], NewSize);

To these instead, respectively:

    LPBYTE BufPtr = (LPBYTE) Buffer->Memory;

    if( (Buffer->Size - Buffer->Position) < Len )

    ::MoveMemory(BufPtr, &BufPtr[Buffer->Position], NewSize);


Gambit 

Remy Lebeau (TeamB) wrote:

>>   if ( MemoryStream->Position + receivelength >= MemoryStream->Size )
>>      MemoryStream->Size += receivelength + 1; // +1 is not needed I think
> 
> No, +1 is not needed.  Neither is checking the (Position + receivelength),
> either.  TMemoryStream's Size property sets the Capacity property, which
> already checks the new size before actually allocating memory.
> TMemoryStream allocates its Memory in 8K chunks.

The adjusting of Size is needed. You have to see it in the context
of the statement that folows:

   int nread = Socket->ReceiveBuf (
     ((char*)MemoryStream->Memory) + MemoryStream->Position, receivelength );

If ->Memory had not sufficiend Size then above statement, that puts bytes
directly in the Memory-buffer, would produce a bufferoverflow.

If I had used TMemoryString::Write() then Size had been adjusted automatically.

See following code that will cause the app to exit because of a bufferoverflow.

To my surprise codeguard does nothing here.

void __fastcall TForm1::Button1Click(TObject *Sender)
{
   TMemoryStream *MemoryStream = new TMemoryStream;

   MemoryStream->Size = 1000;

   char *ptr = (char*)MemoryStream->Memory;

   int size = 20000;

   while ( --size )
      *ptr++ = 0;

   delete MemoryStream;
}

Hans.

Jonathan Benedicto wrote:

> I had only changed one thing, this:
> if( r2 <= 0 )
>     return;

> to:
> if( r1 < 2 )
>     r2 = r1;

That makes the whole difference. Now all bytes are eventually read
of the first delivery.

The next buttonclick will go ok.

So we spoke about different code and did a different test.

Confusing.

Hans.

Remy Lebeau (TeamB) wrote:

>     char *buf = "1234567890";
>     int len = 10;

To handle the len for all situations where strings are to
be transported I would write

      char *buf = "1234567890";
      int len = strlen ( buf );

but rather yet

   len = strlen ( buf ) +1;

The +1 cares for sending of the terminating zero.
This comes in quite handy on the receiving side where the string
is then zero teminated also.

Hans.

"Hans Galema" <notused@notused.nl> wrote in message
news:43672a80$1@newsgroups.borland.com...

> The adjusting of Size is needed.

Adjusting the Size in general is needed, of course.  That is not what I was
arguing against.  The way that you are doing it is not needed.  You don't
need to check whether the (Position + receivelength) will exceed the current
Size.  Partly because that condition will always be true.  Partly because
the Size already does the validation internally so there is no need to
manually duplicate what TMemoryStream is already doing for you
automatically.  And Partly because your use of the '+=' operator is wrong
anyway.  You cannot use the '+=' operator with properties, it doesn't work
correctly.

These lines are unnecessary:

    if ( MemoryStream->Position + receivelength >= MemoryStream->Size )
        MemoryStream->Size += receivelength + 1; // +1 is not needed I think

All that is needed is the following:

    MemoryStream->Size = MemoryStream->Size + receivelength;

If you really want to continue checking the Position beforehand, then you
need to fix your assignment of the Size property, because you are setting it
to the wrong value.  It should be more like like this instead:

    MemoryStream->Size = (MemoryStream->Position + receivelength);

> If ->Memory had not sufficiend Size then above statement, that
> puts bytes directly in the Memory-buffer, would produce a bufferoverflow.

That is not the point I was arguing.  Of course the Size has to be
sufficient before reading into it.  But you are performing unnecessary
operations in regards to it.


Gambit

"Hans Galema" <notused@notused.nl> wrote in message
news:43672d94$1@newsgroups.borland.com...

> The +1 cares for sending of the terminating zero.

Sending the null terminator is unnecessary in the code bing discussed.
Sending the length is sufficient.

> This comes in quite handy on the receiving side where
> the string is then zero teminated also.

The string is already null terminated automatically when the AnsiString is
allocated.


Gambit

Remy Lebeau (TeamB) wrote:

> ... You don't
> need to check whether the (Position + receivelength) will exceed the current
> Size. 

Well I have to. But you did not know why because I did not tell the
whole story. I never size the stream down. It's Size will only grow
(when needed). Size just adjusts to the biggest message received.

 > You cannot use the '+=' operator with properties, it doesn't work
> correctly.

I would believe that. And I know the reason why. But then += works
correct here. Anyhow I changed the code.

Hans.

Remy Lebeau (TeamB) wrote:

> The string is already null terminated automatically when the AnsiString is
> allocated.

Where is the AnsiString ?

HAns.

>To these instead, respectively:
>
>    LPBYTE BufPtr = (LPBYTE) Buffer->Memory;
>
>    if( (Buffer->Size - Buffer->Position) < Len )
>
>    ::MoveMemory(BufPtr, &BufPtr[Buffer->Position], NewSize);

It is perfect right now. Thank you very much Gambit, Hans and Jonathan for your help, I have learned a lot of think with your post in those days.

one more think :
  ProcessPacket(Socket, &BufPtr[Buffer->Position], Len);
it give me a warning : W8072 Suspicious pointer arithmetic
How can I fix it to remove those warning ?

Thank you again
Alfred

Remy Lebeau (TeamB) wrote:

> If you don't do that, then the stream will slowly continue allocating more
> and more memory until it runs out.  You should reduce the Size periodically
> to reduce the amount of unused memory.

That is hard to believe. What would be the reason then ? If a client first
sends it biggest message and then millions of small messages then tho code does
not force Size to increase and hence Capacity stays the same too. So there are no
allocations going on. What would run out then ?

Hans.

Remy Lebeau (TeamB) wrote:

>>Where is the AnsiString ?
> 
> In Alfred example code:
> 
>     Memo->Lines->Add(AnsiString((char*)Data, DataLength));

That is of lot of hassle compared by sending the terminating
zero of a char buf. Moreover if the sender sends a strings in
a char buf then the receiver likely handles them as such and will
be glad seeing a terminating zero.

As you can see in my first post concerning this zero I never
said that it was needed but that it came in handy.

Hans.

"Alfred" <alf@no.spam.not.valid.email> wrote in message 
news:4367f9ea$1@newsgroups.borland.com...

>  ProcessPacket(Socket, &BufPtr[Buffer->Position], Len);
> it give me a warning : W8072 Suspicious pointer arithmetic
> How can I fix it to remove those warning ?

If you are using BCB6, then the TStream::Position property has been changed 
to an __int64 instead of an int.  Indexing a pointer using an __int64 value 
will produce that warning (I just ran across that same warning myself this 
week while trying to implement 64-bit streams in my own code).  At runtime, 
it should be ok, though, since TMemoryStream does not actually implement 
64-bit values greater than MaxInt (0x7FFFFFFF).


Gambit 

"Hans Galema" <notused@notused.nl> wrote in message 
news:4368805e$1@newsgroups.borland.com...

> That is of lot of hassle compared by sending the terminating
> zero of a char buf.

Actually, there is a little more overhead involved when constructing an 
AnsiString from a null-terminated string without a length.  AnsiString would 
have to calculate the length of the characters before it can then allocate 
the memory to copy the characters into.  By sending and using the string 
length with the characters, that extra calculation is not needed since the 
length is already known ahead of time.

Besides, the code I gave Alfred is not designed to handle just strings 
alone, but any dynamic-length data in general.  Converting the data to 
AnsiString for display purposes was just his way of testing the code.  The 
code itself doesn't care if the data belongs to an AnsiString or not.

> Moreover if the sender sends a strings in a char buf then the receiver
> likely handles them as such and will be glad seeing a terminating zero.

Not necessarily.  If the original characters were in an appropriately-sized 
buffer, then chances of there being a terminating null character are 
actually less, since the buffer can be processed without a null terminator 
being present.  And if the size of the buffer is known on the receiver's 
side, then the null terminator is still not needed when receiving the 
characters.  Sure, the receiver can allocate extra memory to put a null 
terminator at the end of the buffer, but that is no needed.

> As you can see in my first post concerning this zero I never
> said that it was needed but that it came in handy.

That depennds on how the characters are actually being used.  The sender 
sound not be making assumptions about how the receiver will use the 
characters, and vice versa.


Gambit 

"Hans Galema" <notused@notused.nl> wrote in message 
news:43687e84$1@newsgroups.borland.com...

> That is hard to believe. What would be the reason then ? If a client first
> sends it biggest message and then millions of small messages then tho
> code does not force Size to increase and hence Capacity stays the same
> too. So there are no allocations going on. What would run out then ?

It depends on when and how you are removing the messages from the buffer, 
and whether you are moving the Position, and sizing the buffer based on the 
Position.


Gambit 

>If you are using BCB6, then the TStream::Position property has been changed 
>to an __int64 instead of an int.  Indexing a pointer using an __int64 value 
>will produce that warning (I just ran across that same warning myself this 
>week while trying to implement 64-bit streams in my own code).  At runtime, 
>it should be ok, though, since TMemoryStream does not actually implement 
>64-bit values greater than MaxInt (0x7FFFFFFF).

Thank you very much. I will keep the warning , the finale code is now perfect. I am changing some parts to get access by a proxy too. How can I insert a simple "Int proxy;" in each Socket so each connection have a different "int proxy;" ? Need to use Socket->Data ? how it work ?
Thank you very much to all for the help
Alfred

"Alfred" <alf@no.spam.not.valid.email> wrote in message
news:436a4574$1@newsgroups.borland.com...

> How can I insert a simple "Int proxy;" in each Socket so each connection
> have a different "int proxy;" ? Need to use Socket->Data ?

Yes.

> how it work ?

The Data is just a void* pointer, which a is 32-bit value.  An int is also a
32-bit value.  If all you are storing is an int, then cast it back and forth
directly:

    int i = 12345;
    Socket->Data = (void*) i;
    i = (int) Socket->Data;


Gambit

>The Data is just a void* pointer, which a is 32-bit value.  An int is also a
>32-bit value.  If all you are storing is an int, then cast it back and forth
>directly:
>
>    int i = 12345;
>    Socket->Data = (void*) i;
>    i = (int) Socket->Data;

Thank you very much..
I need to use both TMemoryStream and a simple int. I think I need a struct for it ?

struct DataSocket
{
    int Proxy;
    TMemoryStream *MemoryStream;
};

onconnect :
   DataSocket *Data=new DataSocket;
   Socket->Data = Data;

and to use it ?
   Data->Proxy = 1;
   Data->MemoryStream = ....

It is correct ? ondisconnect only a Socket->Data=NULL or i have to free something ?

thank you very much for all help you all gived me
Alfred

"Alfred" <alf@no.spam.not.valid.email> wrote in message
news:436b445c$1@newsgroups.borland.com...

> I need to use both TMemoryStream and a simple int.
> I think I need a struct for it ?

Yes, you do.

> onconnect :
>    DataSocket *Data=new DataSocket;
>    Socket->Data = Data;

Don't forget to instantiate the TMemoryStream as well:

    DataSocket *Data = new DataSocket;
    Data->Proxy = 0;
    Data->MemoryStream = new TMemoryStream;
    Socket->Data = Data;

And then free everything in the OnDisconnect event:

    DataSocket *Data = (DataSocket*) Socket->Data;
    Socket->Data = NULL;
    delete Data->MemoryStream;
    delete Data;

I would suggest taking it a step further by adding a constructor and
destructor to the struct:

    struct DataSocket
    {
        int Proxy;
        TMemoryStream *MemoryStream;

        DataSocket()
        {
            Proxy = 0;
            MemoryStream = new TMemoryStream;
        }

        ~DataSocket()
        {
            delete MemoryStream;
        }
    };


    OnConnect:
        Socket->Data = new DataSocket;

    OnDisconnect:
        delete (DataSocket*) Socket->Data;
        Socket->Data = NULL;

> and to use it ?
>    Data->Proxy = 1;
>    Data->MemoryStream = ....
>
> It is correct ?

Yes.  In other events, you'll have to cast the Socket's Data property in
order to access the struct:

    DataSocket *Data = (DataSocket *) Socket->Data;
    Data->...

> ondisconnect only a Socket->Data=NULL or i have to free something ?

You have to free everything you allocated in the OnConnect event.


Gambit