[ovs-dev] [PATCH v2 1/2] chutil: introduce a new change-utils lib

[Ben Pfaff]

On Fri, May 20, 2016 at 04:32:05PM -0400, Aaron Conole wrote:
> It will be useful in the future to be able to set ownership and permissions
> on files which Open vSwitch creates. Allowing the specification of such
> ownership and permissions using the standard user:group, uog+-rwxs, and
> numerical forms commonly associated with those actions.
> 
> This patch introduces a new chutil library, currently with a posix command
> implementation. WIN32 support does not exist at this time, but could be added
> in the future.
> 
> As part of this, the daemon-unix.c was refactored to move the ownership
> parsing code to the chutil library. A new set of tests was added, and the
> chmod side is implemented.
> 
> Signed-off-by: Aaron Conole <aconole at redhat.com>

I suggest folding the following in for better conformance to
CodingStyle.md.

With those changes, I'm happy with this patch but I don't want to apply
it until we come to some kind of consensus on patch 2.

Acked-by: Ben Pfaff <blp at ovn.org>

--8<--------------------------cut here-------------------------->8--

diff --git a/lib/chutil-unix.c b/lib/chutil-unix.c
index 208ce6f..b61352a 100644
--- a/lib/chutil-unix.c
+++ b/lib/chutil-unix.c
@@ -15,16 +15,18 @@
  */
 
 #include <config.h>
+
+#include "chutil.h"
+
 #include <errno.h>
-#include <inttypes.h>
 #include <grp.h>
+#include <inttypes.h>
 #include <pwd.h>
 #include <stdlib.h>
 #include <string.h>
 #include <sys/stat.h>
 #include <unistd.h>
 
-#include "chutil.h"
 #include "daemon.h"
 #include "util.h"
 #include "openvswitch/vlog.h"
@@ -46,9 +48,8 @@ VLOG_DEFINE_THIS_MODULE(chutil_unix);
 
 #define SUID_MODES  (S_ISUID | S_ISGID)
 
-/* Convert a chown-style string to uid/gid; supports numeric arguments
- * as well as usernames.
- */
+/* Convert a chown-style string to uid/gid; supports numeric arguments as well
+ * as usernames.  Returns 0 on success, otherwise a positve errno value. */
 int
 ovs_strtousr(const char *user_spec, uid_t *uid, char **user, gid_t *gid,
              bool validate_user_group)
@@ -229,7 +230,7 @@ chmod_getmode(const char *mode, mode_t oldmode)
                 actors_mask = USR_MODES;
             }
             while (*mode) {
-                switch(*mode++) {
+                switch (*mode++) {
                 case 'r':
                     perms_mask |= READ_MODES;
                     break;
@@ -265,8 +266,8 @@ actions:
 }
 
 /* Changes the mode of a file to the mode specified.  Accepts chmod style
- * comma-separated strings.
- */
+ * comma-separated strings.  Returns 0 on success, otherwise a positve errno
+ * value. */
 int
 ovs_chmod(const char *path, const char *mode)
 {
@@ -307,8 +308,8 @@ ovs_chmod(const char *path, const char *mode)
 }
 
 /* Changes the ownership of a file to the mode specified.  Accepts chown style
- * user:group strings.
- */
+ * user:group strings.  Returns 0 on success, otherwise a positve errno
+ * value. */
 int
 ovs_chown(const char *path, const char *owner)
 {
diff --git a/tests/test-chutil.c b/tests/test-chutil.c
index 9057220..4465437 100644
--- a/tests/test-chutil.c
+++ b/tests/test-chutil.c
@@ -60,7 +60,8 @@ with_temp_file(int (*fn)(const char *pathname))
 }
 
 static int
-run_chmod_bad_parsing(const char *pathname) {
+run_chmod_bad_parsing(const char *pathname)
+{
     static char users[] = "bcdefhijklmnpqrstvwxyz";
     static char perms[] = "abcdefghijklmnopqtuvyz";
     static char actions[] = "~`!@#$%^&*()_";
@@ -72,7 +73,7 @@ run_chmod_bad_parsing(const char *pathname) {
         return -1;
     }
 
-    for (itest = users; itest != users+strlen(users); ++itest) {
+    for (itest = users; itest != users + strlen(users); ++itest) {
         char buf[256] = {0};
         mode_t testmode;
         snprintf(buf, sizeof(buf), "%c+rwx", *itest);
@@ -83,7 +84,7 @@ run_chmod_bad_parsing(const char *pathname) {
         }
     }
 
-    for (itest = perms; itest != perms+strlen(perms); ++itest) {
+    for (itest = perms; itest != perms + strlen(perms); ++itest) {
         char buf[256] = {0};
         mode_t testmode;
         snprintf(buf, sizeof(buf), "u+%c", *itest);
@@ -94,7 +95,7 @@ run_chmod_bad_parsing(const char *pathname) {
         }
     }
 
-    for (itest = actions; itest != actions+strlen(actions); ++itest) {
+    for (itest = actions; itest != actions + strlen(actions); ++itest) {
         char buf[256] = {0};
         mode_t testmode;
         snprintf(buf, sizeof(buf), "u%crw", *itest);
@@ -108,7 +109,7 @@ run_chmod_bad_parsing(const char *pathname) {
     return 0;
 }
 
-//skip suid and sgid for now
+/* Skip suid and sgid for now. */
 static int
 run_chmod_str_successes(const char *pathname)
 {
@@ -194,7 +195,7 @@ run_chmod_numeric_successes(const char *pathname)
 static int
 run_ovs_strtouser_successes(void)
 {
-    // seems this is the only user:group combination to exist?
+    /* seems this is the only user:group combination to exist? */
     const char *ugparses[] = {"root:", "root:root", "0:0", "0:", "nobody:",
                               "root", "nobody"};
     size_t iugstr;
@@ -214,8 +215,8 @@ run_ovs_strtouser_successes(void)
 static int
 run_ovs_strtouser_failures(void)
 {
-    // if any of these are successful, you have a poorly configured system
-    // so this test 'failing' is the least of your worries
+    /* If any of these are successful, you have a poorly configured system so
+     * this test 'failing' is the least of your worries. */
     const char *ugparses[] = {"nobody:root", "THISUSERBETTERNOTEXSIST:",
                               ":THISGROUPBETTERNOTEXIST"};
     size_t iugstr;