Had a bug in the handle table unit test.

Had a bug in the handle table unit test. Added GetHandleName to fix the bug and make handle management easier.

TEST=sbox_unittests --gtest_filter=HandleTable.*


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=91270
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=92403

[jschuh, 2011-06-30 01:51:29]

Quick fix for a unit test plus a new helper function.

[nsylvain, 2011-06-30 19:21:46]

http://codereview.chromium.org/7218066/diff/1/sandbox/src/handle_table.cc
File sandbox/src/handle_table.cc (right):

http://codereview.chromium.org/7218066/diff/1/sandbox/src/handle_table.cc#new...
sandbox/src/handle_table.cc:150: size, &size);
should you not align size with handle?

http://codereview.chromium.org/7218066/diff/1/sandbox/src/handle_table_unitte...
File sandbox/src/handle_table_unittest.cc (right):

http://codereview.chromium.org/7218066/diff/1/sandbox/src/handle_table_unitte...
sandbox/src/handle_table_unittest.cc:51:
ASSERT_EQ(NT_SUCCESS(sandbox::GetHandleName(file, &handle_name)), true);
TRUE instead of true ?

[cpu_(ooo_6.6-7.5), 2011-06-30 20:39:39]

lgtm

http://codereview.chromium.org/7218066/diff/1/sandbox/src/handle_table.cc
File sandbox/src/handle_table.cc (right):

http://codereview.chromium.org/7218066/diff/1/sandbox/src/handle_table.cc#new...
sandbox/src/handle_table.cc:156: return result;
I would return a boolean. That reflects its current usage.

[jschuh, 2011-07-01 01:24:26]

http://codereview.chromium.org/7218066/diff/1/sandbox/src/handle_table.cc
File sandbox/src/handle_table.cc (right):

http://codereview.chromium.org/7218066/diff/1/sandbox/src/handle_table.cc#new...
sandbox/src/handle_table.cc:150: size, &size);
On 2011/06/30 19:21:46, nsylvain wrote:
> should you not align size with handle?

Done.

http://codereview.chromium.org/7218066/diff/1/sandbox/src/handle_table.cc#new...
sandbox/src/handle_table.cc:156: return result;
On 2011/06/30 20:39:39, cpu wrote:
> I would return a boolean. That reflects its current usage.

Done.

http://codereview.chromium.org/7218066/diff/1/sandbox/src/handle_table_unitte...
File sandbox/src/handle_table_unittest.cc (right):

http://codereview.chromium.org/7218066/diff/1/sandbox/src/handle_table_unitte...
sandbox/src/handle_table_unittest.cc:51:
ASSERT_EQ(NT_SUCCESS(sandbox::GetHandleName(file, &handle_name)), true);
On 2011/06/30 19:21:46, nsylvain wrote:
> TRUE instead of true ?

No, using a bool not BOOL.

[commit-bot: I haz the power, 2011-07-01 05:10:57]

Change committed as 91270

[nsylvain, 2011-07-01 19:23:28]

http://codereview.chromium.org/7218066/diff/1/sandbox/src/handle_table_unitte...
File sandbox/src/handle_table_unittest.cc (right):

http://codereview.chromium.org/7218066/diff/1/sandbox/src/handle_table_unitte...
sandbox/src/handle_table_unittest.cc:51:
ASSERT_EQ(NT_SUCCESS(sandbox::GetHandleName(file, &handle_name)), true);
On 2011/07/01 01:24:26, Justin Schuh wrote:
> On 2011/06/30 19:21:46, nsylvain wrote:
> > TRUE instead of true ?
> 
> No, using a bool not BOOL.

NT_SUCCESS returns TRUE or FALSE, and ASSERT_EQ compares that with "true", this
will work, but i think it is more correct to use TRUE.