Re: [m5-dev] Review Request: RefCount: Add a unit test for reference counting pointers.

2011-01-04 Thread nathan binkert 
> So do we want to define a standard set of assert like
> macros/functions/whatever? If we're going to go for a fixed format we
> should stick that in a header somewhere. I went with asserts because
> they pretty conveniently check the result and blow up if there's a
> problem, but printing PASS/FAIL would be fine too.

Sounds like a really good idea to me.

  Nate
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: RefCount: Add a unit test for reference counting pointers.

2011-01-04 Thread Gabe Black 
So do we want to define a standard set of assert like
macros/functions/whatever? If we're going to go for a fixed format we
should stick that in a header somewhere. I went with asserts because
they pretty conveniently check the result and blow up if there's a
problem, but printing PASS/FAIL would be fine too.

Gabe

Ali Saidi wrote:
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/365/
>
>
> It's pretty good. I think Nate's suggestion is great and maybe a little 
> comment before each test "// Test setting variable to NULL lowers ref count" 
> would be great. 
>
> - Ali
>
>
> On January 3rd, 2011, 12:56 p.m., Gabe Black wrote:
>
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt,
> and Nathan Binkert.
> By Gabe Black.
>
> /Updated 2011-01-03 12:56:11/
>
>
>   Description
>
> RefCount: Add a unit test for reference counting pointers.
>
> This test exercises each of the functions in the reference counting pointer
> implementation individually (except get()) and verifies they have some
> minimially expected behavior. It also checks that reference counted objects
> are freed when their usage count goes to 0 in some basic situations,
> specifically a pointer being set to NULL and a pointer being deleted.
>
>
>   Diffs
>
> * src/unittest/SConscript (3a790012d6ed)
> * src/unittest/refcnttest.cc (PRE-CREATION)
>
> View Diff 
>
> 
>
> ___
> m5-dev mailing list
> m5-dev@m5sim.org
> http://m5sim.org/mailman/listinfo/m5-dev
>   

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: RefCount: Add a unit test for reference counting pointers.

2011-01-04 Thread Ali Saidi 

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/365/#review618
---


It's pretty good. I think Nate's suggestion is great and maybe a little comment 
before each test "// Test setting variable to NULL lowers ref count" would be 
great. 

- Ali


On 2011-01-03 12:56:11, Gabe Black wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/365/
> ---
> 
> (Updated 2011-01-03 12:56:11)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> RefCount: Add a unit test for reference counting pointers.
> 
> This test exercises each of the functions in the reference counting pointer
> implementation individually (except get()) and verifies they have some
> minimially expected behavior. It also checks that reference counted objects
> are freed when their usage count goes to 0 in some basic situations,
> specifically a pointer being set to NULL and a pointer being deleted.
> 
> 
> Diffs
> -
> 
>   src/unittest/SConscript 3a790012d6ed 
>   src/unittest/refcnttest.cc PRE-CREATION 
> 
> Diff: http://reviews.m5sim.org/r/365/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gabe
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: RefCount: Add a unit test for reference counting pointers.

2011-01-03 Thread Nathan Binkert 

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/365/#review597
---

Ship it!


I have some suggestions, but overall, it looks good.  It would be nice to at 
least have the return value of the test indicate success or failure as it would 
be nice to actually include these in the regressions in the future.  Thanks!


src/unittest/refcnttest.cc


This diff looks overall correct, though the use of assert to do this is a 
bit odd.  Perhaps replace assert with something more like (though it might not 
be very meaningful without a description of what you're testing to go with it):

#define test(X) do {
bool test = (X);
cprintf("%-7s (line %3d) %s\n", test ? "SUCCESS" : "FAIL", __LINE__, #X);
if (!test) {
Debug::break();
failures++;
}
} while (0)

at the exit of main(), i'd print "total failures" and also do "return 
!!failures"  "!!x" is equivalent to x ? 1 : 0 or int(bool(x)), if you don't 
like the shorthand.


- Nathan


On 2011-01-03 12:56:11, Gabe Black wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/365/
> ---
> 
> (Updated 2011-01-03 12:56:11)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> RefCount: Add a unit test for reference counting pointers.
> 
> This test exercises each of the functions in the reference counting pointer
> implementation individually (except get()) and verifies they have some
> minimially expected behavior. It also checks that reference counted objects
> are freed when their usage count goes to 0 in some basic situations,
> specifically a pointer being set to NULL and a pointer being deleted.
> 
> 
> Diffs
> -
> 
>   src/unittest/SConscript 3a790012d6ed 
>   src/unittest/refcnttest.cc PRE-CREATION 
> 
> Diff: http://reviews.m5sim.org/r/365/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gabe
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: RefCount: Add a unit test for reference counting pointers.

2011-01-03 Thread Gabe Black 

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/365/
---

(Updated 2011-01-03 12:56:11.143277)


Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan 
Binkert.


Summary
---

RefCount: Add a unit test for reference counting pointers.

This test exercises each of the functions in the reference counting pointer
implementation individually (except get()) and verifies they have some
minimially expected behavior. It also checks that reference counted objects
are freed when their usage count goes to 0 in some basic situations,
specifically a pointer being set to NULL and a pointer being deleted.


Diffs (updated)
-

  src/unittest/SConscript 3a790012d6ed 
  src/unittest/refcnttest.cc PRE-CREATION 

Diff: http://reviews.m5sim.org/r/365/diff


Testing
---


Thanks,

Gabe

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

[m5-dev] Review Request: RefCount: Add a unit test for reference counting pointers.

2011-01-03 Thread Gabe Black 

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/365/
---

Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan 
Binkert.


Summary
---

RefCount: Add a unit test for reference counting pointers.

This test exercises each of the functions in the reference counting pointer
implementation individually (except get()) and verifies they have some
minimially expected behavior. It also checks that reference counted objects
are freed when their usage count goes to 0 in some basic situations,
specifically a pointer being set to NULL and a pointer being deleted.


Diffs
-

  src/unittest/SConscript 3a790012d6ed 
  src/unittest/refcnttest.cc PRE-CREATION 

Diff: http://reviews.m5sim.org/r/365/diff


Testing
---


Thanks,

Gabe

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev