[forensics] Appease clang-tidy
- Pass std::shared_ptr by const-ref
- Remove std::move(trivially-copyable type)
- Initialize network watcher with const-ref rather than shared_ptr
- Match argument name in comment to function declaration
Change-Id: Ic9abe0d174ab669bf3d21896339e98b7b2a3a87c
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/514801
Fuchsia-Auto-Submit: Tamir Duberstein <tamird@google.com>
Reviewed-by: Francois Rousseau <frousseau@google.com>
Commit-Queue: Tamir Duberstein <tamird@google.com>
diff --git a/src/developer/forensics/crash_reports/crash_reporter.cc b/src/developer/forensics/crash_reports/crash_reporter.cc
index b14c901..92604f6 100644
--- a/src/developer/forensics/crash_reports/crash_reporter.cc
+++ b/src/developer/forensics/crash_reports/crash_reporter.cc
@@ -93,8 +93,8 @@
} // namespace
std::unique_ptr<CrashReporter> CrashReporter::TryCreate(
- async_dispatcher_t* dispatcher, std::shared_ptr<sys::ServiceDirectory> services,
- timekeeper::Clock* clock, std::shared_ptr<InfoContext> info_context, Config config,
+ async_dispatcher_t* dispatcher, const std::shared_ptr<sys::ServiceDirectory>& services,
+ timekeeper::Clock* clock, const std::shared_ptr<InfoContext>& info_context, Config config,
AnnotationMap default_annotations, CrashRegister* crash_register) {
std::unique_ptr<SnapshotManager> snapshot_manager = std::make_unique<SnapshotManager>(
dispatcher, services, clock, kSnapshotSharedRequestWindow, kGarbageCollectedSnapshotsPath,
@@ -105,19 +105,17 @@
auto crash_server =
std::make_unique<CrashServer>(services, kCrashServerUrl, snapshot_manager.get(), tags.get());
- return std::unique_ptr<CrashReporter>(
- new CrashReporter(dispatcher, std::move(services), clock, std::move(info_context),
- std::move(config), std::move(default_annotations), crash_register,
- std::move(tags), std::move(snapshot_manager), std::move(crash_server)));
+ return std::make_unique<CrashReporter>(
+ dispatcher, std::move(services), clock, std::move(info_context), config,
+ std::move(default_annotations), crash_register, std::move(tags), std::move(snapshot_manager),
+ std::move(crash_server));
}
-CrashReporter::CrashReporter(async_dispatcher_t* dispatcher,
- std::shared_ptr<sys::ServiceDirectory> services,
- timekeeper::Clock* clock, std::shared_ptr<InfoContext> info_context,
- Config config, AnnotationMap default_annotations,
- CrashRegister* crash_register, std::unique_ptr<LogTags> tags,
- std::unique_ptr<SnapshotManager> snapshot_manager,
- std::unique_ptr<CrashServer> crash_server)
+CrashReporter::CrashReporter(
+ async_dispatcher_t* dispatcher, const std::shared_ptr<sys::ServiceDirectory>& services,
+ timekeeper::Clock* clock, const std::shared_ptr<InfoContext>& info_context, Config config,
+ AnnotationMap default_annotations, CrashRegister* crash_register, std::unique_ptr<LogTags> tags,
+ std::unique_ptr<SnapshotManager> snapshot_manager, std::unique_ptr<CrashServer> crash_server)
: dispatcher_(dispatcher),
executor_(dispatcher),
services_(services),
@@ -131,7 +129,7 @@
snapshot_manager_.get()),
product_quotas_(dispatcher_, config.daily_per_product_quota),
info_(info_context),
- network_watcher_(dispatcher_, services_),
+ network_watcher_(dispatcher_, *services_),
reporting_policy_watcher_(MakeReportingPolicyWatcher(dispatcher_, services, config)),
device_id_provider_ptr_(dispatcher_, services_) {
FX_CHECK(dispatcher_);
diff --git a/src/developer/forensics/crash_reports/crash_reporter.h b/src/developer/forensics/crash_reports/crash_reporter.h
index 3981b79..9fabaca 100644
--- a/src/developer/forensics/crash_reports/crash_reporter.h
+++ b/src/developer/forensics/crash_reports/crash_reporter.h
@@ -39,16 +39,15 @@
// Static factory method.
//
// Returns nullptr if the crash reporter cannot be instantiated.
- static std::unique_ptr<CrashReporter> TryCreate(async_dispatcher_t* dispatcher,
- std::shared_ptr<sys::ServiceDirectory> services,
- timekeeper::Clock* clock,
- std::shared_ptr<InfoContext> info_context,
- Config config, AnnotationMap default_annotations,
- CrashRegister* crash_register);
+ static std::unique_ptr<CrashReporter> TryCreate(
+ async_dispatcher_t* dispatcher, const std::shared_ptr<sys::ServiceDirectory>& services,
+ timekeeper::Clock* clock, const std::shared_ptr<InfoContext>& info_context, Config config,
+ AnnotationMap default_annotations, CrashRegister* crash_register);
// For testing purposes and injecting a stub CrashServer.
- CrashReporter(async_dispatcher_t* dispatcher, std::shared_ptr<sys::ServiceDirectory> services,
- timekeeper::Clock* clock, std::shared_ptr<InfoContext> info_context, Config config,
+ CrashReporter(async_dispatcher_t* dispatcher,
+ const std::shared_ptr<sys::ServiceDirectory>& services, timekeeper::Clock* clock,
+ const std::shared_ptr<InfoContext>& info_context, Config config,
AnnotationMap default_annotations, CrashRegister* crash_register,
std::unique_ptr<LogTags> tags, std::unique_ptr<SnapshotManager> snapshot_manager,
std::unique_ptr<CrashServer> crash_server);
diff --git a/src/developer/forensics/crash_reports/network_watcher.cc b/src/developer/forensics/crash_reports/network_watcher.cc
index f5f9053..2a310de 100644
--- a/src/developer/forensics/crash_reports/network_watcher.cc
+++ b/src/developer/forensics/crash_reports/network_watcher.cc
@@ -10,9 +10,9 @@
namespace forensics::crash_reports {
NetworkWatcher::NetworkWatcher(async_dispatcher_t* dispatcher,
- std::shared_ptr<sys::ServiceDirectory> services) {
+ const sys::ServiceDirectory& services) {
fuchsia::net::interfaces::StatePtr state;
- zx_status_t status = services->Connect(state.NewRequest(dispatcher));
+ zx_status_t status = services.Connect(state.NewRequest(dispatcher));
if (status != ZX_OK) {
FX_PLOGS(ERROR, status) << "Failed to connect to " << fuchsia::net::interfaces::State::Name_
<< "; cannot watch for network reachability status";
diff --git a/src/developer/forensics/crash_reports/network_watcher.h b/src/developer/forensics/crash_reports/network_watcher.h
index d8c5f9d..afaacf8 100644
--- a/src/developer/forensics/crash_reports/network_watcher.h
+++ b/src/developer/forensics/crash_reports/network_watcher.h
@@ -19,7 +19,7 @@
// this occurs.
class NetworkWatcher {
public:
- NetworkWatcher(async_dispatcher_t* dispatcher, std::shared_ptr<sys::ServiceDirectory> services);
+ NetworkWatcher(async_dispatcher_t* dispatcher, const sys::ServiceDirectory& services);
// Register a callback to be called when the network reachability status changes.
void Register(::fit::function<void(bool)> on_reachable);
diff --git a/src/developer/forensics/crash_reports/tests/queue_unittest.cc b/src/developer/forensics/crash_reports/tests/queue_unittest.cc
index a7d51cb..267a811 100644
--- a/src/developer/forensics/crash_reports/tests/queue_unittest.cc
+++ b/src/developer/forensics/crash_reports/tests/queue_unittest.cc
@@ -119,7 +119,7 @@
class QueueTest : public UnitTestFixture {
public:
- QueueTest() : network_watcher_(dispatcher(), services()) {}
+ QueueTest() : network_watcher_(dispatcher(), *services()){};
void SetUp() override {
info_context_ =
@@ -141,7 +141,7 @@
InjectServiceProvider(network_reachability_provider_.get());
}
- void SetUpQueue(std::vector<CrashServer::UploadStatus> upload_attempt_results =
+ void SetUpQueue(const std::vector<CrashServer::UploadStatus>& upload_attempt_results =
std::vector<CrashServer::UploadStatus>{}) {
report_id_ = 1;
snapshot_manager_ = std::make_unique<SnapshotManager>(
@@ -338,7 +338,7 @@
});
reporting_policy_watcher_.Set(ReportingPolicy::kUndecided);
- auto report_id = AddNewReport(/*is_hourly_upload=*/false);
+ auto report_id = AddNewReport(/*is_hourly_report=*/false);
ASSERT_TRUE(report_id);
EXPECT_TRUE(queue_->Contains(*report_id));
@@ -358,11 +358,11 @@
cobalt::Event(cobalt::UploadAttemptState::kUploaded, 1u),
}));
- report_id = AddNewReport(/*is_hourly_upload=*/false);
+ report_id = AddNewReport(/*is_hourly_report=*/false);
ASSERT_TRUE(report_id);
EXPECT_TRUE(queue_->Contains(*report_id));
- const auto hourly_report_id = AddNewReport(/*is_hourly_upload=*/true);
+ const auto hourly_report_id = AddNewReport(/*is_hourly_report=*/true);
ASSERT_TRUE(hourly_report_id);
EXPECT_TRUE(queue_->Contains(*hourly_report_id));
