preemptive bug report?

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

preemptive bug report?

James Lemieux
Dearest Fools,
 
    The clown car we call FilterList.matchNone() probably has a bug, though I spotted it via sight reading code, not by receiving a stacktrace. The line:
 
                // filter out all remaining items in this list
                updates.addDelete(0, size());
 
should probably be:
 
                // filter out all remaining items in this list
                updates.addDelete(0, size()-1);
 
because addDelete() takes the startIndex and endIndex as args, not the length. Doh!
 
Bonzo Lemieux.
Reply | Threaded
Open this post in threaded view
|

Re: preemptive bug report?

Rob Eden
BasicEventList.clear() does use "size() - 1", so I would assume you're right, Bonzo. I'll write a test case to catch the problem and fix it.

Weebo

On Aug 2, 2005, at 2:58 PM, James Lemieux wrote:

Dearest Fools,
 
    The clown car we call FilterList.matchNone() probably has a bug, though I spotted it via sight reading code, not by receiving a stacktrace. The line:
 
                // filter out all remaining items in this list
                updates.addDelete(0, size());
 
should probably be:
 
                // filter out all remaining items in this list
                updates.addDelete(0, size()-1);
 
because addDelete() takes the startIndex and endIndex as args, not the length. Doh!
 
Bonzo Lemieux.


-- 

"Let your heart soar as high as it will.
 Refuse to be average."
                           - A. W. Tozer

Reply | Threaded
Open this post in threaded view
|

Re: preemptive bug report?

Rob Eden
Well, it actually works either way because ListEventBlock is doing sanity checking. However, since James is correct and the other way is the correct value, I've changed it. There's also now a unit test that hits those methods.

Rob

On Aug 2, 2005, at 4:41 PM, Rob Eden wrote:

BasicEventList.clear() does use "size() - 1", so I would assume you're right, Bonzo. I'll write a test case to catch the problem and fix it.

Weebo

On Aug 2, 2005, at 2:58 PM, James Lemieux wrote:

Dearest Fools,
 
    The clown car we call FilterList.matchNone() probably has a bug, though I spotted it via sight reading code, not by receiving a stacktrace. The line:
 
                // filter out all remaining items in this list
                updates.addDelete(0, size());
 
should probably be:
 
                // filter out all remaining items in this list
                updates.addDelete(0, size()-1);
 
because addDelete() takes the startIndex and endIndex as args, not the length. Doh!
 
Bonzo Lemieux.


-- 

"Let your heart soar as high as it will.
 Refuse to be average."
                           - A. W. Tozer



-- 

"Let your heart soar as high as it will.
 Refuse to be average."
                           - A. W. Tozer

Reply | Threaded
Open this post in threaded view
|

Re: preemptive bug report?

James Lemieux
To actually notice the error, you can use ConsistencyTestList (a misnomer) which should fail on the errorific version. Add that guy as a listener to the FilterList in your testcase and you should be able to flush the bug out.
 
FYI, ListEventBlock isn't guarding against this case. It isn't aware of the size of the list. It only compares indices with each other for sanity.
 
Jesse James.

 
On 8/2/05, Rob Eden <[hidden email]> wrote:
Well, it actually works either way because ListEventBlock is doing sanity checking. However, since James is correct and the other way is the correct value, I've changed it. There's also now a unit test that hits those methods.

 
Rob

On Aug 2, 2005, at 4:41 PM, Rob Eden wrote:

BasicEventList.clear() does use "size() - 1", so I would assume you're right, Bonzo. I'll write a test case to catch the problem and fix it.

 
Weebo

On Aug 2, 2005, at 2:58 PM, James Lemieux wrote:

Dearest Fools,
 
    The clown car we call FilterList.matchNone() probably has a bug, though I spotted it via sight reading code, not by receiving a stacktrace. The line:
 
                // filter out all remaining items in this list
                updates.addDelete(0, size());
 
should probably be:
 
                // filter out all remaining items in this list
                updates.addDelete(0, size()-1);
 
because addDelete() takes the startIndex and endIndex as args, not the length. Doh!
 
Bonzo Lemieux.


 
-- 

 
"Let your heart soar as high as it will.
 Refuse to be average."
                           - A. W. Tozer

 


 
-- 

 
"Let your heart soar as high as it will.
 Refuse to be average."
                           - A. W. Tozer

 

Reply | Threaded
Open this post in threaded view
|

Re: preemptive bug report?

Rob Eden
Sure 'nuf. Thanks.

Rob

On Aug 2, 2005, at 5:24 PM, James Lemieux wrote:

> To actually notice the error, you can use ConsistencyTestList (a  
> misnomer) which should fail on the errorific version. Add that guy  
> as a listener to the FilterList in your testcase and you should be  
> able to flush the bug out.
>
> FYI, ListEventBlock isn't guarding against this case. It isn't  
> aware of the size of the list. It only compares indices with each  
> other for sanity.
>
> Jesse James.


--

"Let your heart soar as high as it will.
  Refuse to be average."
                            - A. W. Tozer


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]