AU: disable GPIOs in production; some structural changes
Since we are not making use of the GPIO funcionality in UE for the
moment, it's been advised that it should be disabled. This CL does just
that, plus a few small changes:
* Adds a "no-op" GPIO implementation, which simply returns a constant
value every time it's being asked whether test-mode was signaled (in
this case, we set it to return false).
* The GPIO handler is embedded in SystemState. This makes sense from
both the conceptual and usability standpoint. The SystemState object
can be parametrized to initialize either a real or a no-op GPIO
handler.
BUG=chromium-os:32263
TEST=passes unit tests; does not engage GPIO protocol on x86-alex
Change-Id: I8121647baa7611041073dcf305beddab57c0e49c
Reviewed-on: https://gerrit.chromium.org/gerrit/40633
Reviewed-by: Jay Srinivasan <jaysri@chromium.org>
Commit-Queue: Gilad Arnold <garnold@chromium.org>
Tested-by: Gilad Arnold <garnold@chromium.org>
diff --git a/gpio_handler.cc b/gpio_handler.cc
index 7145bbb..caf7679 100644
--- a/gpio_handler.cc
+++ b/gpio_handler.cc
@@ -578,4 +578,11 @@
return is_handshake_completed_;
}
+
+bool NoopGpioHandler::IsTestModeSignaled() {
+ LOG(INFO) << "GPIOs not engaged, defaulting to "
+ << (is_test_mode_ ? "test" : "normal") << " mode";
+ return is_test_mode_;
+}
+
} // namespace chromeos_update_engine
diff --git a/gpio_handler.h b/gpio_handler.h
index 0868cb5..1e25009 100644
--- a/gpio_handler.h
+++ b/gpio_handler.h
@@ -303,6 +303,26 @@
DISALLOW_COPY_AND_ASSIGN(StandardGpioHandler);
};
+
+// A "no-op" GPIO handler, initialized to return either test or normal mode
+// signal. This is useful for disabling the GPIO functionality in production
+// code.
+class NoopGpioHandler : public GpioHandler {
+ public:
+ // This constructor accepts a single argument, which is the value to be
+ // returned by repeated calls to IsTestModeSignaled().
+ NoopGpioHandler(bool is_test_mode) : is_test_mode_(is_test_mode) {}
+
+ // Returns the constant Boolean value handed to the constructor.
+ virtual bool IsTestModeSignaled();
+
+ private:
+ // Stores the constant value to return on subsequent test mode checks.
+ bool is_test_mode_;
+
+ DISALLOW_COPY_AND_ASSIGN(NoopGpioHandler);
+};
+
} // namespace chromeos_update_engine
#endif // CHROMEOS_PLATFORM_UPDATE_ENGINE_GPIO_HANDLER_H__
diff --git a/main.cc b/main.cc
index 833afb2..c956278 100644
--- a/main.cc
+++ b/main.cc
@@ -21,7 +21,6 @@
#include "update_engine/dbus_constants.h"
#include "update_engine/dbus_interface.h"
#include "update_engine/dbus_service.h"
-#include "update_engine/gpio_handler.h"
#include "update_engine/subprocess.h"
#include "update_engine/terminator.h"
#include "update_engine/update_attempter.h"
@@ -168,7 +167,9 @@
GMainLoop* loop = g_main_loop_new(g_main_context_default(), FALSE);
chromeos_update_engine::RealSystemState real_system_state;
- LOG_IF(ERROR, !real_system_state.Initialize())
+ // TODO(garnold) s/false/true/ once we decide to activate actual GPIO-based
+ // protocol for testing of MP-signed images (chromium-os:25400).
+ LOG_IF(ERROR, !real_system_state.Initialize(false))
<< "Failed to initialize system state.";
// Sets static members for the certificate checker.
@@ -178,20 +179,10 @@
chromeos_update_engine::CertificateChecker::set_openssl_wrapper(
&openssl_wrapper);
- // Initialize a GPIO handler. Defer GPIO discovery to ensure the udev has
- // ample time to export the devices. Also require that test mode is physically
- // queries at most once and the result cached, for a more consistent update
- // behavior.
- chromeos_update_engine::StandardUdevInterface udev_iface;
- chromeos_update_engine::EintrSafeFileDescriptor file_descriptor;
- chromeos_update_engine::StandardGpioHandler
- gpio_handler(&udev_iface, &file_descriptor, true, true);
-
// Create the update attempter:
chromeos_update_engine::ConcreteDbusGlib dbus;
chromeos_update_engine::UpdateAttempter update_attempter(&real_system_state,
- &dbus,
- &gpio_handler);
+ &dbus);
// Create the dbus service object:
dbus_g_object_type_install_info(UPDATE_ENGINE_TYPE_SERVICE,
@@ -204,7 +195,6 @@
// Schedule periodic update checks.
chromeos_update_engine::UpdateCheckScheduler scheduler(&update_attempter,
- &gpio_handler,
&real_system_state);
scheduler.Run();
diff --git a/mock_gpio_handler.h b/mock_gpio_handler.h
new file mode 100644
index 0000000..bee1cf6
--- /dev/null
+++ b/mock_gpio_handler.h
@@ -0,0 +1,20 @@
+// Copyright (c) 2013 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROMEOS_PLATFORM_UPDATE_ENGINE_MOCK_GPIO_HANDLER_H__
+#define CHROMEOS_PLATFORM_UPDATE_ENGINE_MOCK_GPIO_HANDLER_H__
+
+#include "gmock/gmock.h"
+#include "update_engine/gpio_handler.h"
+
+namespace chromeos_update_engine {
+
+class MockGpioHandler: public GpioHandler {
+ public:
+ MOCK_METHOD0(IsTestModeSignaled, bool());
+};
+
+} // namespace chromeos_update_engine
+
+#endif // CHROMEOS_PLATFORM_UPDATE_ENGINE_MOCK_GPIO_HANDLER_H__
diff --git a/mock_system_state.h b/mock_system_state.h
index bb6ee98..2b17ce4 100644
--- a/mock_system_state.h
+++ b/mock_system_state.h
@@ -10,6 +10,7 @@
#include <metrics/metrics_library_mock.h>
#include <policy/mock_device_policy.h>
+#include "update_engine/mock_gpio_handler.h"
#include "update_engine/mock_payload_state.h"
#include "update_engine/prefs_mock.h"
#include "update_engine/system_state.h"
@@ -20,49 +21,56 @@
// OOBE is completed even when there's no such marker file, etc.
class MockSystemState : public SystemState {
public:
- MockSystemState() : prefs_(&mock_prefs_) {
+ inline MockSystemState() : prefs_(&mock_prefs_) {
mock_payload_state_.Initialize(&mock_prefs_);
+ mock_gpio_handler_ = new testing::NiceMock<MockGpioHandler>();
}
- virtual ~MockSystemState() {}
+ inline virtual ~MockSystemState() {
+ delete mock_gpio_handler_;
+ }
MOCK_METHOD0(IsOOBEComplete, bool());
MOCK_METHOD1(set_device_policy, void(const policy::DevicePolicy*));
MOCK_CONST_METHOD0(device_policy, const policy::DevicePolicy*());
- virtual ConnectionManager* connection_manager() {
+ inline virtual ConnectionManager* connection_manager() {
return connection_manager_;
}
- virtual MetricsLibraryInterface* metrics_lib() {
+ inline virtual MetricsLibraryInterface* metrics_lib() {
return &mock_metrics_lib_;
}
- virtual PrefsInterface* prefs() {
+ inline virtual PrefsInterface* prefs() {
return prefs_;
}
- virtual PayloadStateInterface* payload_state() {
+ inline virtual PayloadStateInterface* payload_state() {
return &mock_payload_state_;
}
+ inline virtual GpioHandler* gpio_handler() const {
+ return mock_gpio_handler_;
+ }
+
// MockSystemState-specific public method.
- void set_connection_manager(ConnectionManager* connection_manager) {
+ inline void set_connection_manager(ConnectionManager* connection_manager) {
connection_manager_ = connection_manager;
}
- MetricsLibraryMock* mock_metrics_lib() {
+ inline MetricsLibraryMock* mock_metrics_lib() {
return &mock_metrics_lib_;
}
- void set_prefs(PrefsInterface* prefs) {
+ inline void set_prefs(PrefsInterface* prefs) {
prefs_ = prefs;
}
- testing::NiceMock<PrefsMock> *mock_prefs() {
+ inline testing::NiceMock<PrefsMock> *mock_prefs() {
return &mock_prefs_;
}
- MockPayloadState* mock_payload_state() {
+ inline MockPayloadState* mock_payload_state() {
return &mock_payload_state_;
}
@@ -71,6 +79,7 @@
MetricsLibraryMock mock_metrics_lib_;
testing::NiceMock<PrefsMock> mock_prefs_;
testing::NiceMock<MockPayloadState> mock_payload_state_;
+ testing::NiceMock<MockGpioHandler>* mock_gpio_handler_;
// These are pointers to objects which caller can override.
PrefsInterface* prefs_;
diff --git a/system_state.cc b/system_state.cc
index 78d5036..fc3ba57 100644
--- a/system_state.cc
+++ b/system_state.cc
@@ -15,7 +15,7 @@
: device_policy_(NULL),
connection_manager_(this) {}
-bool RealSystemState::Initialize() {
+bool RealSystemState::Initialize(bool enable_gpio) {
metrics_lib_.Init();
if (!prefs_.Init(FilePath(kPrefsDirectory))) {
@@ -26,6 +26,22 @@
if (!payload_state_.Initialize(&prefs_))
return false;
+ // Initialize the GPIO handler as instructed.
+ if (enable_gpio) {
+ // A real GPIO handler. Defer GPIO discovery to ensure the udev has ample
+ // time to export the devices. Also require that test mode is physically
+ // queried at most once and the result cached, for a more consistent update
+ // behavior.
+ udev_iface_.reset(new StandardUdevInterface());
+ file_descriptor_.reset(new EintrSafeFileDescriptor());
+ gpio_handler_.reset(new StandardGpioHandler(udev_iface_.get(),
+ file_descriptor_.get(),
+ true, true));
+ } else {
+ // A no-op GPIO handler, always indicating a non-test mode.
+ gpio_handler_.reset(new NoopGpioHandler(false));
+ }
+
// All is well. Initialization successful.
return true;
}
diff --git a/system_state.h b/system_state.h
index a04c434..772205c 100644
--- a/system_state.h
+++ b/system_state.h
@@ -10,6 +10,7 @@
#include <policy/libpolicy.h>
#include <update_engine/connection_manager.h>
+#include <update_engine/gpio_handler.h>
#include <update_engine/payload_state.h>
#include <update_engine/prefs.h>
@@ -46,6 +47,9 @@
// Gets the interface for the payload state object.
virtual PayloadStateInterface* payload_state() = 0;
+
+ // Returns a pointer to the GPIO handler.
+ virtual GpioHandler* gpio_handler() const = 0;
};
// A real implementation of the SystemStateInterface which is
@@ -83,9 +87,14 @@
return &payload_state_;
}
+ // Returns a pointer to the GPIO handler.
+ virtual inline GpioHandler* gpio_handler() const {
+ return gpio_handler_.get();
+ }
+
// Initializs this concrete object. Other methods should be invoked only
// if the object has been initialized successfully.
- bool Initialize();
+ bool Initialize(bool enable_gpio);
private:
// The latest device policy object from the policy provider.
@@ -104,6 +113,13 @@
// All state pertaining to payload state such as
// response, URL, backoff states.
PayloadState payload_state_;
+
+ // Pointer to a GPIO handler and other needed modules (note that the order of
+ // declaration significant for destruction, as the latter depends on the
+ // former).
+ scoped_ptr<UdevInterface> udev_iface_;
+ scoped_ptr<FileDescriptor> file_descriptor_;
+ scoped_ptr<GpioHandler> gpio_handler_;
};
} // namespace chromeos_update_engine
diff --git a/udev_interface.h b/udev_interface.h
index b9765a8..4b8fc17 100644
--- a/udev_interface.h
+++ b/udev_interface.h
@@ -22,6 +22,8 @@
// more appropriate to an object-oriented setting...
class UdevInterface {
public:
+ virtual ~UdevInterface() {}
+
// Interfaces for various udev closers. All of these are merely containers for
// a single pointer to some udev handle, which invoke the provided interface's
// unref method and nullify the handle upon destruction. It should suffice for
@@ -133,6 +135,8 @@
// Implementation of libudev interface using concrete udev calls.
class StandardUdevInterface : public UdevInterface {
public:
+ virtual ~StandardUdevInterface() {}
+
// Concrete udev API wrappers utilizing the standard libudev calls.
virtual const char* ListEntryGetName(struct udev_list_entry* list_entry) {
return udev_list_entry_get_name(list_entry);
diff --git a/update_attempter.cc b/update_attempter.cc
index a0ee020..3aa9749 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -108,8 +108,7 @@
}
UpdateAttempter::UpdateAttempter(SystemState* system_state,
- DbusGlibInterface* dbus_iface,
- GpioHandler* gpio_handler)
+ DbusGlibInterface* dbus_iface)
: processor_(new ActionProcessor()),
system_state_(system_state),
dbus_service_(NULL),
@@ -133,8 +132,7 @@
policy_provider_(NULL),
is_using_test_url_(false),
is_test_mode_(false),
- is_test_update_attempted_(false),
- gpio_handler_(gpio_handler) {
+ is_test_update_attempted_(false) {
prefs_ = system_state->prefs();
if (utils::FileExists(kUpdateCompletedMarker))
status_ = UPDATE_STATUS_UPDATED_NEED_REBOOT;
@@ -521,8 +519,8 @@
// Read GPIO signals and determine whether this is an automated test scenario.
// For safety, we only allow a test update to be performed once; subsequent
// update requests will be carried out normally.
- bool is_test_mode = (!is_test_update_attempted_ && gpio_handler_ &&
- gpio_handler_->IsTestModeSignaled());
+ bool is_test_mode = (!is_test_update_attempted_ &&
+ system_state_->gpio_handler()->IsTestModeSignaled());
if (is_test_mode) {
LOG(WARNING) << "this is a test mode update attempt!";
is_test_update_attempted_ = true;
diff --git a/update_attempter.h b/update_attempter.h
index df32536..ec0567b 100644
--- a/update_attempter.h
+++ b/update_attempter.h
@@ -18,7 +18,6 @@
#include "update_engine/action_processor.h"
#include "update_engine/chrome_browser_proxy_resolver.h"
#include "update_engine/download_action.h"
-#include "update_engine/gpio_handler.h"
#include "update_engine/omaha_request_params.h"
#include "update_engine/omaha_response_handler_action.h"
#include "update_engine/proxy_resolver.h"
@@ -61,8 +60,7 @@
static const int kMaxDeltaUpdateFailures;
UpdateAttempter(SystemState* system_state,
- DbusGlibInterface* dbus_iface,
- GpioHandler* gpio_handler);
+ DbusGlibInterface* dbus_iface);
virtual ~UpdateAttempter();
// Checks for update and, if a newer version is available, attempts to update
@@ -359,9 +357,6 @@
// A flag indicating whether a test update cycle was already attempted.
bool is_test_update_attempted_;
- // GPIO handler object.
- GpioHandler* gpio_handler_;
-
// The current scatter factor as found in the policy setting.
base::TimeDelta scatter_factor_;
diff --git a/update_attempter_mock.h b/update_attempter_mock.h
index 6432cd8..8b47c35 100644
--- a/update_attempter_mock.h
+++ b/update_attempter_mock.h
@@ -17,7 +17,7 @@
public:
explicit UpdateAttempterMock(MockSystemState* mock_system_state,
MockDbusGlib* dbus)
- : UpdateAttempter(mock_system_state, dbus, NULL) {}
+ : UpdateAttempter(mock_system_state, dbus) {}
MOCK_METHOD6(Update, void(const std::string& app_version,
const std::string& omaha_url,
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index aa6e94f..61d9010 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -40,7 +40,7 @@
public:
explicit UpdateAttempterUnderTest(MockSystemState* mock_system_state,
MockDbusGlib* dbus)
- : UpdateAttempter(mock_system_state, dbus, NULL) {}
+ : UpdateAttempter(mock_system_state, dbus) {}
};
class UpdateAttempterTest : public ::testing::Test {
@@ -143,7 +143,7 @@
OmahaResponse response;
response.poll_interval = 234;
action.SetOutputObject(response);
- UpdateCheckScheduler scheduler(&attempter_, NULL, &mock_system_state_);
+ UpdateCheckScheduler scheduler(&attempter_, &mock_system_state_);
attempter_.set_update_check_scheduler(&scheduler);
EXPECT_CALL(*prefs_, GetInt64(kPrefsDeltaUpdateFailures, _)).Times(0);
attempter_.ActionCompleted(NULL, &action, kActionCodeSuccess);
@@ -413,7 +413,7 @@
}
TEST_F(UpdateAttempterTest, PingOmahaTest) {
- UpdateCheckScheduler scheduler(&attempter_, NULL, &mock_system_state_);
+ UpdateCheckScheduler scheduler(&attempter_, &mock_system_state_);
scheduler.enabled_ = true;
EXPECT_FALSE(scheduler.scheduled_);
attempter_.set_update_check_scheduler(&scheduler);
diff --git a/update_check_scheduler.cc b/update_check_scheduler.cc
index ff597c6..85e734f 100644
--- a/update_check_scheduler.cc
+++ b/update_check_scheduler.cc
@@ -20,7 +20,6 @@
const int UpdateCheckScheduler::kTimeoutRegularFuzz = 10 * 60;
UpdateCheckScheduler::UpdateCheckScheduler(UpdateAttempter* update_attempter,
- GpioHandler* gpio_handler,
SystemState* system_state)
: update_attempter_(update_attempter),
enabled_(false),
@@ -28,7 +27,6 @@
last_interval_(0),
poll_interval_(0),
is_test_update_attempted_(false),
- gpio_handler_(gpio_handler),
system_state_(system_state) {}
UpdateCheckScheduler::~UpdateCheckScheduler() {}
@@ -91,10 +89,10 @@
me->scheduled_ = false;
bool is_test_mode = false;
+ GpioHandler* gpio_handler = me->system_state_->gpio_handler();
if (me->system_state_->IsOOBEComplete() ||
(is_test_mode = (!me->is_test_update_attempted_ &&
- me->gpio_handler_ &&
- me->gpio_handler_->IsTestModeSignaled()))) {
+ gpio_handler->IsTestModeSignaled()))) {
if (is_test_mode) {
LOG(WARNING)
<< "test mode signaled, allowing update check prior to OOBE complete";
diff --git a/update_check_scheduler.h b/update_check_scheduler.h
index 8cfcc46..9a5faa5 100644
--- a/update_check_scheduler.h
+++ b/update_check_scheduler.h
@@ -43,7 +43,6 @@
static const int kTimeoutMaxBackoffInterval;
UpdateCheckScheduler(UpdateAttempter* update_attempter,
- GpioHandler* gpio_handler,
SystemState* system_state);
virtual ~UpdateCheckScheduler();
@@ -132,9 +131,6 @@
// A flag indicating whether a test update cycle was already attempted.
bool is_test_update_attempted_;
- // GPIO handler object.
- GpioHandler* gpio_handler_;
-
// The external state of the system outside the update_engine process.
SystemState* system_state_;
diff --git a/update_check_scheduler_unittest.cc b/update_check_scheduler_unittest.cc
index ac31957..b751efa 100644
--- a/update_check_scheduler_unittest.cc
+++ b/update_check_scheduler_unittest.cc
@@ -34,7 +34,7 @@
public:
UpdateCheckSchedulerUnderTest(UpdateAttempter* update_attempter,
MockSystemState* mock_system_state)
- : UpdateCheckScheduler(update_attempter, NULL, mock_system_state),
+ : UpdateCheckScheduler(update_attempter, mock_system_state),
mock_system_state_(mock_system_state) {}
MOCK_METHOD2(GTimeoutAddSeconds, guint(guint seconds, GSourceFunc function));