Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(294)

Side by Side Diff: base/win/scoped_process_information.h

Issue 9700038: ScopedProcessInformation protects against process/thread handle leaks from CreateProcess calls. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Remove changes from another CL. Created 8 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch | Annotate | Revision Log
OLDNEW
(Empty)
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file.
4
5 #ifndef BASE_WIN_SCOPED_PROCESS_INFORMATION_H_
6 #define BASE_WIN_SCOPED_PROCESS_INFORMATION_H_
7 #pragma once
8
9 #include <windows.h>
10
11 #include "base/win/scoped_handle.h"
12
13 namespace base {
14 namespace win {
15
16 class ProcessInfoTraits {
17 public:
18 typedef PROCESS_INFORMATION Handle;
19
20 // Releases the PROCESS_INFORMATION's process and thread handles.
21 static bool CloseHandle(PROCESS_INFORMATION handle) {
alexeypa (please no reviews) 2012/03/15 02:49:16 You really want "PROCESS_INFORMATION& handle" here
erikwright (departed) 2012/03/15 03:29:17 Well, the GenericScopedHandle is not expecting thi
22 bool ret = true;
23 if (handle.hThread && !::CloseHandle(handle.hThread))
24 ret = false;
25 if (handle.hProcess && !::CloseHandle(handle.hProcess))
26 ret = false;
27 handle = NullHandle();
alexeypa (please no reviews) 2012/03/15 02:49:16 |handle| is a local copy on the stack. This does n
erikwright (departed) 2012/03/15 03:29:17 Done.
28 return ret;
29 }
30
31 static bool IsHandleValid(PROCESS_INFORMATION handle) {
alexeypa (please no reviews) 2012/03/15 02:49:16 const PROCESS_INFORMATION&
erikwright (departed) 2012/03/15 03:29:17 Done.
32 return handle.hThread || handle.hProcess ||
33 handle.dwProcessId || handle.dwThreadId;
34 }
35
36 static bool IsSame(PROCESS_INFORMATION lhs, PROCESS_INFORMATION rhs) {
alexeypa (please no reviews) 2012/03/15 02:49:16 const PROCESS_INFORMATION&
erikwright (departed) 2012/03/15 03:29:17 Done.
37 return lhs.hProcess == rhs.hProcess &&
38 lhs.hThread == rhs.hThread &&
39 lhs.dwProcessId == rhs.dwProcessId &&
40 lhs.dwThreadId == rhs.dwThreadId;
41 }
42
43 static PROCESS_INFORMATION NullHandle() {
alexeypa (please no reviews) 2012/03/15 02:49:16 I don't think returning raw structures like this i
erikwright (departed) 2012/03/15 03:29:17 Hmm. Then the client needs to have both a PROCESS_
alexeypa (please no reviews) 2012/03/15 16:38:27 After a second thought, wrapping a pointer does no
44 PROCESS_INFORMATION null_process_information = {0};
45 return null_process_information;
46 }
47 };
48
49 // Manages the closing of process and thread handles from PROCESS_INFORMATION
50 // structures. Allows clients to take ownership of either handle independently.
51 class ScopedProcessInformation : public GenericScopedHandle<ProcessInfoTraits> {
grt (UTC plus 2) 2012/03/15 02:59:30 did you consider something like this: class Scope
erikwright (departed) 2012/03/15 03:29:17 WRT Get(), it could also just be process_handle()
grt (UTC plus 2) 2012/03/15 18:01:09 It might be a can of worms, but you could: Handle*
52 public:
53 ScopedProcessInformation()
54 : GenericScopedHandle(ProcessInfoTraits::NullHandle()) {}
55
56 explicit ScopedProcessInformation(Handle handle)
57 : GenericScopedHandle(handle) {}
58
59 // Transfers ownership of the process handle away from this object. The
60 // hProcess and dwProcessId members will be reset.
61 HANDLE TakeProcessHandle() {
62 PROCESS_INFORMATION process_info = Take();
63 HANDLE process = process_info.hProcess;
64 process_info.hProcess = NULL;
65 process_info.dwProcessId = 0;
66 Set(process_info);
alexeypa (please no reviews) 2012/03/15 16:38:27 I would just modified the structure directly here
erikwright (departed) 2012/03/15 18:02:44 It's a private member. I'd rather not make it prot
67 return process;
68 }
69
70 // Transfers ownership of the thread handle away from this object. The hThread
71 // and dwThreadId members will be reset.
72 HANDLE TakeThreadHandle() {
73 PROCESS_INFORMATION process_info = Take();
74 HANDLE thread = process_info.hThread;
75 process_info.hThread = NULL;
76 process_info.dwThreadId = 0;
alexeypa (please no reviews) 2012/03/15 16:38:27 Same as above.
77 Set(process_info);
78 return thread;
79 }
80 };
81
82 } // namespace win
83 } // namespace base
84
85 #endif // BASE_SCOPED_HANDLE_WIN_H_
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698