[PATCH] simplify permission handling

Initialize the defaults in udev_config.c instead of namedev.c.
Replace macro by expanded code. Switch to mode_t instead
of string value. Add and clarify some comments.

diff --git a/namedev.c b/namedev.c
index c043cf339e1695a85ea641c9440fb3315dc4b8b0..0bc28bd028cc4eee3822b6540946986004364b2f 100644 (file)
--- a/namedev.c
+++ b/namedev.c
@@ -101,7 +101,7 @@ static int strcmp_pattern(const char *p, const char *s)
        return 1;
 }
 
-static struct perm_device *find_perm(char *name)
+static struct perm_device *find_perm_entry(const char *name)
 {
        struct perm_device *perm;
 
@@ -113,32 +113,6 @@ static struct perm_device *find_perm(char *name)
        return NULL;
 }
 
-static mode_t get_default_mode(void)
-{
-       mode_t mode = 0600;     /* default to owner rw only */
-
-       if (strlen(default_mode_str) != 0)
-               mode = strtol(default_mode_str, NULL, 8);
-
-       return mode;
-}
-
-static char *get_default_owner(void)
-{
-       if (strlen(default_owner_str) == 0)
-               strfieldcpy(default_owner_str, "root");
-
-       return default_owner_str;
-}
-
-static char *get_default_group(void)
-{
-       if (strlen(default_group_str) == 0)
-               strfieldcpy(default_group_str, "root");
-
-       return default_group_str;
-}
-
 /* extract possible {attr} and move str behind it */
 static char *get_format_attribute(char **str)
 {
@@ -586,7 +560,8 @@ static int match_place(struct config_device *dev, struct sysfs_class_device *cla
        return 0;
 }
 
-static int match_rule(struct config_device *dev, struct sysfs_class_device *class_dev, struct udevice *udev, struct sysfs_device *sysfs_device)
+static int match_rule(struct udevice *udev, struct config_device *dev,
+                     struct sysfs_class_device *class_dev, struct sysfs_device *sysfs_device)
 {
        while (1) {
                /* check for matching bus value */
@@ -595,7 +570,8 @@ static int match_rule(struct config_device *dev, struct sysfs_class_device *clas
                                dbg("device has no bus");
                                goto try_parent;
                        }
-                       dbg("check for " FIELD_BUS " dev->bus='%s' sysfs_device->bus='%s'", dev->bus, sysfs_device->bus);
+                       dbg("check for " FIELD_BUS " dev->bus='%s' sysfs_device->bus='%s'",
+                           dev->bus, sysfs_device->bus);
                        if (strcmp_pattern(dev->bus, sysfs_device->bus) != 0) {
                                dbg(FIELD_BUS " is not matching");
                                goto try_parent;
@@ -606,7 +582,8 @@ static int match_rule(struct config_device *dev, struct sysfs_class_device *clas
 
                /* check for matching kernel name */
                if (dev->kernel[0] != '\0') {
-                       dbg("check for " FIELD_KERNEL " dev->kernel='%s' class_dev->name='%s'", dev->kernel, class_dev->name);
+                       dbg("check for " FIELD_KERNEL " dev->kernel='%s' class_dev->name='%s'",
+                           dev->kernel, class_dev->name);
                        if (strcmp_pattern(dev->kernel, class_dev->name) != 0) {
                                dbg(FIELD_KERNEL " is not matching");
                                goto try_parent;
@@ -617,7 +594,8 @@ static int match_rule(struct config_device *dev, struct sysfs_class_device *clas
 
                /* check for matching subsystem */
                if (dev->subsystem[0] != '\0') {
-                       dbg("check for " FIELD_SUBSYSTEM " dev->subsystem='%s' class_dev->name='%s'", dev->subsystem, class_dev->name);
+                       dbg("check for " FIELD_SUBSYSTEM " dev->subsystem='%s' class_dev->name='%s'",
+                           dev->subsystem, class_dev->name);
                        if (strcmp_pattern(dev->subsystem, udev->subsystem) != 0) {
                                dbg(FIELD_SUBSYSTEM " is not matching");
                                goto try_parent;
@@ -628,7 +606,8 @@ static int match_rule(struct config_device *dev, struct sysfs_class_device *clas
 
                /* check for matching driver */
                if (dev->driver[0] != '\0') {
-                       dbg("check for " FIELD_DRIVER " dev->driver='%s' sysfs_device->driver_name='%s'", dev->driver, sysfs_device->driver_name);
+                       dbg("check for " FIELD_DRIVER " dev->driver='%s' sysfs_device->driver_name='%s'",
+                           dev->driver, sysfs_device->driver_name);
                        if (strcmp_pattern(dev->driver, sysfs_device->driver_name) != 0) {
                                dbg(FIELD_DRIVER " is not matching");
                                goto try_parent;
@@ -757,13 +736,15 @@ int namedev_name_device(struct udevice *udev, struct sysfs_class_device *class_d
        /* look for a matching rule to apply */
        list_for_each_entry(dev, &config_device_list, node) {
                dbg("process rule");
-               if (match_rule(dev, class_dev, udev, sysfs_device) == 0) {
+               if (match_rule(udev, dev, class_dev, sysfs_device) == 0) {
+                       /* empty name and symlink will not create any node */
                        if (dev->name[0] == '\0' && dev->symlink[0] == '\0') {
                                info("configured rule in '%s' at line %i applied, '%s' is ignored",
                                     dev->config_file, dev->config_line, udev->kernel_name);
                                return -1;
                        }
 
+                       /* collect symlinks for the final matching rule */
                        if (dev->symlink[0] != '\0') {
                                char temp[NAME_SIZE];
 
@@ -776,6 +757,7 @@ int namedev_name_device(struct udevice *udev, struct sysfs_class_device *class_d
                                strfieldcat(udev->symlink, temp);
                        }
 
+                       /* rule matches */
                        if (dev->name[0] != '\0') {
                                /* apply all_partitions flag only at a main block device */
                                if (dev->partitions > 0 &&
@@ -784,45 +766,51 @@ int namedev_name_device(struct udevice *udev, struct sysfs_class_device *class_d
 
                                info("configured rule in '%s' at line %i applied, '%s' becomes '%s'",
                                     dev->config_file, dev->config_line, udev->kernel_name, dev->name);
+
                                strfieldcpy(udev->name, dev->name);
-                               goto found;
+                               apply_format(udev, udev->name, sizeof(udev->name), class_dev, sysfs_device);
+                               strfieldcpy(udev->config_file, dev->config_file);
+                               udev->config_line = dev->config_line;
+                               udev->ignore_remove = dev->ignore_remove;
+
+                               if (udev->type == 'n')
+                                       goto done;
+
+                               udev->partitions = dev->partitions;
+                               udev->mode = dev->mode;
+                               strfieldcpy(udev->owner, dev->owner);
+                               strfieldcpy(udev->group, dev->group);
+
+                               goto perms;
                        }
                }
        }
-       /* no rule was found so we use the kernel name */
-       strfieldcpy(udev->name, udev->kernel_name);
-       if (udev->type == 'n')
-               goto done;
-       else
-               goto perms;
 
-found:
-       apply_format(udev, udev->name, sizeof(udev->name), class_dev, sysfs_device);
-       strfieldcpy(udev->config_file, dev->config_file);
-       udev->config_line = dev->config_line;
+       /* no rule matched, so we use the kernel name */
+       strfieldcpy(udev->name, udev->kernel_name);
 
        if (udev->type == 'n')
                goto done;
 
-       udev->partitions = dev->partitions;
-       udev->ignore_remove = dev->ignore_remove;
-
-       /* get permissions given in rule */
-       set_empty_perms(udev, dev->mode,
-                             dev->owner,
-                             dev->group);
-
 perms:
-       /* get permissions given in config file or set defaults */
-       perm = find_perm(udev->name);
+       /* apply permissions from permissions file to empty fields */
+       perm = find_perm_entry(udev->name);
        if (perm != NULL) {
-               set_empty_perms(udev, perm->mode,
-                                     perm->owner,
-                                     perm->group);
+               if (udev->mode == 0000)
+                       udev->mode = perm->mode;
+               if (udev->owner[0] == '\0')
+                       strfieldcpy(udev->owner, perm->owner);
+               if (udev->group[0] == '\0')
+                       strfieldcpy(udev->group, perm->group);
        }
-       set_empty_perms(udev, get_default_mode(),
-                             get_default_owner(),
-                             get_default_group());
+
+       /* apply permissions from config to empty fields */
+       if (udev->mode == 0000)
+               udev->mode = default_mode;
+       if (udev->owner[0] == '\0')
+               strfieldcpy(udev->owner, default_owner);
+       if (udev->group[0] == '\0')
+               strfieldcpy(udev->group, default_group);
 
        dbg("name, '%s' is going to have owner='%s', group='%s', mode = %#o",
            udev->name, udev->owner, udev->group, udev->mode);

diff --git a/namedev.h b/namedev.h
index 8e552c85684ba580a3f43ffc55ace21b5a089bd3..e29279243bca829838408d00582984bf140c7123 100644 (file)
--- a/namedev.h
+++ b/namedev.h
@@ -60,14 +60,6 @@ struct sysfs_class_device;
 #define RULEFILE_SUFFIX                ".rules"
 #define PERMFILE_SUFFIX                ".permissions"
 
-#define set_empty_perms(dev, m, o, g)          \
-       if (dev->mode == 0)                     \
-               dev->mode = m;                  \
-       if (dev->owner[0] == '\0')              \
-               strfieldcpy(dev->owner, o);     \
-       if (dev->group[0] == '\0')              \
-               strfieldcpy(dev->group, g);
-
 struct sysfs_pair {
        char file[FILE_SIZE];
        char value[VALUE_SIZE];
@@ -87,9 +79,9 @@ struct config_device {
        char name[NAME_SIZE];
        char symlink[NAME_SIZE];
        struct sysfs_pair sysfs_pair[MAX_SYSFS_PAIRS];
-       char owner[OWNER_SIZE];
-       char group[GROUP_SIZE];
-       unsigned int mode;
+       char owner[USER_SIZE];
+       char group[USER_SIZE];
+       mode_t mode;
        int partitions;
        int ignore_remove;
        char config_file[NAME_SIZE];
@@ -100,8 +92,8 @@ struct perm_device {
        struct list_head node;
 
        char name[NAME_SIZE];
-       char owner[OWNER_SIZE];
-       char group[GROUP_SIZE];
+       char owner[USER_SIZE];
+       char group[USER_SIZE];
        unsigned int mode;
 };
 

diff --git a/namedev_parse.c b/namedev_parse.c
index d39141bfc7536d1718f271502a887014eabbd12d..5b8c5ab2d6989a32d8648b6ce048128116d8f144 100644 (file)
--- a/namedev_parse.c
+++ b/namedev_parse.c
@@ -79,12 +79,19 @@ static int add_perm_dev(struct perm_device *new_dev)
        struct perm_device *dev;
        struct perm_device *tmp_dev;
 
-       /* update the values if we already have the device */
+       /* if we already have that entry, just update the values */
        list_for_each_entry(dev, &perm_device_list, node) {
                if (strcmp(new_dev->name, dev->name) != 0)
                        continue;
 
-               set_empty_perms(dev, new_dev->mode, new_dev->owner, new_dev->group);
+               /* don't overwrite values from earlier entries */
+               if (dev->mode == 0000)
+                       dev->mode = new_dev->mode;
+               if (dev->owner[0] == '\0')
+                       strfieldcpy(dev->owner, new_dev->owner);
+               if (dev->owner[0] == '\0')
+                       strfieldcpy(dev->group, new_dev->group);
+
                return 0;
        }
 
@@ -95,7 +102,8 @@ static int add_perm_dev(struct perm_device *new_dev)
 
        memcpy(tmp_dev, new_dev, sizeof(*tmp_dev));
        list_add_tail(&tmp_dev->node, &perm_device_list);
-       //dump_perm_dev(tmp_dev);
+       /* dump_perm_dev(tmp_dev); */
+
        return 0;
 }
 

diff --git a/udev.h b/udev.h
index 0703b884cfd85b5c75392fa0afbff5d306963913..cff991e57a040b34206833a83d88d8b3e6d30db3 100644 (file)
--- a/udev.h
+++ b/udev.h
@@ -30,9 +30,7 @@
 #define COMMENT_CHARACTER              '#'
 
 #define NAME_SIZE                      256
-#define OWNER_SIZE                     32
-#define GROUP_SIZE                     32
-#define MODE_SIZE                      8
+#define USER_SIZE                      32
 
 #define ACTION_SIZE                    32
 #define DEVPATH_SIZE                   256
@@ -53,8 +51,8 @@ struct udevice {
 
        char name[NAME_SIZE];
        char symlink[NAME_SIZE];
-       char owner[OWNER_SIZE];
-       char group[GROUP_SIZE];
+       char owner[USER_SIZE];
+       char group[USER_SIZE];
        mode_t mode;
        char type;
        int major;
@@ -86,9 +84,9 @@ extern char udev_db_path[PATH_MAX+NAME_MAX];
 extern char udev_permissions_filename[PATH_MAX+NAME_MAX];
 extern char udev_config_filename[PATH_MAX+NAME_MAX];
 extern char udev_rules_filename[PATH_MAX+NAME_MAX];
-extern char default_mode_str[MODE_SIZE];
-extern char default_owner_str[OWNER_SIZE];
-extern char default_group_str[GROUP_SIZE];
+extern mode_t default_mode;
+extern char default_owner[USER_SIZE];
+extern char default_group[USER_SIZE];
 extern int udev_log;
 extern int udev_dev_d;
 extern int udev_hotplug_d;

diff --git a/udev_config.c b/udev_config.c
index 9549471b8bc718455eb34587cac47d9b1a49bf8c..62eb728ea3283ec5c322bf7542699b1879e9b1fd 100644 (file)
--- a/udev_config.c
+++ b/udev_config.c
@@ -46,15 +46,15 @@ char udev_db_path[PATH_MAX+NAME_MAX];
 char udev_permissions_filename[PATH_MAX+NAME_MAX];
 char udev_rules_filename[PATH_MAX+NAME_MAX];
 char udev_config_filename[PATH_MAX+NAME_MAX];
-char default_mode_str[MODE_SIZE];
-char default_owner_str[OWNER_SIZE];
-char default_group_str[GROUP_SIZE];
+mode_t default_mode;
+char default_owner[USER_SIZE];
+char default_group[USER_SIZE];
 int udev_log;
 int udev_dev_d;
 int udev_hotplug_d;
 
 
-static int string_is_true(char *str)
+static int string_is_true(const char *str)
 {
        if (strcasecmp(str, "true") == 0)
                return 1;
@@ -67,16 +67,19 @@ static int string_is_true(char *str)
 
 static void init_variables(void)
 {
-       char *env;
-
-       /* fill up the defaults.
-        * If any config values are specified, they will
-        * override these values. */
-       strfieldcpy(udev_root, UDEV_ROOT);
-       strfieldcpy(udev_db_path, UDEV_DB);
-       strfieldcpy(udev_config_filename, UDEV_CONFIG_FILE);
-       strfieldcpy(udev_rules_filename, UDEV_RULES_FILE);
-       strfieldcpy(udev_permissions_filename, UDEV_PERMISSION_FILE);
+       const char *env;
+
+       /* If any config values are specified, they will override these values. */
+       strcpy(udev_root, UDEV_ROOT);
+       strcpy(udev_db_path, UDEV_DB);
+       strcpy(udev_config_filename, UDEV_CONFIG_FILE);
+       strcpy(udev_rules_filename, UDEV_RULES_FILE);
+       strcpy(udev_permissions_filename, UDEV_PERMISSION_FILE);
+
+       strcpy(default_owner, "root");
+       strcpy(default_group, "root");
+       default_mode = 0600;
+
        udev_log = string_is_true(UDEV_LOG_DEFAULT);
 
        udev_dev_d = 1;
@@ -210,17 +213,17 @@ static int parse_config_file(void)
                }
 
                if (strcasecmp(variable, "default_mode") == 0) {
-                       strfieldcpy(default_mode_str, value);
+                       default_mode = strtol(value, NULL, 8);
                        continue;
                }
 
                if (strcasecmp(variable, "default_owner") == 0) {
-                       strfieldcpy(default_owner_str, value);
+                       strfieldcpy(default_owner, value);
                        continue;
                }
 
                if (strcasecmp(variable, "default_group") == 0) {
-                       strfieldcpy(default_group_str, value);
+                       strfieldcpy(default_group, value);
                        continue;
                }
 
@@ -274,7 +277,7 @@ static void get_dirs(void)
        dbg("udev_db_path = %s", udev_db_path);
        dbg("udev_rules_filename = %s", udev_rules_filename);
        dbg("udev_permissions_filename = %s", udev_permissions_filename);
-       dbg("udev_log_str = %d", udev_log);
+       dbg("udev_log = %d", udev_log);
 }
 
 void udev_init_config(void)