Merge pull request #9229 from tfc/small-improvements

Remove warnings, small improvements

(cherry picked from commit 5ac87a75dd65c19c50976559a6f7810c315cd9d5)
Change-Id: I88349b6e954398dde83c845f42d41a9dd89ba9e0

3 files changed, 51 insertions, 44 deletions

diff --git a/src/libstore/builtins/buildenv.cc b/src/libstore/builtins/buildenv.cc
index 7bba33fb9..c8911d153 100644
--- a/src/libstore/builtins/buildenv.cc
+++ b/src/libstore/builtins/buildenv.cc
@@ -174,15 +174,19 @@ void builtinBuildenv(const BasicDerivation & drv)
     /* Convert the stuff we get from the environment back into a
      * coherent data type. */
     Packages pkgs;
-    auto derivations = tokenizeString<Strings>(getAttr("derivations"));
-    while (!derivations.empty()) {
-        /* !!! We're trusting the caller to structure derivations env var correctly */
-        auto active = derivations.front(); derivations.pop_front();
-        auto priority = stoi(derivations.front()); derivations.pop_front();
-        auto outputs = stoi(derivations.front()); derivations.pop_front();
-        for (auto n = 0; n < outputs; n++) {
-            auto path = derivations.front(); derivations.pop_front();
-            pkgs.emplace_back(path, active != "false", priority);
+    {
+        auto derivations = tokenizeString<Strings>(getAttr("derivations"));
+
+        auto itemIt = derivations.begin();
+        while (itemIt != derivations.end()) {
+            /* !!! We're trusting the caller to structure derivations env var correctly */
+            const bool active = "false" != *itemIt++;
+            const int priority = stoi(*itemIt++);
+            const size_t outputs = stoul(*itemIt++);
+
+            for (size_t n {0}; n < outputs; n++) {
+                pkgs.emplace_back(std::move(*itemIt++), active, priority);
+            }
         }
     }
 
diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc
index ae91b859b..77f23b0b3 100644
--- a/src/libstore/content-address.cc
+++ b/src/libstore/content-address.cc
@@ -29,12 +29,13 @@ std::string ContentAddressMethod::renderPrefix() const
 
 ContentAddressMethod ContentAddressMethod::parsePrefix(std::string_view & m)
 {
-    ContentAddressMethod method = FileIngestionMethod::Flat;
-    if (splitPrefix(m, "r:"))
-        method = FileIngestionMethod::Recursive;
-    else if (splitPrefix(m, "text:"))
-        method = TextIngestionMethod {};
-    return method;
+    if (splitPrefix(m, "r:")) {
+        return FileIngestionMethod::Recursive;
+    }
+    else if (splitPrefix(m, "text:")) {
+        return TextIngestionMethod {};
+    }
+    return FileIngestionMethod::Flat;
 }
 
 std::string ContentAddressMethod::render(HashType ht) const
diff --git a/src/libstore/lock.cc b/src/libstore/lock.cc
index 7202a64b3..165e4969f 100644
--- a/src/libstore/lock.cc
+++ b/src/libstore/lock.cc
@@ -7,6 +7,31 @@
 
 namespace nix {
 
+#if __linux__
+
+static std::vector<gid_t> get_group_list(const char *username, gid_t group_id)
+{
+    std::vector<gid_t> gids;
+    gids.resize(32); // Initial guess
+
+    auto getgroupl_failed {[&] {
+        int ngroups = gids.size();
+        int err = getgrouplist(username, group_id, gids.data(), &ngroups);
+        gids.resize(ngroups);
+        return err == -1;
+    }};
+
+    // The first error means that the vector was not big enough.
+    // If it happens again, there is some different problem.
+    if (getgroupl_failed() && getgroupl_failed()) {
+        throw SysError("failed to get list of supplementary groups for '%s'", username);
+    }
+
+    return gids;
+}
+#endif
+
+
 struct SimpleUserLock : UserLock
 {
     AutoCloseFD fdUserLock;
@@ -67,37 +92,14 @@ struct SimpleUserLock : UserLock
                     throw Error("the Nix user should not be a member of '%s'", settings.buildUsersGroup);
 
                 #if __linux__
-                /* Get the list of supplementary groups of this build
-                   user.  This is usually either empty or contains a
-                   group such as "kvm".  */
-                int ngroups = 32; // arbitrary initial guess
-                std::vector<gid_t> gids;
-                gids.resize(ngroups);
-
-                int err = getgrouplist(
-                    pw->pw_name, pw->pw_gid,
-                    gids.data(),
-                    &ngroups);
-
-                /* Our initial size of 32 wasn't sufficient, the
-                   correct size has been stored in ngroups, so we try
-                   again. */
-                if (err == -1) {
-                    gids.resize(ngroups);
-                    err = getgrouplist(
-                        pw->pw_name, pw->pw_gid,
-                        gids.data(),
-                        &ngroups);
-                }
-
-                // If it failed once more, then something must be broken.
-                if (err == -1)
-                    throw Error("failed to get list of supplementary groups for '%s'", pw->pw_name);
+                /* Get the list of supplementary groups of this user. This is
+                 * usually either empty or contains a group such as "kvm". */
 
                 // Finally, trim back the GID list to its real size.
-                for (auto i = 0; i < ngroups; i++)
-                    if (gids[i] != lock->gid)
-                        lock->supplementaryGIDs.push_back(gids[i]);
+                for (auto gid : get_group_list(pw->pw_name, pw->pw_gid)) {
+                    if (gid != lock->gid)
+                        lock->supplementaryGIDs.push_back(gid);
+                }
                 #endif
 
                 return lock;