On 25/03/2014 4:39 a.m., Alex Rousskov wrote: > 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.
Ouch. Sorry. I checked knotter but did not see anything, it seems to have been down for a bit though. No particular urgency. Just that I thought it was done, checked out okay and was hoping to move on updating the parser work on top of the new buffer. I will make a push to get the compile issues gone in the project branch. If you want it reverted from trunk until review I can do that meanwhile. Amos > >> 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 >>> >>