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

[Nick Mathewson <nickm@xxxxxxxxxxxxx>]

On Wed, Jun 8, 2011 at 8:38 PM, Nir Soffer <nirsof@xxxxxxxxx> wrote:
 [...]
>> 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 = 0
>
> This 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.

This, with your change to fix the race conditions, looks good to me.
I've got a few tweaks to apply on top of it, and I've split out the
parts that were already in my patch from the ones that weren't, for
git clarit.  Now the whole mess is in my github repository, in a
branch called "21_end_of_buffer" in
git://github.com/nmathewson/Libevent.git .

 [...]
> 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.

Well, because char is going to be 8 bits, ((char)-1) *will* be cast to
a valid character everywhere.  I've changed it to follow the getchar
interface of returning either an unsigned char or returning -1.

> 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.

Hm.  Just to be sure, I'm having this check the same condition as the
other code (_internal.chain==NULL).

> 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?

If you like my tweaks, I think this is a branch ready to merge onto 2.1.

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