On Jun 8, 2011, at 8:50 PM, Nick Mathewson wrote:
On Tue, Jun 7, 2011 at 9:30 AM, Nir Soffer <nirsof@xxxxxxxxx> wrote:On Jun 7, 2011, at 4:03 AM, Nick Mathewson wrote:On Mon, Jun 6, 2011 at 8:37 PM, Nir Soffer <nirsof@xxxxxxxxx> wrote:Here's another patch that might make stuff work. Before I'd apply it,I'd like to have a look through everything that's using evbuffer_ptr_set() and evbuffer_ptr right now to make sure that nothing will freak out if it gets a pointer like this. What do you think?Here is a new test case that fail with this patch: /* Search the next 18 bytes for "attack" */tt_int_op(evbuffer_ptr_set(buf, &end, 18, EVBUFFER_PTR_SET), ==, 0);pos = evbuffer_search_range(buf, "attack", 6, NULL, &end); tt_int_op(pos.pos, ==, 11);The use case is simple - I want to limit the search to some range, which happen to be longer then the buffer. To make this work with this patch, Ihave to do something like this: size_t length = evbuffer_get_length(buf); if (limit > length) limit = length; evbuffer_ptr_set(buf, &end, limit, EVBUFFER_PTR_SET); pos = evbuffer_search_range(buf, "needle", 6, NULL, &end); But what I would like to do is this: evbuffer_ptr_set(buf, &end, limit, EVBUFFER_PTR_SET); pos = evbuffer_search_range(buf, "needle", 6, NULL, &end);So evebuffer_ptr_set should succeed even if position is after the end of thebuffer;Hm. I'm not sure that's sensible. It's reasonable to talk about a position "one after the end of the buffer" so that you can say "search the whole buffer", but I"m not sure that it's reasonable to talk about a position more than one after then end of the buffer.
Thinking more about it, it seems that requiring a valid range when you search is ok. Similar APIs like NSString <http://developer.apple.com/library/mac/#documentation/cocoa/reference/foundation/Classes/NSString_Class/Reference/NSString.html > do enforce this by raising an exception when you try to search a range which exceeds the actual range.
So the fix is as you suggested to allow the range that includes the whole buffer.
If you really need the behavior your describing, why not write a trivial function in your program like:
I already did a similar solution, because my code must work now with current release. But I don't like these workarounds, and others will have to re-implement this after wasting time on this confusing one-off bug.
Also, there's another problem with my original patch. By my logic, if the buffer is empty and has no chains at all, then evbuffer_ptr_set(buf, ptr, 0, EVBUFFER_PTR_SET) ought to work, but even with my patch it still doesn't. More work needed, it would seem.
I think I have a working patch. The special "end of buffer" ptr is: ptr.pos = buffer->total_len ptr._internal.chain = NULL ptr._internal.pos_in_chain = 0This is the first byte in the nonexistent chain after last chain. This works with both empty buffer without any chain, and buffer with some chains.
Code that check for null chain works without any change. I added a check in other places for ptr.pos >= buffer->total_len, in case you call a function with the special "end" ptr.
There is one dark corner in the internal evbuffer_getchar - if called with the "end" ptr, it returns -1, which may be valid char on system where char is unsigned. It is used in only one place, using found ptr, which cannot be the end ptr, so it will not cause any problem with current code.
There is some duplication in evbuffer_peek, returning nothing if peeking at the end of the buffer. I did not want to enter the critical section in this case.
The internal evbuffer_substract works, but is not efficient in this case - it will search from the first chain since the end ptr chain is NULL. This code is used once with found ptr, so it should not be a problem.
I added tests for searching and peeking at the end of the buffer and setting a ptr on an empty buffer with no chains.
I did not check yet other code that may use evbuffer_ptr outside buffer.c.
What do you think?
Attachment:
0003-Allow-searching-up-to-the-end-of-the-buffer.patch
Description: Binary data