Skip to content

Commit

Permalink
Fix GetDiskUsage and add flag parsing overload
Browse files Browse the repository at this point in the history
`ParseInt` was failing on the output of `du` because it is in a format
like: "<total num> <filepath>".  This version splits that and takes the first portion.  We should update
this later to not rely on `du`, b/385384942 should encompass that.

Returning to the `atoi` implementation was not a good idea for the bytes
version, because it was still susceptible to overflow issues.

Changed the return value to be a `size_t` instead of a
signed number, because the sizes will not (should not?) ever be
negative.

Added a `GflagsCompatFlag` overload for reading in flags that can be
compared to the output of this helper.

The `GetDiskUsageGigabytes` will still be used in the upcomign `cvd
cache`, while the `*Bytes` version will be used when we sync these
changes back to AOSP.

Test: use the `cvd_cache` branch and run `cvd cache size`
  • Loading branch information
cjreynol committed Dec 23, 2024
1 parent ab11291 commit 66924f9
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 20 deletions.
44 changes: 26 additions & 18 deletions base/cvd/cuttlefish/common/libs/utils/files.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
#include <array>
#include <cerrno>
#include <chrono>
#include <cstddef>
#include <cstdio>
#include <cstdlib>
#include <cstring>
Expand Down Expand Up @@ -76,6 +77,8 @@

namespace cuttlefish {

static constexpr char kWhitespaceCharacters[] = " \n\t\r\v\f";

bool FileExists(const std::string& path, bool follow_symlinks) {
struct stat st {};
return (follow_symlinks ? stat : lstat)(path.c_str(), &st) == 0;
Expand Down Expand Up @@ -613,35 +616,40 @@ bool FileIsSocket(const std::string& path) {
}

// return unit determined by the `--block-size` argument
Result<long> GetDiskUsage(const std::string& path,
const std::string& size_arg) {
Result<std::size_t> GetDiskUsage(const std::string& path,
const std::string& size_arg) {
Command du_cmd("du");
du_cmd.AddParameter("-s"); // summarize, only output total
du_cmd.AddParameter(
"--apparent-size"); // apparent size rather than device usage
du_cmd.AddParameter("--block-size=" + size_arg);
du_cmd.AddParameter(path);
SharedFD read_fd;
SharedFD write_fd;
SharedFD::Pipe(&read_fd, &write_fd);
du_cmd.RedirectStdIO(Subprocess::StdIOChannel::kStdOut, write_fd);
auto subprocess = du_cmd.Start();
std::array<char, 1024> text_output{};
const auto bytes_read = read_fd->Read(text_output.data(), text_output.size());
CF_EXPECTF(bytes_read > 0, "Failed to read from pipe: {}", strerror(errno));
std::move(subprocess).Wait();
int result;
CF_EXPECTF(android::base::ParseInt(text_output.data(), &result),
"Failure parsing \"{}\" to integer.", text_output.data());

std::string out;
std::string err;
int return_code = RunWithManagedStdio(std::move(du_cmd), nullptr, &out, &err);
CF_EXPECTF(return_code == 0, "Failed to run `du` command. stderr: {}", err);
CF_EXPECTF(!out.empty(), "No output read from `du` command. stderr: {}", err);
std::vector<std::string> split_out =
android::base::Tokenize(out, kWhitespaceCharacters);
CF_EXPECTF(!split_out.empty(),
"No valid output read from `du` command in \"{}\"", out);
std::string total = split_out.front();

std::size_t result;
CF_EXPECTF(android::base::ParseUint(total, &result),
"Failure parsing \"{}\" to integer.", total);
return result;
}

Result<long> GetDiskUsageBytes(const std::string& path) {
return GetDiskUsage(path, "1");
Result<std::size_t> GetDiskUsageBytes(const std::string& path) {
return CF_EXPECTF(GetDiskUsage(path, "1"),
"Unable to determine disk usage of file \"{}\"", path);
}

Result<long> GetDiskUsageGigabytes(const std::string& path) {
return GetDiskUsage(path, "1G");
Result<std::size_t> GetDiskUsageGigabytes(const std::string& path) {
return CF_EXPECTF(GetDiskUsage(path, "1G"),
"Unable to determine disk usage of file \"{}\"", path);
}

/**
Expand Down
5 changes: 3 additions & 2 deletions base/cvd/cuttlefish/common/libs/utils/files.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <sys/types.h>

#include <chrono>
#include <cstddef>
#include <optional>
#include <string>
#include <vector>
Expand Down Expand Up @@ -78,8 +79,8 @@ std::string cpp_basename(const std::string& str);
bool FileIsSocket(const std::string& path);
// Get disk usage of a path. If this path is a directory, disk usage will
// account for all files under this folder(recursively).
Result<long> GetDiskUsageBytes(const std::string& path);
Result<long> GetDiskUsageGigabytes(const std::string& path);
Result<std::size_t> GetDiskUsageBytes(const std::string& path);
Result<std::size_t> GetDiskUsageGigabytes(const std::string& path);

// acloud related API
std::string FindImage(const std::string& search_path,
Expand Down
21 changes: 21 additions & 0 deletions base/cvd/cuttlefish/common/libs/utils/flag_parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

#include <algorithm>
#include <cerrno>
#include <cstddef>
#include <cstdlib>
#include <cstring>
#include <functional>
Expand All @@ -34,6 +35,7 @@

#include <android-base/logging.h>
#include <android-base/parsebool.h>
#include <android-base/parseint.h>
#include <android-base/scopeguard.h>
#include <android-base/strings.h>
#include <fmt/format.h>
Expand Down Expand Up @@ -566,6 +568,25 @@ Flag GflagsCompatFlag(const std::string& name, int32_t& value) {
return GflagsCompatNumericFlagGeneric(name, value);
}

template <typename T>
static Flag GflagsCompatUnsignedNumericFlagGeneric(const std::string& name,
T& value) {
return GflagsCompatFlag(name)
.Getter([&value]() { return std::to_string(value); })
.Setter([&value](const FlagMatch& match) -> Result<void> {
T result;
CF_EXPECTF(android::base::ParseUint<T>(match.value, &result),
"Failed to parse \"{}\" as an unsigned integer",
match.value);
value = result;
return {};
});
}

Flag GflagsCompatFlag(const std::string& name, std::size_t& value) {
return GflagsCompatUnsignedNumericFlagGeneric(name, value);
}

Flag GflagsCompatFlag(const std::string& name, bool& value) {
return GflagsCompatBoolFlagBase(name)
.Getter([&value]() { return fmt::format("{}", value); })
Expand Down
1 change: 1 addition & 0 deletions base/cvd/cuttlefish/common/libs/utils/flag_parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ Flag UnexpectedArgumentGuard();
Flag GflagsCompatFlag(const std::string& name);
Flag GflagsCompatFlag(const std::string& name, std::string& value);
Flag GflagsCompatFlag(const std::string& name, std::int32_t& value);
Flag GflagsCompatFlag(const std::string& name, std::size_t& value);
Flag GflagsCompatFlag(const std::string& name, bool& value);
Flag GflagsCompatFlag(const std::string& name, std::vector<std::string>& value);
Flag GflagsCompatFlag(const std::string& name, std::vector<bool>& value,
Expand Down

0 comments on commit 66924f9

Please sign in to comment.