[Repost] New ChannelBuffer type proposed
Trustin Lee (이희승)
trustin at gmail.com
Tue Sep 29 04:46:59 EDT 2009
I see a problem with ChannelBuffers.wrappedBuffer(). It's not a big
problem as we can fix it easily though. The problem is that
ChannelBuffers.wrappedBuffer(ChannelBuffer) will create a
CompositeChannelBuffer even when the specified buffer is not a
composite buffer.
Due to the latest regression I've put into 3.1.4, I think I have to
release 3.1.5 soon. And then I will merge your work to the trunk and
proceed to 3.2.0.ALPHA1.
Thanks!
— Trustin Lee, http://gleamynode.net/
On Sun, Sep 27, 2009 at 4:33 PM, Frederic Bregier <fredbregier at free.fr> wrote:
>
> Hi Trustin,
>
> I've just create the "aggregate" branch. It includes the modified
> CompositeChannelBuffer, ChannelBuffers (correction of wrapped methods) , and
> the extension of the existing two JUnit test case (BigEndianComposite and
> LittleEndianComposite ChannelBufferTest).
>
> I've rerun the Junit test and all seem ok.
>
> Cheers,
> Frederic
>
>
> Frederic Bregier wrote:
>>
>> Hi Trustin,
>>
>> Ok with the new branch.
>> I will create it named "aggregate" in order to know for what reason it is
>> there, if you're ok.
>>
>> Cheers,
>> Frederic
>>
>>
>> Trustin Lee wrote:
>>>
>>> Hi Frederic,
>>>
>>> On Fri, Sep 25, 2009 at 2:14 PM, Frederic Bregier <fredbregier at free.fr>
>>> wrote:
>>>>
>>>> Hi Trustin,
>>>>
>>>> I'm agree that we could replace CompositeChannelBuffer with this one (so
>>>> not
>>>> introducing a new name).
>>>> I had done like this in order to allow anyone to test it without
>>>> removing
>>>> CCB from Netty.
>>>> Now that you think it is ok, I will propose you the new code for CCB
>>>> using
>>>> the one from the Aggregate.
>>>>
>>>> For wrappedCheckedBuffer, in fact, it was strictly in the same mind that
>>>> for
>>>> CCB, that is to say that I did not wanted to remove the default
>>>> wrappedBuffer from ChannelBuffers. But of course, I would propose you
>>>> the
>>>> patch for ChannelBuffers such that wrappedBuffer take care of this
>>>> without
>>>> introducing any new method...
>>>>
>>>> Give me a day or two (at least before the end of the week-end), and I
>>>> will
>>>> post a new set of java code (replacement for CompositeChannelBuffer and
>>>> its
>>>> Junit test, and the patch for ChannelBuffers).
>>>> Are you ok with this ?
>>>
>>> Sure. Sounds great!
>>>
>>>> I could also directly patch on trunk but if you don't mind, I would
>>>> prefer
>>>> you check a last time before comiting, ok ?
>>>
>>> Let's create a new temporary branch. I still need to maintain 3.1.x
>>> branch and this feature should go to 3.2. WDYT?
>>>
>>> Thanks!
>>>
>>> — Trustin Lee, http://gleamynode.net/
>>>
>>>>
>>>> Cheers,
>>>> Frederic
>>>>
>>>>
>>>>
>>>> Trustin Lee wrote:
>>>>>
>>>>> Hi Frederic,
>>>>>
>>>>> The code looks OK, but I'm still not sure if we need to make
>>>>> wrappedCheckedBuffer() public. We could modify
>>>>> ChannelBuffers.wrappedBuffer() to recognize composite buffers using
>>>>> instanceof.
>>>>>
>>>>> Also, if your implementation works in the same way with
>>>>> CompositeChannelBuffer, then we could simply replace it with your
>>>>> buffer type and rename it to CompositeChannelBuffer, rather than
>>>>> introducing a new name?
>>>>>
>>>>> Thanks for your patience
>>>>>
>>>>> — Trustin Lee, http://gleamynode.net/
>>>>>
>>>>>
>>>>>
>>>>> On Thu, Sep 10, 2009 at 5:13 AM, Frederic Bregier <fredbregier at free.fr>
>>>>> wrote:
>>>>>>
>>>>>> Hi Trustin,
>>>>>>
>>>>>> I've found a bug in the slice implementation when start is not at 0.
>>>>>> So I correct this and add a last line in the testDiscardReadBytes
>>>>>> method
>>>>>> of
>>>>>> the Junit test (in Aggregate since it override the default one from
>>>>>> Abstract
>>>>>> Junit test since this original test could be buggy for the test of not
>>>>>> equality of extra bytes as I wrote in previous mail).
>>>>>>
>>>>>> Here are the two files:
>>>>>> http://n2.nabble.com/file/n3613619/AggregateChannelBuffer.java
>>>>>> AggregateChannelBuffer.java
>>>>>> http://n2.nabble.com/file/n3613619/AggregateChannelBufferTest.java
>>>>>> AggregateChannelBufferTest.java
>>>>>>
>>>>>> Cheers,
>>>>>> Frederic
>>>>>>
>>>>>>
>>>>>> Frederic Bregier wrote:
>>>>>>>
>>>>>>> Hi Trustin,
>>>>>>>
>>>>>>> Ok I write and test the slice functionality (using Junit).
>>>>>>> It seems OK.
>>>>>>>
>>>>>>> I still feel like writeBytes should be left as the default
>>>>>>> implementation
>>>>>>> (ie the one from AbstractChannelBuffer) since the setBytes method
>>>>>>> should
>>>>>>> do the trick as usual.
>>>>>>> I test by using the default implementation, and all tests pass too...
>>>>>>> So, except if you think that writing by slice from source could be
>>>>>>> useful
>>>>>>> (if the source is an aggregate one), I think this method could be
>>>>>>> removed
>>>>>>> from Aggregate (so not implementing in the modified version of
>>>>>>> Component
>>>>>>> if ok) so letting the default one be called.
>>>>>>> I put a comment on this method.
>>>>>>>
>>>>>>> Cheers,
>>>>>>> Frederic
>>>>>>>
>>>>>>> http://n2.nabble.com/file/n3565134/AggregateChannelBuffer.java
>>>>>>> AggregateChannelBuffer.java
>>>>>>> http://n2.nabble.com/file/n3565134/AggregateChannelBufferTest.java
>>>>>>> AggregateChannelBufferTest.java
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>> -----
>>>>>> Hardware/Software Architect
>>>>>> --
>>>>>> View this message in context:
>>>>>> http://n2.nabble.com/New-ChannelBuffer-type-proposed-tp3496606p3613619.html
>>>>>> Sent from the Netty Developer Group mailing list archive at
>>>>>> Nabble.com.
>>>>>> _______________________________________________
>>>>>> netty-dev mailing list
>>>>>> netty-dev at lists.jboss.org
>>>>>> https://lists.jboss.org/mailman/listinfo/netty-dev
>>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> netty-dev mailing list
>>>>> netty-dev at lists.jboss.org
>>>>> https://lists.jboss.org/mailman/listinfo/netty-dev
>>>>>
>>>>>
>>>>
>>>>
>>>> -----
>>>> Hardware/Software Architect
>>>> --
>>>> View this message in context:
>>>> http://n2.nabble.com/New-ChannelBuffer-type-proposed-tp3496606p3710508.html
>>>> Sent from the Netty Developer Group mailing list archive at Nabble.com.
>>>>
>>>> _______________________________________________
>>>> netty-dev mailing list
>>>> netty-dev at lists.jboss.org
>>>> https://lists.jboss.org/mailman/listinfo/netty-dev
>>>>
>>>
>>> _______________________________________________
>>> netty-dev mailing list
>>> netty-dev at lists.jboss.org
>>> https://lists.jboss.org/mailman/listinfo/netty-dev
>>>
>>>
>>
>>
>
>
> -----
> Hardware/Software Architect
> --
> View this message in context: http://n2.nabble.com/New-ChannelBuffer-type-proposed-tp3496606p3719936.html
> Sent from the Netty Developer Group mailing list archive at Nabble.com.
>
> _______________________________________________
> netty-dev mailing list
> netty-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/netty-dev
>
More information about the netty-dev
mailing list