fixed issue 349227: Uninit error due to DuplicateHandle failure - add check on DuplicateHandle and …

fixed issue 349227: Uninit error due to DuplicateHandle failure - add check on DuplicateHandle and assign duped to be NULL on failure
BUG=349227
R=sky@chromium.org, wfh@chromium.org, jschuh@chromium.org
TEST=manual+drmemory

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=255955

[sky, 2014-03-05 17:36:49]

https://codereview.chromium.org/181013016/diff/1/content/public/test/mock_ren...
File content/public/test/mock_render_process_host.cc (right):

https://codereview.chromium.org/181013016/diff/1/content/public/test/mock_ren...
content/public/test/mock_render_process_host.cc:146: HANDLE duped;
Can you instead explicitly set to duped to NULL here?

[Will Harris, 2014-03-05 17:40:35]

https://codereview.chromium.org/181013016/diff/1/content/public/test/mock_ren...
File content/public/test/mock_render_process_host.cc (right):

https://codereview.chromium.org/181013016/diff/1/content/public/test/mock_ren...
content/public/test/mock_render_process_host.cc:149: duped = NULL;
should this be INVALID_HANDLE_VALUE?

[jschuh, 2014-03-05 17:52:02]

https://codereview.chromium.org/181013016/diff/1/content/public/test/mock_ren...
File content/public/test/mock_render_process_host.cc (right):

https://codereview.chromium.org/181013016/diff/1/content/public/test/mock_ren...
content/public/test/mock_render_process_host.cc:149: duped = NULL;
On 2014/03/05 17:40:35, Will Harris wrote:
> should this be INVALID_HANDLE_VALUE?

No. INVALID_HANDLE_VALUE can bite you because it aliases to the process
pseudo-handle. NULL should be used instead, except for when dealing with the
small number of Win16-derived APIs that require INVALID_HANDLE_VALUE (e.g.
CreateFile and friends).

[zhaoqin, 2014-03-05 19:09:41]

https://codereview.chromium.org/181013016/diff/1/content/public/test/mock_ren...
File content/public/test/mock_render_process_host.cc (right):

https://codereview.chromium.org/181013016/diff/1/content/public/test/mock_ren...
content/public/test/mock_render_process_host.cc:146: HANDLE duped;
On 2014/03/05 17:36:50, sky wrote:
> Can you instead explicitly set to duped to NULL here?

I think it is better to set on failure instead of here.
Because the DuplicateHandle may set duped to be any value (e.g.,
INVALID_HANDLE_VALUE) if the call failed and the value may change cross windows
versions, since the behavior is not documented.
Setting duped to be NULL on failure will handle such cases, so it is safer.

[sky, 2014-03-05 21:23:20]

Actually, why not set it both places? Seems safest to me.

[zhaoqin, 2014-03-10 15:50:43]

ping

[Will Harris, 2014-03-10 15:55:28]

On 2014/03/10 15:50:43, zhaoqin wrote:
> ping

LGTM

[sky, 2014-03-10 16:12:56]

LGTM

[zhaoqin, 2014-03-10 16:14:14]

The CQ bit was checked by zhaoqin@google.com

[zhaoqin1, 2014-03-10 16:31:39]

The CQ bit was unchecked by zhaoqin@chromium.org

[zhaoqin1, 2014-03-10 16:31:41]

The CQ bit was checked by zhaoqin@chromium.org

[zhaoqin, 2014-03-10 16:39:49]

Committed patchset #2 manually as r255955 (presubmit successful).