Re: [PATCH] Convert the ConnStateData input buffer to SBuf

On 9/03/2014 7:34 p.m., Alex Rousskov wrote:
> On 03/08/2014 04:13 AM, Amos Jeffries wrote:
>> Prepare the way to efficiently parse client requests using SBuf based
>> parser-ng.
>>
<snip>
>
> Please redesign to avoid these problems. Please let me know if you need
> help with that -- I have a couple of ideas how this can be done, but I
> suspect yours are going to be better since you have spent a lot more
> time with this code.
>

Good points.

Attached patch extends the earlier one so IoCallback stores a
raw-pointer to the ConnStateData::In::buf.

This is now specifically to the SBuf member object rather than its
MemBlob or char* backing stores. So only the short (blocking)
FD_READ_METHOD() call needs to provide any synchronous guarantees.
We particularly need a raw-pointer to the ConnStateData member to
prevent the same possible read/consume collisions causing problems when
it comes to merging the two separate SBuf later (by keeping only one SBuf).

>
>> - comm_read(clientConnection, in.addressToReadInto(), getAvailableBufferLength(), reader);
>> + comm_read(clientConnection, in.buf.rawSpace(in.buf.spaceSize()), in.buf.spaceSize()-1, reader);
>
> If this code survives, please allocate the buffer outside the comm_read
> call. There was already an ugly dependency on the right parameter
> evaluation order, and your changes make it look even worse.
>

It is moved inside the deep read operations now, just before
FD_READ_METHOD is used. If I am understanding what you meant then that
should fix it properly.

Amos