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
Change-Id: I1e7d3a8820a79362245041251057fbeed2f7979b
diff --git a/init/builtins.cpp b/init/builtins.cpp
index a1eab06..2d4d78b 100644
--- a/init/builtins.cpp
+++ b/init/builtins.cpp
@@ -151,9 +151,8 @@
}
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_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 @@
}
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;
}
@@ -668,9 +666,8 @@
}
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;
@@ -737,39 +734,38 @@
}
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;
}