----------------------------------------------------------- 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 <http://reviews.m5sim.org/r/365/#comment832> 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
