<div><span class="gmail_quote">On 6/1/07, <b class="gmail_sendername"><a href="mailto:jmzhou.ml@gmail.com">jmzhou.ml@gmail.com</a></b> <<a href="mailto:jmzhou.ml@gmail.com">jmzhou.ml@gmail.com</a>> wrote:</span>
<blockquote class="gmail_quote" style="PADDING-LEFT: 1ex; MARGIN: 0px 0px 0px 0.8ex; BORDER-LEFT: #ccc 1px solid">Oooops, I think there is a potential flaw in my Analysis point 2. Imagine<br>that a parsing will just consume an incoming packet - nothing more and
<br>nothing less. After the parsing, data_available probably will not be false<br>because of the way flow buffer is implemented - orig_data_begin_ will only<br>be increased to orig_data_end_ upon the next request! So, at the while loop,
<br>data_available = true, have_pending_request = true (from last request), and<br>ready = true (from last request, too). So we end up with entering the loop<br>again! In fact, to check orig_data_begin < orig_data_end_ in the current
<br>data_available is wrong from its intention.</blockquote>
<div> </div>
<div>I was well aware of that fact, but did not see a problem here, as in my SSL analyzer I manually assure that after parsing a data packet, the corresponding data is consumed. In that case, data_available() works just as intended.
</div>
<div> </div>
<div>I don't think, that it is a good idea to introduce side effects into the end() function just for a slight increase in performance. On one hand, the end() function is in the public interface of the buffer and you should never significantly change the behavior of a public method (some binpac users might rely on that behavior), except for very good reasons. On the other hand, in cases where there might be a serious performance problem with the current behavior, the programmer can manually avoid this by explicitly consuming the data, like I do in my analyzer (of course this should be documented ;-).
</div>
<div> </div>
<blockquote class="gmail_quote" style="PADDING-LEFT: 1ex; MARGIN: 0px 0px 0px 0.8ex; BORDER-LEFT: #ccc 1px solid">I think we need to do something with the flow buffer implementation. The<br>current method to increase orig_data_begin_ upon new request is tricky and
<br>difficult to make things correct. My suggestion is to increase orig_data_begin_<br>in the end() method. The reason is: whenever you request data from the flow<br>buffer, you always call begin() and end() in sequence. These methods are the
<br>only ways that you can access the data in the flow buffer. After the call<br>of end(), the decoder will not call begin() and end() again except in one<br>case which I will address shortly. Thus, it is safe to increase orig_data_begin_
<br>pointer in the end() call. After this change, checking orig_data_begin_ <<br>orig_data_end_ in data_available() method is correct. BTW, it is not needed<br>to check buffer_n_ > 0 in data_available(). Because if something is buffered,
<br>and we reach the entry point of the while loop, orig_data_begin_ must be<br>equal to orig_data_end_. The next parsing must use new data instead of the<br>data in flow buffer. In addition, in NewFrame and NewLine, there's no need
<br>to increase orig_data_begin_ by frame_length_ after the change of end().</blockquote>
<div> </div>
<div>Again, data_available() is a public method that is not just intended for the single use we have discussed so far (I use it explicitly in my analyzer). You can always use it to check whether there is currently any data available, and of course this includes the buffer.
</div><br>Tobias</div>