Skip to content

Commit

Permalink
Move ScopedFileDescriptor to dbus/file_descriptor.h
Browse files Browse the repository at this point in the history
ScopedFileDescriptor was initially introduced to facilitate the
management of a 'lifeline' FD used when asking permission_broker to
open a port in the device's firewall on CrOS.  It's actually a scoped
wrapper around DBus::FileDescriptor, not a platform file descriptor. I
would like to use it to facilitate some other similar functionality in
which I create a pipe and pass one end over DBus, so it seems like a
good idea to move ScopedFileDescriptor into the dbus library directly.

BUG=chromium:481340
TEST=FirewallHole unit tests still pass
STATUS=Fixed
R=keybuk, reillyg, stevenjb

Review URL: https://codereview.chromium.org/1090243003

Cr-Commit-Position: refs/heads/master@{#327089}
  • Loading branch information
cmasone authored and Commit bot committed Apr 27, 2015
1 parent d89b76b commit b44fbaa
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 31 deletions.
21 changes: 7 additions & 14 deletions chromeos/network/firewall_hole.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ const char* PortTypeToString(FirewallHole::PortType type) {
void PortReleased(FirewallHole::PortType type,
uint16_t port,
const std::string& interface,
FirewallHole::ScopedFileDescriptor lifeline_fd,
dbus::ScopedFileDescriptor lifeline_fd,
bool success) {
if (!success) {
LOG(WARNING) << "Failed to release firewall hole for "
Expand All @@ -61,20 +61,13 @@ void PortReleased(FirewallHole::PortType type,

} // namespace

void CHROMEOS_EXPORT FirewallHole::FileDescriptorDeleter::operator()(
dbus::FileDescriptor* fd) {
base::WorkerPool::PostTask(
FROM_HERE, base::Bind(&base::DeletePointer<dbus::FileDescriptor>, fd),
false);
}

// static
void FirewallHole::Open(PortType type,
uint16_t port,
const std::string& interface,
const OpenCallback& callback) {
ScopedFileDescriptor lifeline_local(new dbus::FileDescriptor());
ScopedFileDescriptor lifeline_remote(new dbus::FileDescriptor());
dbus::ScopedFileDescriptor lifeline_local(new dbus::FileDescriptor());
dbus::ScopedFileDescriptor lifeline_remote(new dbus::FileDescriptor());

// This closure shares pointers with the one below. PostTaskAndReply
// guarantees that it will always be deleted first.
Expand Down Expand Up @@ -109,8 +102,8 @@ FirewallHole::~FirewallHole() {
void FirewallHole::RequestPortAccess(PortType type,
uint16_t port,
const std::string& interface,
ScopedFileDescriptor lifeline_local,
ScopedFileDescriptor lifeline_remote,
dbus::ScopedFileDescriptor lifeline_local,
dbus::ScopedFileDescriptor lifeline_remote,
const OpenCallback& callback) {
if (!lifeline_local->is_valid() || !lifeline_remote->is_valid()) {
callback.Run(nullptr);
Expand Down Expand Up @@ -140,7 +133,7 @@ void FirewallHole::RequestPortAccess(PortType type,
void FirewallHole::PortAccessGranted(PortType type,
uint16_t port,
const std::string& interface,
ScopedFileDescriptor lifeline_fd,
dbus::ScopedFileDescriptor lifeline_fd,
const FirewallHole::OpenCallback& callback,
bool success) {
if (success) {
Expand All @@ -154,7 +147,7 @@ void FirewallHole::PortAccessGranted(PortType type,
FirewallHole::FirewallHole(PortType type,
uint16_t port,
const std::string& interface,
ScopedFileDescriptor lifeline_fd)
dbus::ScopedFileDescriptor lifeline_fd)
: type_(type),
port_(port),
interface_(interface),
Expand Down
23 changes: 6 additions & 17 deletions chromeos/network/firewall_hole.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,7 @@
#include "base/callback_forward.h"
#include "base/memory/scoped_ptr.h"
#include "chromeos/chromeos_export.h"

namespace dbus {
class FileDescriptor;
}
#include "dbus/file_descriptor.h"

namespace chromeos {

Expand All @@ -29,14 +26,6 @@ class CHROMEOS_EXPORT FirewallHole {

typedef base::Callback<void(scoped_ptr<FirewallHole>)> OpenCallback;

// This provides a simple way to pass around file descriptors since they must
// be closed on a thread that is allowed to perform I/O.
struct FileDescriptorDeleter {
void CHROMEOS_EXPORT operator()(dbus::FileDescriptor* fd);
};
typedef scoped_ptr<dbus::FileDescriptor, FileDescriptorDeleter>
ScopedFileDescriptor;

// Opens a port on the system firewall for the given network interface (or all
// interfaces if |interface| is ""). The hole will be closed when the object
// provided to the callback is destroyed.
Expand All @@ -51,28 +40,28 @@ class CHROMEOS_EXPORT FirewallHole {
static void RequestPortAccess(PortType type,
uint16_t port,
const std::string& interface,
ScopedFileDescriptor lifeline_local,
ScopedFileDescriptor lifeline_remote,
dbus::ScopedFileDescriptor lifeline_local,
dbus::ScopedFileDescriptor lifeline_remote,
const OpenCallback& callback);

static void PortAccessGranted(PortType type,
uint16_t port,
const std::string& interface,
ScopedFileDescriptor lifeline_fd,
dbus::ScopedFileDescriptor lifeline_fd,
const FirewallHole::OpenCallback& callback,
bool success);

FirewallHole(PortType type,
uint16_t port,
const std::string& interface,
ScopedFileDescriptor lifeline_fd);
dbus::ScopedFileDescriptor lifeline_fd);

const PortType type_;
const uint16_t port_;
const std::string interface_;

// A file descriptor used by firewalld to track the lifetime of this process.
ScopedFileDescriptor lifeline_fd_;
dbus::ScopedFileDescriptor lifeline_fd_;
};

} // namespace chromeos
Expand Down
9 changes: 9 additions & 0 deletions dbus/file_descriptor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,21 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "base/bind.h"
#include "base/files/file.h"
#include "base/location.h"
#include "base/logging.h"
#include "base/threading/worker_pool.h"
#include "dbus/file_descriptor.h"

namespace dbus {

void CHROME_DBUS_EXPORT FileDescriptor::Deleter::operator()(
FileDescriptor* fd) {
base::WorkerPool::PostTask(
FROM_HERE, base::Bind(&base::DeletePointer<FileDescriptor>, fd), false);
}

FileDescriptor::~FileDescriptor() {
if (owner_)
base::File auto_closer(value_);
Expand Down
10 changes: 10 additions & 0 deletions dbus/file_descriptor.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#define DBUS_FILE_DESCRIPTOR_H_

#include "base/basictypes.h"
#include "base/memory/scoped_ptr.h"
#include "dbus/dbus_export.h"

namespace dbus {
Expand Down Expand Up @@ -33,6 +34,12 @@ namespace dbus {
// with i/o restrictions.
class CHROME_DBUS_EXPORT FileDescriptor {
public:
// This provides a simple way to pass around file descriptors since they must
// be closed on a thread that is allowed to perform I/O.
struct Deleter {
void CHROME_DBUS_EXPORT operator()(FileDescriptor* fd);
};

// Permits initialization without a value for passing to
// dbus::MessageReader::PopFileDescriptor to fill in and from int values.
FileDescriptor() : value_(-1), owner_(false), valid_(false) {}
Expand Down Expand Up @@ -70,6 +77,9 @@ class CHROME_DBUS_EXPORT FileDescriptor {
DISALLOW_COPY_AND_ASSIGN(FileDescriptor);
};

using ScopedFileDescriptor =
scoped_ptr<FileDescriptor, FileDescriptor::Deleter> CHROME_DBUS_EXPORT;

} // namespace dbus

#endif // DBUS_FILE_DESCRIPTOR_H_

0 comments on commit b44fbaa

Please sign in to comment.