Commit ee9dc849 authored by spang's avatar spang Committed by Commit bot

Revert of Move ScopedFileDescriptor to dbus/file_descriptor.h (patchset #1...

Revert of Move ScopedFileDescriptor to dbus/file_descriptor.h (patchset #1 id:1 of https://codereview.chromium.org/1090243003/)

Reason for revert:
Breaks shared_library build with broken exports:

./../dbus/dbus_export.h:20:65: error: ignoring attributes applied to class type 'scoped_ptr<dbus::FileDescriptor, dbus::FileDescriptor::Deleter>' outside of definition [-Werror=attributes]
 #define CHROME_DBUS_EXPORT __attribute__((visibility("default")))
                                                                 ^
../../dbus/file_descriptor.h:81:57: note: in expansion of macro 'CHROME_DBUS_EXPORT'
     scoped_ptr<FileDescriptor, FileDescriptor::Deleter> CHROME_DBUS_EXPORT;

Original issue's description:
> Move ScopedFileDescriptor to dbus/file_descriptor.h
>
> 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
>
> Committed: https://crrev.com/b44fbaabbe9f91e63ad8aa164609a6b31e0de57e
> Cr-Commit-Position: refs/heads/master@{#327089}

TBR=keybuk@chromium.org,reillyg@chromium.org,stevenjb@chromium.org,cmasone@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=chromium:481340

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

Cr-Commit-Position: refs/heads/master@{#327103}
parent bc743cc1
...@@ -50,7 +50,7 @@ const char* PortTypeToString(FirewallHole::PortType type) { ...@@ -50,7 +50,7 @@ const char* PortTypeToString(FirewallHole::PortType type) {
void PortReleased(FirewallHole::PortType type, void PortReleased(FirewallHole::PortType type,
uint16_t port, uint16_t port,
const std::string& interface, const std::string& interface,
dbus::ScopedFileDescriptor lifeline_fd, FirewallHole::ScopedFileDescriptor lifeline_fd,
bool success) { bool success) {
if (!success) { if (!success) {
LOG(WARNING) << "Failed to release firewall hole for " LOG(WARNING) << "Failed to release firewall hole for "
...@@ -61,13 +61,20 @@ void PortReleased(FirewallHole::PortType type, ...@@ -61,13 +61,20 @@ void PortReleased(FirewallHole::PortType type,
} // namespace } // namespace
void CHROMEOS_EXPORT FirewallHole::FileDescriptorDeleter::operator()(
dbus::FileDescriptor* fd) {
base::WorkerPool::PostTask(
FROM_HERE, base::Bind(&base::DeletePointer<dbus::FileDescriptor>, fd),
false);
}
// static // static
void FirewallHole::Open(PortType type, void FirewallHole::Open(PortType type,
uint16_t port, uint16_t port,
const std::string& interface, const std::string& interface,
const OpenCallback& callback) { const OpenCallback& callback) {
dbus::ScopedFileDescriptor lifeline_local(new dbus::FileDescriptor()); ScopedFileDescriptor lifeline_local(new dbus::FileDescriptor());
dbus::ScopedFileDescriptor lifeline_remote(new dbus::FileDescriptor()); ScopedFileDescriptor lifeline_remote(new dbus::FileDescriptor());
// This closure shares pointers with the one below. PostTaskAndReply // This closure shares pointers with the one below. PostTaskAndReply
// guarantees that it will always be deleted first. // guarantees that it will always be deleted first.
...@@ -102,8 +109,8 @@ FirewallHole::~FirewallHole() { ...@@ -102,8 +109,8 @@ FirewallHole::~FirewallHole() {
void FirewallHole::RequestPortAccess(PortType type, void FirewallHole::RequestPortAccess(PortType type,
uint16_t port, uint16_t port,
const std::string& interface, const std::string& interface,
dbus::ScopedFileDescriptor lifeline_local, ScopedFileDescriptor lifeline_local,
dbus::ScopedFileDescriptor lifeline_remote, ScopedFileDescriptor lifeline_remote,
const OpenCallback& callback) { const OpenCallback& callback) {
if (!lifeline_local->is_valid() || !lifeline_remote->is_valid()) { if (!lifeline_local->is_valid() || !lifeline_remote->is_valid()) {
callback.Run(nullptr); callback.Run(nullptr);
...@@ -133,7 +140,7 @@ void FirewallHole::RequestPortAccess(PortType type, ...@@ -133,7 +140,7 @@ void FirewallHole::RequestPortAccess(PortType type,
void FirewallHole::PortAccessGranted(PortType type, void FirewallHole::PortAccessGranted(PortType type,
uint16_t port, uint16_t port,
const std::string& interface, const std::string& interface,
dbus::ScopedFileDescriptor lifeline_fd, ScopedFileDescriptor lifeline_fd,
const FirewallHole::OpenCallback& callback, const FirewallHole::OpenCallback& callback,
bool success) { bool success) {
if (success) { if (success) {
...@@ -147,7 +154,7 @@ void FirewallHole::PortAccessGranted(PortType type, ...@@ -147,7 +154,7 @@ void FirewallHole::PortAccessGranted(PortType type,
FirewallHole::FirewallHole(PortType type, FirewallHole::FirewallHole(PortType type,
uint16_t port, uint16_t port,
const std::string& interface, const std::string& interface,
dbus::ScopedFileDescriptor lifeline_fd) ScopedFileDescriptor lifeline_fd)
: type_(type), : type_(type),
port_(port), port_(port),
interface_(interface), interface_(interface),
......
...@@ -11,7 +11,10 @@ ...@@ -11,7 +11,10 @@
#include "base/callback_forward.h" #include "base/callback_forward.h"
#include "base/memory/scoped_ptr.h" #include "base/memory/scoped_ptr.h"
#include "chromeos/chromeos_export.h" #include "chromeos/chromeos_export.h"
#include "dbus/file_descriptor.h"
namespace dbus {
class FileDescriptor;
}
namespace chromeos { namespace chromeos {
...@@ -26,6 +29,14 @@ class CHROMEOS_EXPORT FirewallHole { ...@@ -26,6 +29,14 @@ class CHROMEOS_EXPORT FirewallHole {
typedef base::Callback<void(scoped_ptr<FirewallHole>)> OpenCallback; 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 // 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 // interfaces if |interface| is ""). The hole will be closed when the object
// provided to the callback is destroyed. // provided to the callback is destroyed.
...@@ -40,28 +51,28 @@ class CHROMEOS_EXPORT FirewallHole { ...@@ -40,28 +51,28 @@ class CHROMEOS_EXPORT FirewallHole {
static void RequestPortAccess(PortType type, static void RequestPortAccess(PortType type,
uint16_t port, uint16_t port,
const std::string& interface, const std::string& interface,
dbus::ScopedFileDescriptor lifeline_local, ScopedFileDescriptor lifeline_local,
dbus::ScopedFileDescriptor lifeline_remote, ScopedFileDescriptor lifeline_remote,
const OpenCallback& callback); const OpenCallback& callback);
static void PortAccessGranted(PortType type, static void PortAccessGranted(PortType type,
uint16_t port, uint16_t port,
const std::string& interface, const std::string& interface,
dbus::ScopedFileDescriptor lifeline_fd, ScopedFileDescriptor lifeline_fd,
const FirewallHole::OpenCallback& callback, const FirewallHole::OpenCallback& callback,
bool success); bool success);
FirewallHole(PortType type, FirewallHole(PortType type,
uint16_t port, uint16_t port,
const std::string& interface, const std::string& interface,
dbus::ScopedFileDescriptor lifeline_fd); ScopedFileDescriptor lifeline_fd);
const PortType type_; const PortType type_;
const uint16_t port_; const uint16_t port_;
const std::string interface_; const std::string interface_;
// A file descriptor used by firewalld to track the lifetime of this process. // A file descriptor used by firewalld to track the lifetime of this process.
dbus::ScopedFileDescriptor lifeline_fd_; ScopedFileDescriptor lifeline_fd_;
}; };
} // namespace chromeos } // namespace chromeos
......
...@@ -2,21 +2,12 @@ ...@@ -2,21 +2,12 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
#include "base/bind.h"
#include "base/files/file.h" #include "base/files/file.h"
#include "base/location.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/threading/worker_pool.h"
#include "dbus/file_descriptor.h" #include "dbus/file_descriptor.h"
namespace dbus { 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() { FileDescriptor::~FileDescriptor() {
if (owner_) if (owner_)
base::File auto_closer(value_); base::File auto_closer(value_);
......
...@@ -6,7 +6,6 @@ ...@@ -6,7 +6,6 @@
#define DBUS_FILE_DESCRIPTOR_H_ #define DBUS_FILE_DESCRIPTOR_H_
#include "base/basictypes.h" #include "base/basictypes.h"
#include "base/memory/scoped_ptr.h"
#include "dbus/dbus_export.h" #include "dbus/dbus_export.h"
namespace dbus { namespace dbus {
...@@ -34,12 +33,6 @@ namespace dbus { ...@@ -34,12 +33,6 @@ namespace dbus {
// with i/o restrictions. // with i/o restrictions.
class CHROME_DBUS_EXPORT FileDescriptor { class CHROME_DBUS_EXPORT FileDescriptor {
public: 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 // Permits initialization without a value for passing to
// dbus::MessageReader::PopFileDescriptor to fill in and from int values. // dbus::MessageReader::PopFileDescriptor to fill in and from int values.
FileDescriptor() : value_(-1), owner_(false), valid_(false) {} FileDescriptor() : value_(-1), owner_(false), valid_(false) {}
...@@ -77,9 +70,6 @@ class CHROME_DBUS_EXPORT FileDescriptor { ...@@ -77,9 +70,6 @@ class CHROME_DBUS_EXPORT FileDescriptor {
DISALLOW_COPY_AND_ASSIGN(FileDescriptor); DISALLOW_COPY_AND_ASSIGN(FileDescriptor);
}; };
using ScopedFileDescriptor =
scoped_ptr<FileDescriptor, FileDescriptor::Deleter> CHROME_DBUS_EXPORT;
} // namespace dbus } // namespace dbus
#endif // DBUS_FILE_DESCRIPTOR_H_ #endif // DBUS_FILE_DESCRIPTOR_H_
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment