🚀 go-pugleaf

"Peter" <piotrtrzeciak@yahoo.com> wrote in message 
news:479f7070$1@newsgroups.borland.com...

> Here is the code:

Other than error checking that is missing, I see nothing else in that code 
that would cause the kind of problems you are describing.

> ImageData *imageData;

What does the declaration of ImageData actually look like?

> imageData = (ImageData *)malloc(sizeof(ImageData));

malloc() can return NULL, which you are not checking for.  In fact, since 
you always free() the ImageData instance before exiting, there is no point 
in dynamically allocating it at all.  You could declare it on the stack 
instead and let the compiler manage it for you.

> imageData->INITC = RawToBytes("\x00\xA1", 2);
> imageData->DISPLAY = RawToBytes("\x00\xA9", 2);
> imageData->EVN_MESSAGE = RawToBytes("\x00\xA7", 2);

Why are you treating the command IDs as 2-byte buffers instead of 2-byte 
integers?  That would make your processing much cleaner.  You could get rid 
of the INITC, DISPLAY, and EVN_MESSAGE members altogether and just use 
compile-time constants instead.

> imageData->requested_length = imageData->length - 2;

You are not checking to make sure that requested_length is greater than 0 
before reading and processing the buffer.

> // This return is here only for testing to exit without reading more data.

That would be a memory leak since you don't free the ImageData instance 
before returning.

> if ( (imageData->rcv_buffer[0] == imageData->INITC[0]) && 
> (imageData->rcv_buffer[1] == imageData->INITC[1]) && (imageData->length > 
> 4) ) {
> // INITC as OINT, no reads, just write a response to the socket
> processOint(AContext, imageData);
> }
> else {
> if ( (imageData->rcv_buffer[0] == imageData->INITC[0]) && 
> (imageData->rcv_buffer[1] == imageData->INITC[1]) && (imageData->length 
> <=4) ) {
> // INITC as INIT, no reads, just write a response to the socket
> processInit(AContext, imageData);
> }

Those if statements are a bit redundant.  You could clean that up as follows 
(more so if you get rid of the INITC buffer and use integers instead):

    if( (imageData->rcv_buffer[0] == imageData->INITC[0]) && 
(imageData->rcv_buffer[1] == imageData->INITC[1]) )
    {
        if( imageData->length > 4 )
        {
            // INITC as OINT, no reads, just write a response to the socket
            processOint(AContext, imageData);
        }
        else
        {
            // INITC as INIT, no reads, just write a response to the socket
            processInit(AContext, imageData);
        }
    }

> Now, I put in a return statement in the middle, just to see if I can get
> more than one connection, but it still fails.

You need to be more specific about what "it fails" actually means.

> For any "command" other than DISPLAY, it works fine, because they
> only come as single connections. If I get a series of DISPLAY commands,
> the first one usually goes ok (depends on the timing), but the second 
> client
> connection kills the service.

Then most likely your processing and responding code, which you have no 
shown yet, is not managing the data/connection properly and corrupting the 
service in some way.

> The only other thing I can think of, is that I'm adding the TIdTCPServer
> component to a wrong place (TService), and that's why it gets "clobbered".

Putting a TIdTCPServer inside the TService works perfectly fine when used 
properly.


Gambit