Ben Pfaff <b...@ovn.org> writes:
> 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 <acon...@redhat.com>
>
> I suggest folding the following in for better conformance to
> CodingStyle.md.
D'oh! I'm trying to undo 15 years of one particular c++ coding style,
so apologies that I keep messing these up.
> 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.
Agreed.
Thanks for the review, Ben!
-Aaron
> Acked-by: Ben Pfaff <b...@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;
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
