Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 11a3aeea authored by Tom Cherry's avatar Tom Cherry
Browse files

init: introduce Result<T> for return values and error handling

init tries to propagate error information up to build context before
logging errors.  This is a good thing, however too often init has the
overly verbose paradigm for error handling, below:

bool CalculateResult(const T& input, U* output, std::string* err)

bool CalculateAndUseResult(const T& input, std::string* err) {
  U output;
  std::string calculate_result_err;
  if (!CalculateResult(input, &output, &calculate_result_err)) {
    *err = "CalculateResult " + input + " failed: " +
      calculate_result_err;
      return false;
  }
  UseResult(output);
  return true;
}

Even more common are functions that return only true/false but also
require passing a std::string* err in order to see the error message.

This change introduces a Result<T> that is use to either hold a
successful return value of type T or to hold an error message as a
std::string.  If the functional only returns success or a failure with
an error message, Result<Success> may be used.  The classes Error and
ErrnoError are used to indicate a failed Result<T>.

A successful Result<T> is constructed implicitly from any type that
can be implicitly converted to T or from the constructor arguments for
T.  This allows you to return a type T directly from a function that
returns Result<T>.

Error and ErrnoError are used to construct a Result<T> has
failed. Each of these classes take an ostream as an input and are
implicitly cast to a Result<T> containing that failure.  ErrnoError()
additionally appends ": " + strerror(errno) to the end of  the failure
string to aid in interacting with C APIs.

The end result is that the above code snippet is turned into the much
clearer example below:

Result<U> CalculateResult(const T& input);

Result<Success> CalculateAndUseResult(const T& input) {
  auto output = CalculateResult(input);
  if (!output) {
    return Error() << "CalculateResult " << input << " failed: "
                   << output.error();
  }
  UseResult(*output);
  return Success();
}

This change also makes this conversion for some of the util.cpp
functions that used the old paradigm.

Test: boot bullhead, init unit tests
Merged-In: I1e7d3a8820a79362245041251057fbeed2f7979b
Change-Id: I1e7d3a8820a79362245041251057fbeed2f7979b
parent d467db9b
Loading
Loading
Loading
Loading
+1 −0
Original line number Diff line number Diff line
@@ -159,6 +159,7 @@ cc_test {
        "devices_test.cpp",
        "init_test.cpp",
        "property_service_test.cpp",
        "result_test.cpp",
        "service_test.cpp",
        "ueventd_test.cpp",
        "util_test.cpp",
+27 −31
Original line number Diff line number Diff line
@@ -151,9 +151,8 @@ static int do_class_restart(const std::vector<std::string>& args) {
}

static int do_domainname(const std::vector<std::string>& args) {
    std::string err;
    if (!WriteFile("/proc/sys/kernel/domainname", args[1], &err)) {
        LOG(ERROR) << err;
    if (auto result = WriteFile("/proc/sys/kernel/domainname", args[1]); !result) {
        LOG(ERROR) << "Unable to write to /proc/sys/kernel/domainname: " << result.error();
        return -1;
    }
    return 0;
@@ -199,9 +198,8 @@ static int do_export(const std::vector<std::string>& args) {
}

static int do_hostname(const std::vector<std::string>& args) {
    std::string err;
    if (!WriteFile("/proc/sys/kernel/hostname", args[1], &err)) {
        LOG(ERROR) << err;
    if (auto result = WriteFile("/proc/sys/kernel/hostname", args[1]); !result) {
        LOG(ERROR) << "Unable to write to /proc/sys/kernel/hostname: " << result.error();
        return -1;
    }
    return 0;
@@ -244,22 +242,22 @@ static int do_mkdir(const std::vector<std::string>& args) {
    }

    if (args.size() >= 4) {
        uid_t uid;
        std::string decode_uid_err;
        if (!DecodeUid(args[3], &uid, &decode_uid_err)) {
            LOG(ERROR) << "Unable to find UID for '" << args[3] << "': " << decode_uid_err;
        auto uid = DecodeUid(args[3]);
        if (!uid) {
            LOG(ERROR) << "Unable to decode UID for '" << args[3] << "': " << uid.error();
            return -1;
        }
        gid_t gid = -1;
        Result<gid_t> gid = -1;

        if (args.size() == 5) {
            if (!DecodeUid(args[4], &gid, &decode_uid_err)) {
                LOG(ERROR) << "Unable to find GID for '" << args[3] << "': " << decode_uid_err;
            gid = DecodeUid(args[4]);
            if (!gid) {
                LOG(ERROR) << "Unable to decode GID for '" << args[3] << "': " << gid.error();
                return -1;
            }
        }

        if (lchown(args[1].c_str(), uid, gid) == -1) {
        if (lchown(args[1].c_str(), *uid, *gid) == -1) {
            return -errno;
        }

@@ -651,9 +649,8 @@ static int do_verity_update_state(const std::vector<std::string>& args) {
}

static int do_write(const std::vector<std::string>& args) {
    std::string err;
    if (!WriteFile(args[1], args[2], &err)) {
        LOG(ERROR) << err;
    if (auto result = WriteFile(args[1], args[2]); !result) {
        LOG(ERROR) << "Unable to write to file '" << args[1] << "': " << result.error();
        return -1;
    }
    return 0;
@@ -720,39 +717,38 @@ static int do_readahead(const std::vector<std::string>& args) {
}

static int do_copy(const std::vector<std::string>& args) {
    std::string data;
    std::string err;
    if (!ReadFile(args[1], &data, &err)) {
        LOG(ERROR) << err;
    auto file_contents = ReadFile(args[1]);
    if (!file_contents) {
        LOG(ERROR) << "Could not read input file '" << args[1] << "': " << file_contents.error();
        return -1;
    }
    if (!WriteFile(args[2], data, &err)) {
        LOG(ERROR) << err;
    if (auto result = WriteFile(args[2], *file_contents); !result) {
        LOG(ERROR) << "Could not write to output file '" << args[2] << "': " << result.error();
        return -1;
    }
    return 0;
}

static int do_chown(const std::vector<std::string>& args) {
    uid_t uid;
    std::string decode_uid_err;
    if (!DecodeUid(args[1], &uid, &decode_uid_err)) {
        LOG(ERROR) << "Unable to find UID for '" << args[1] << "': " << decode_uid_err;
    auto uid = DecodeUid(args[1]);
    if (!uid) {
        LOG(ERROR) << "Unable to decode UID for '" << args[1] << "': " << uid.error();
        return -1;
    }

    // GID is optional and pushes the index of path out by one if specified.
    const std::string& path = (args.size() == 4) ? args[3] : args[2];
    gid_t gid = -1;
    Result<gid_t> gid = -1;

    if (args.size() == 4) {
        if (!DecodeUid(args[2], &gid, &decode_uid_err)) {
            LOG(ERROR) << "Unable to find GID for '" << args[2] << "': " << decode_uid_err;
        gid = DecodeUid(args[2]);
        if (!gid) {
            LOG(ERROR) << "Unable to decode GID for '" << args[2] << "': " << gid.error();
            return -1;
        }
    }

    if (lchown(path.c_str(), uid, gid) == -1) return -errno;
    if (lchown(path.c_str(), *uid, *gid) == -1) return -errno;

    return 0;
}
+2 −3
Original line number Diff line number Diff line
@@ -156,10 +156,9 @@ TEST(init, EventTriggerOrderMultipleFiles) {
                               "execute 3";
    // clang-format on
    // WriteFile() ensures the right mode is set
    std::string err;
    ASSERT_TRUE(WriteFile(std::string(dir.path) + "/a.rc", dir_a_script, &err));
    ASSERT_TRUE(WriteFile(std::string(dir.path) + "/a.rc", dir_a_script));

    ASSERT_TRUE(WriteFile(std::string(dir.path) + "/b.rc", "on boot\nexecute 5", &err));
    ASSERT_TRUE(WriteFile(std::string(dir.path) + "/b.rc", "on boot\nexecute 5"));

    // clang-format off
    std::string start_script = "import " + std::string(first_import.path) + "\n"
+5 −6
Original line number Diff line number Diff line
@@ -102,15 +102,14 @@ void Parser::ParseData(const std::string& filename, const std::string& data) {
bool Parser::ParseConfigFile(const std::string& path) {
    LOG(INFO) << "Parsing file " << path << "...";
    android::base::Timer t;
    std::string data;
    std::string err;
    if (!ReadFile(path, &data, &err)) {
        LOG(ERROR) << err;
    auto config_contents = ReadFile(path);
    if (!config_contents) {
        LOG(ERROR) << "Unable to read config file '" << path << "': " << config_contents.error();
        return false;
    }

    data.push_back('\n');  // TODO: fix parse_config.
    ParseData(path, data);
    config_contents->push_back('\n');  // TODO: fix parse_config.
    ParseData(path, *config_contents);
    for (const auto& [section_name, section_parser] : section_parsers_) {
        section_parser->EndFile();
    }
+6 −6
Original line number Diff line number Diff line
@@ -588,14 +588,14 @@ static void load_properties(char *data, const char *filter)
// "ro.foo.*" is a prefix match, and "ro.foo.bar" is an exact match.
static void load_properties_from_file(const char* filename, const char* filter) {
    Timer t;
    std::string data;
    std::string err;
    if (!ReadFile(filename, &data, &err)) {
        PLOG(WARNING) << "Couldn't load property file: " << err;
    auto file_contents = ReadFile(filename);
    if (!file_contents) {
        PLOG(WARNING) << "Couldn't load property file '" << filename
                      << "': " << file_contents.error();
        return;
    }
    data.push_back('\n');
    load_properties(&data[0], filter);
    file_contents->push_back('\n');
    load_properties(file_contents->data(), filter);
    LOG(VERBOSE) << "(Loading properties from " << filename << " took " << t << ".)";
}

Loading