Re: [Libevent-users] [PATCH] evbuffer_search_range fails when searching the last byte

[Nick Mathewson <nickm@xxxxxxxxxxxxx>]

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, I
> have 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 the
> buffer;

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.

This is analogous to how pointers are defined to work in C: if you
have a 100-element array a, then &a[100] is defined, but &a[101] is
not.

The implementation has an issue, too: pos->pos will be set with its
pos field out of synch with the actual value of the _internal fields,
so that looking at pos->pos will tell you that you're searching up to
the Nth byte of the buffer, when really you're only searching up to
whatever the length of the buffer was when you called
evbuffer_ptr_set().

If you really need the behavior your describing, why not write a
trivial function in your program like:

int
my_evbuffer_ptr_set_clipped(struct evbuffer *buf, struct evbuffer_ptr *pos,
    size_t position, enum evbuffer_ptr_how how)
{
   size_t max;
   switch (how) {
     case EVBUFFER_PTR_SET:
       max = evbuffer_get_length(buf);
       break;
      case EVBUFFER_PTR_ADD:
       max = evbuffer_get_length(buf) - pos->pos;
       break;
   }
   if (position > max)
      position = max;
   return evbuffer_ptr_set(buf, pos, position, how);
}

and call that instead?

....
....

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.


peace,
-- 
Nick
***********************************************************************
To unsubscribe, send an e-mail to majordomo@xxxxxxxxxxxxx with
unsubscribe libevent-users    in the body.