Skip to content

Commit 3f105ec

Browse files
committed
Fix dive_client_cli lint error.
- Remove unused includes. - Fix almost all clang-tidy issues. - Remaining cppcoreguidelines-pro-bounds-array-to-pointer-decay need to be fixed in constants.h - Rewrite array initialization.
1 parent de8db66 commit 3f105ec

File tree

2 files changed

+70
-64
lines changed

2 files changed

+70
-64
lines changed

capture_service/dive_client_cli.cc

Lines changed: 70 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -15,23 +15,18 @@ limitations under the License.
1515
*/
1616

1717
#include <filesystem>
18-
#include <future>
1918
#include <iostream>
2019
#include <ostream>
2120
#include <string>
22-
#include <system_error>
2321
#include <thread>
2422

25-
#include "absl/base/no_destructor.h"
2623
#include "absl/flags/flag.h"
27-
#include "absl/flags/internal/flag.h"
2824
#include "absl/flags/parse.h"
2925
#include "absl/flags/usage.h"
3026
#include "absl/flags/usage_config.h"
3127
#include "absl/strings/str_cat.h"
3228
#include "absl/strings/str_format.h"
3329
#include "absl/strings/str_split.h"
34-
3530
#include "android_application.h"
3631
#include "command_utils.h"
3732
#include "constants.h"
@@ -57,8 +52,10 @@ struct GlobalOptions
5752

5853
struct CommandContext
5954
{
55+
// NOLINTBEGIN(cppcoreguidelines-avoid-const-or-ref-data-members)
6056
Dive::DeviceManager& mgr;
6157
const GlobalOptions& options;
58+
// NOLINTEND(cppcoreguidelines-avoid-const-or-ref-data-members)
6259
};
6360

6461
enum class Command
@@ -98,14 +95,15 @@ absl::Status CmdGfxrCapture(const CommandContext& ctx);
9895
absl::Status CmdGfxrReplay(const CommandContext& ctx);
9996
absl::Status CmdCleanup(const CommandContext& ctx);
10097

101-
absl::Status ValidateAlwaysOk(const GlobalOptions&)
98+
absl::Status ValidateAlwaysOk(const GlobalOptions& /*options*/)
10299
{
103100
return absl::OkStatus();
104101
}
105102

106-
absl::Status ValidateCleanup(const GlobalOptions& o)
103+
absl::Status ValidateCleanup(const GlobalOptions& options)
107104
{
108-
return o.package.empty() ? absl::InvalidArgumentError("Missing --package") : absl::OkStatus();
105+
return options.package.empty() ? absl::InvalidArgumentError("Missing --package") :
106+
absl::OkStatus();
109107
}
110108

111109
// Helper to validate options common to run/capture commands.
@@ -139,7 +137,7 @@ absl::Status ValidateRunOptions(const GlobalOptions& options)
139137
"arm64-v8a", "arm64-v8", "armeabi-v7a", "x86", "x86_64"
140138
};
141139

142-
if (valid_archs.find(options.device_architecture) == valid_archs.end())
140+
if (!valid_archs.contains(options.device_architecture))
143141
{
144142
return absl::InvalidArgumentError(
145143
absl::StrCat("Invalid --device_architecture '",
@@ -167,43 +165,43 @@ absl::Status ValidateGfxrReplayOptions(const GlobalOptions& options)
167165
return absl::OkStatus();
168166
}
169167

170-
constexpr std::array<CommandDef, 7> kCommandDefs = { {
171-
{ Command::kListDevice,
172-
"list_device",
173-
"List connected Android devices.",
174-
ValidateAlwaysOk,
175-
CmdListDevice },
176-
{ Command::kListPackage,
177-
"list_package",
178-
"List installable packages on the selected device.",
179-
ValidateAlwaysOk,
180-
CmdListPackage },
181-
{ Command::kRunPackage,
182-
"run",
183-
"Run an app for manual testing or external capture.",
184-
ValidateRunOptions,
185-
CmdRunPackage },
186-
{ Command::kPm4Capture,
187-
"pm4_capture",
188-
"Run an app and trigger a PM4 capture after a delay.",
189-
ValidateRunOptions,
190-
CmdPm4Capture },
191-
{ Command::kGfxrCapture,
192-
"gfxr_capture",
193-
"Run an app and enable GFXR capture via key-press.",
194-
ValidateRunOptions,
195-
CmdGfxrCapture },
196-
{ Command::kGfxrReplay,
197-
"gfxr_replay",
198-
"Deploy and run a GFXR replay.",
199-
ValidateGfxrReplayOptions,
200-
CmdGfxrReplay },
201-
{ Command::kCleanup,
202-
"cleanup",
203-
"Clean up app-specific settings on the device.",
204-
ValidateCleanup,
205-
CmdCleanup },
206-
} };
168+
constexpr std::array kCommandDefs = {
169+
CommandDef{ .cmd = Command::kListDevice,
170+
.name = "list_device",
171+
.description = "List connected Android devices.",
172+
.validator = ValidateAlwaysOk,
173+
.executor = CmdListDevice },
174+
CommandDef{ .cmd = Command::kListPackage,
175+
.name = "list_package",
176+
.description = "List installable packages on the selected device.",
177+
.validator = ValidateAlwaysOk,
178+
.executor = CmdListPackage },
179+
CommandDef{ .cmd = Command::kRunPackage,
180+
.name = "run",
181+
.description = "Run an app for manual testing or external capture.",
182+
.validator = ValidateRunOptions,
183+
.executor = CmdRunPackage },
184+
CommandDef{ .cmd = Command::kPm4Capture,
185+
.name = "pm4_capture",
186+
.description = "Run an app and trigger a PM4 capture after a delay.",
187+
.validator = ValidateRunOptions,
188+
.executor = CmdPm4Capture },
189+
CommandDef{ .cmd = Command::kGfxrCapture,
190+
.name = "gfxr_capture",
191+
.description = "Run an app and enable GFXR capture via key-press.",
192+
.validator = ValidateRunOptions,
193+
.executor = CmdGfxrCapture },
194+
CommandDef{ .cmd = Command::kGfxrReplay,
195+
.name = "gfxr_replay",
196+
.description = "Deploy and run a GFXR replay.",
197+
.validator = ValidateGfxrReplayOptions,
198+
.executor = CmdGfxrReplay },
199+
CommandDef{ .cmd = Command::kCleanup,
200+
.name = "cleanup",
201+
.description = "Clean up app-specific settings on the device.",
202+
.validator = ValidateCleanup,
203+
.executor = CmdCleanup },
204+
};
207205

208206
// Generates the help string dynamically for ABSL_FLAG.
209207
std::string GenerateCommandFlagHelp()
@@ -261,9 +259,9 @@ absl::StatusOr<Dive::AndroidDevice*> GetTargetDevice(Dive::DeviceManager& mgr,
261259
{
262260
std::string
263261
msg = "Multiple devices connected. Specify --device [serial].\nAvailables:\n";
264-
for (const auto& d : list)
262+
for (const auto& device_info : list)
265263
{
266-
msg.append("\t" + d.GetDisplayName() + "\n");
264+
msg.append("\t" + device_info.GetDisplayName() + "\n");
267265
}
268266
return absl::InvalidArgumentError(msg);
269267
}
@@ -272,14 +270,14 @@ absl::StatusOr<Dive::AndroidDevice*> GetTargetDevice(Dive::DeviceManager& mgr,
272270
{
273271
bool found = false;
274272
std::string msg;
275-
for (const auto& d : list)
273+
for (const auto& device_info : list)
276274
{
277-
if (d.m_serial == target_serial)
275+
if (device_info.m_serial == target_serial)
278276
{
279277
found = true;
280278
break;
281279
}
282-
msg.append("\t" + d.GetDisplayName() + "\n");
280+
msg.append("\t" + device_info.GetDisplayName() + "\n");
283281
}
284282
if (!found)
285283
{
@@ -387,10 +385,14 @@ absl::Status TriggerPm4Capture(Dive::DeviceManager& mgr, const std::string& down
387385
return absl::UnavailableError("Connection failed: " + std::string(status.message()));
388386
}
389387

390-
absl::StatusOr<std::string> capture_file_path = client.StartPm4Capture();
391-
if (!capture_file_path.ok())
388+
std::string capture_file_path;
392389
{
393-
return capture_file_path.status();
390+
auto res = client.StartPm4Capture();
391+
if (!res.ok())
392+
{
393+
return res.status();
394+
}
395+
capture_file_path = *res;
394396
}
395397

396398
std::filesystem::path target_download_dir(download_dir);
@@ -400,10 +402,10 @@ absl::Status TriggerPm4Capture(Dive::DeviceManager& mgr, const std::string& down
400402
target_download_dir.string());
401403
}
402404

403-
std::filesystem::path p(*capture_file_path);
404-
std::string download_file_path = (target_download_dir / p.filename()).string();
405+
auto capture_file_name = std::filesystem::path(capture_file_path).filename();
406+
std::string download_file_path = (target_download_dir / capture_file_name).string();
405407

406-
status = client.DownloadFileFromServer(*capture_file_path, download_file_path);
408+
status = client.DownloadFileFromServer(capture_file_path, download_file_path);
407409
if (!status.ok())
408410
{
409411
return status;
@@ -416,9 +418,10 @@ absl::Status TriggerPm4Capture(Dive::DeviceManager& mgr, const std::string& down
416418
// Checks if the capture directory on the device is currently done.
417419
absl::Status IsCaptureFinished(Dive::DeviceManager& mgr, const std::string& gfxr_capture_directory)
418420
{
419-
std::string on_device_capture_directory = absl::StrCat(Dive::kDeviceCapturePath,
421+
std::string on_device_capture_directory = absl::StrCat(Dive::kDeviceCapturePath,
420422
"/",
421423
gfxr_capture_directory);
424+
422425
std::string command = "shell lsof " + on_device_capture_directory;
423426
absl::StatusOr<std::string> output = mgr.GetDevice()->Adb().RunAndGetResult(command);
424427

@@ -427,10 +430,10 @@ absl::Status IsCaptureFinished(Dive::DeviceManager& mgr, const std::string& gfxr
427430
std::cout << "Error checking directory: " << output.status().message() << std::endl;
428431
}
429432

430-
std::stringstream ss(output->c_str());
433+
std::stringstream sst(output->c_str());
431434
std::string line;
432435
int line_count = 0;
433-
while (std::getline(ss, line))
436+
while (std::getline(sst, line))
434437
{
435438
line_count++;
436439
}
@@ -440,6 +443,7 @@ absl::Status IsCaptureFinished(Dive::DeviceManager& mgr, const std::string& gfxr
440443
}
441444

442445
// Renames the screenshot file locally to match the GFXR capture file name.
446+
// NOLINTNEXTLINE(bugprone-easily-swappable-parameters)
443447
absl::Status RenameScreenshotFile(const std::filesystem::path& full_target_download_dir,
444448
const std::filesystem::path& gfxr_capture_file_name)
445449
{
@@ -518,9 +522,7 @@ absl::Status RetrieveGfxrCapture(Dive::DeviceManager& mgr, const GlobalOptions&
518522
std::string(output.status().message()));
519523
}
520524

521-
std::vector<std::string> file_list = absl::StrSplit(std::string(output->data()),
522-
'\n',
523-
absl::SkipEmpty());
525+
std::vector<std::string> file_list = absl::StrSplit(*output, '\n', absl::SkipEmpty());
524526

525527
if (file_list.empty())
526528
{
@@ -570,6 +572,7 @@ absl::Status RetrieveGfxrCapture(Dive::DeviceManager& mgr, const GlobalOptions&
570572
}
571573

572574
// Triggers a GFXR capture on the device, allowing for multiple captures and screenshot.
575+
// NOLINTNEXTLINE(readability-function-cognitive-complexity)
573576
absl::Status TriggerGfxrCapture(Dive::DeviceManager& mgr, const GlobalOptions& options)
574577
{
575578
std::cout
@@ -786,11 +789,15 @@ bool AbslParseFlag(absl::string_view text, Command* command, std::string* error)
786789
std::string AbslUnparseFlag(Command command)
787790
{
788791
if (command == Command::kNone)
792+
{
789793
return "";
794+
}
790795
for (const auto& def : kCommandDefs)
791796
{
792797
if (def.cmd == command)
798+
{
793799
return def.name;
800+
}
794801
}
795802
return "unknown";
796803
}

src/dive/common/app_types.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ limitations under the License.
1616

1717
#pragma once
1818

19-
#include <algorithm>
2019
#include <array>
2120
#include <string_view>
2221

0 commit comments

Comments
 (0)