ipc/ipc_channel_win.cc - Issue 588383003: IPC: Use ScopedHandle instead of a raw HANDLE for the private members.

Unified Diff: ipc/ipc_channel_win.cc

Issue 588383003: IPC: Use ScopedHandle instead of a raw HANDLE for the private members. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Created 6 years, 3 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 side-by-side diff with in-line comments

Download patch

Index: ipc/ipc_channel_win.cc

diff --git a/ipc/ipc_channel_win.cc b/ipc/ipc_channel_win.cc

index cff6cd4335686c09007f18ce054608729b0b6659..1230c2a821406721307579cb8a5d9e625297c95a 100644

--- a/ipc/ipc_channel_win.cc

+++ b/ipc/ipc_channel_win.cc

@@ -59,7 +59,6 @@ ChannelWin::ChannelWin(const IPC::ChannelHandle &channel_handle,

: ChannelReader(listener),

input_state_(this),

output_state_(this),

- pipe_(INVALID_HANDLE_VALUE),

peer_pid_(base::kNullProcessId),

waiting_connect_(mode & MODE_SERVER_FLAG),

processing_incoming_(false),

@@ -85,14 +84,12 @@ void ChannelWin::Close() {

debug_flags_ |= CLOSED;

if (input_state_.is_pending || output_state_.is_pending)

- CancelIo(pipe_);

+ CancelIo(pipe_.Get());

// Closing the handle at this point prevents us from issuing more requests

// form OnIOCompleted().

- if (pipe_ != INVALID_HANDLE_VALUE) {

- CloseHandle(pipe_);

- pipe_ = INVALID_HANDLE_VALUE;

- }

+ if (pipe_.IsValid())

+ pipe_.Close();

if (input_state_.is_pending)

debug_flags_ |= WAIT_FOR_READ;

@@ -145,9 +142,7 @@ base::ProcessId ChannelWin::GetSelfPID() const {

}

cpu_(ooo_6.6-7.5) 2014/09/25 23:07:27 morita has a lgtmed change here.

rvargas (doing something else) 2014/09/25 23:26:20 Yes, I know. This part is just for completeness.

ChannelHandle ChannelWin::TakePipeHandle() {

- ChannelHandle handle = ChannelHandle(pipe_);

- pipe_ = INVALID_HANDLE_VALUE;

- return handle;

+ return ChannelHandle(pipe_.Take());

}

// static

@@ -164,12 +159,12 @@ ChannelWin::ReadState ChannelWin::ReadData(

char* buffer,

int buffer_len,

int* /* bytes_read */) {

- if (INVALID_HANDLE_VALUE == pipe_)

+ if (!pipe_.IsValid())

return READ_FAILED;

debug_flags_ |= READ_MSG;

DWORD bytes_read = 0;

- BOOL ok = ReadFile(pipe_, buffer, buffer_len,

+ BOOL ok = ReadFile(pipe_.Get(), buffer, buffer_len,

&bytes_read, &input_state_.context.overlapped);

if (!ok) {

DWORD err = GetLastError();

@@ -250,11 +245,15 @@ const base::string16 ChannelWin::PipeName(

}

bool ChannelWin::CreatePipe(const IPC::ChannelHandle &channel_handle,

- Mode mode) {

- DCHECK_EQ(INVALID_HANDLE_VALUE, pipe_);

+ Mode mode) {

+ DCHECK(!pipe_.IsValid());

base::string16 pipe_name;

// If we already have a valid pipe for channel just copy it.

if (channel_handle.pipe.handle) {

+ // TODO(rvargas) crbug.com/415294: ChannelHandle should either go away in

+ // favor of two independent entities (name/file), or it should be a move-

+ // only type with a base::File member. In any case, this code should not

cpu_(ooo_6.6-7.5) 2014/09/25 23:07:27 is this justin thing I was talking about?

rvargas (doing something else) 2014/09/25 23:26:20 Nope. Amit added this long time ago... it's not us

+ // call DuplicateHandle.

DCHECK(channel_handle.name.empty());

pipe_name = L"Not Available"; // Just used for LOG

// Check that the given pipe confirms to the specified mode. We can

@@ -268,46 +267,48 @@ bool ChannelWin::CreatePipe(const IPC::ChannelHandle &channel_handle,

LOG(WARNING) << "Inconsistent open mode. Mode :" << mode;

return false;

}

+ HANDLE local_handle;

if (!DuplicateHandle(GetCurrentProcess(),

channel_handle.pipe.handle,

GetCurrentProcess(),

- &pipe_,

+ &local_handle,

FALSE,

DUPLICATE_SAME_ACCESS)) {

LOG(WARNING) << "DuplicateHandle failed. Error :" << GetLastError();

return false;

}

+ pipe_.Set(local_handle);

} else if (mode & MODE_SERVER_FLAG) {

DCHECK(!channel_handle.pipe.handle);

const DWORD open_mode = PIPE_ACCESS_DUPLEX | FILE_FLAG_OVERLAPPED |

FILE_FLAG_FIRST_PIPE_INSTANCE;

pipe_name = PipeName(channel_handle.name, &client_secret_);

validate_client_ = !!client_secret_;

- pipe_ = CreateNamedPipeW(pipe_name.c_str(),

- open_mode,

- PIPE_TYPE_BYTE | PIPE_READMODE_BYTE,

- 1,

- Channel::kReadBufferSize,

- 5000,

- NULL);

+ pipe_.Set(CreateNamedPipeW(pipe_name.c_str(),

+ open_mode,

+ PIPE_TYPE_BYTE | PIPE_READMODE_BYTE,

+ 1,

+ Channel::kReadBufferSize,

+ 5000,

+ NULL));

} else if (mode & MODE_CLIENT_FLAG) {

DCHECK(!channel_handle.pipe.handle);

pipe_name = PipeName(channel_handle.name, &client_secret_);

- pipe_ = CreateFileW(pipe_name.c_str(),

- GENERIC_READ | GENERIC_WRITE,

- 0,

- NULL,

- OPEN_EXISTING,

- SECURITY_SQOS_PRESENT | SECURITY_IDENTIFICATION |

- FILE_FLAG_OVERLAPPED,

- NULL);

+ pipe_.Set(CreateFileW(pipe_name.c_str(),

+ GENERIC_READ | GENERIC_WRITE,

+ 0,

+ NULL,

+ OPEN_EXISTING,

+ SECURITY_SQOS_PRESENT | SECURITY_IDENTIFICATION |

+ FILE_FLAG_OVERLAPPED,

+ NULL));

} else {

NOTREACHED();

}

- if (pipe_ == INVALID_HANDLE_VALUE) {

+ if (!pipe_.IsValid()) {

// If this process is being closed, the pipe may be gone already.

PLOG(WARNING) << "Unable to create pipe \"" << pipe_name << "\" in "

<< (mode & MODE_SERVER_FLAG ? "server" : "client") << " mode";

@@ -324,8 +325,7 @@ bool ChannelWin::CreatePipe(const IPC::ChannelHandle &channel_handle,

int32 secret = validate_client_ ? 0 : client_secret_;

if (!m->WriteInt(GetCurrentProcessId()) ||

(secret && !m->WriteUInt32(secret))) {

- CloseHandle(pipe_);

- pipe_ = INVALID_HANDLE_VALUE;

+ pipe_.Close();

return false;

}

@@ -341,10 +341,10 @@ bool ChannelWin::Connect() {

if (!thread_check_.get())

thread_check_.reset(new base::ThreadChecker());

- if (pipe_ == INVALID_HANDLE_VALUE)

+ if (!pipe_.IsValid())

return false;

- base::MessageLoopForIO::current()->RegisterIOHandler(pipe_, this);

+ base::MessageLoopForIO::current()->RegisterIOHandler(pipe_.Get(), this);

// Check to see if there is a client connected to our pipe...

if (waiting_connect_)

@@ -374,10 +374,10 @@ bool ChannelWin::ProcessConnection() {

input_state_.is_pending = false;

// Do we have a client connected to our pipe?

- if (INVALID_HANDLE_VALUE == pipe_)

+ if (!pipe_.IsValid())

return false;

- BOOL ok = ConnectNamedPipe(pipe_, &input_state_.context.overlapped);

+ BOOL ok = ConnectNamedPipe(pipe_.Get(), &input_state_.context.overlapped);

debug_flags_ |= CALLED_CONNECT;

DWORD err = GetLastError();

@@ -433,7 +433,7 @@ bool ChannelWin::ProcessOutgoingMessages(

if (output_queue_.empty())

return true;

- if (INVALID_HANDLE_VALUE == pipe_)

+ if (!pipe_.IsValid())

return false;

// Write to pipe...

@@ -444,7 +444,7 @@ bool ChannelWin::ProcessOutgoingMessages(

writing_ = true;

write_size_ = static_cast<uint32>(m->size());

write_error_ = 0;

- BOOL ok = WriteFile(pipe_,

+ BOOL ok = WriteFile(pipe_.Get(),

m->data(),

write_size_,

NULL,

@@ -507,7 +507,7 @@ void ChannelWin::OnIOCompleted(

input_state_.is_pending = false;

if (!bytes_transfered)

ok = false;

- else if (pipe_ != INVALID_HANDLE_VALUE)

+ else if (pipe_.IsValid())

ok = AsyncReadComplete(bytes_transfered);

} else {

DCHECK(!bytes_transfered);

@@ -528,7 +528,7 @@ void ChannelWin::OnIOCompleted(

}

ok = ProcessOutgoingMessages(context, bytes_transfered);

}

- if (!ok && INVALID_HANDLE_VALUE != pipe_) {

+ if (!ok && pipe_.IsValid()) {

// We don't want to re-enter Close().

Close();

listener()->OnChannelError();

« no previous file with comments | « ipc/ipc_channel_win.h ('k') | no next file » | no next file with comments »