On Sun, 23 Mar 2014, Amos Jeffries wrote:

If there are no objections I will commit this shortly.

I could not review the adjusted patch in time for your commit due to travel. I tried to warn you about this on IRC, but it probably did not work. I do not really understand the urgency with getting this change committed, especially since significant changes were required to the initial submission. For the future, perhaps we can adjust the commit-after-objections rules to avoid such situations.

Alex.


On 16/03/2014 1:08 a.m., Amos Jeffries wrote:
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


Reply via email to