[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 2/2] security: update user and group parsing in security_dac.c



On 10/06/12 04:52, Marcelo Cerri wrote:
The functions virGetUserID and virGetGroupID are now able to parse
user/group names and IDs in a similar way to coreutils' chown. So, user
and group parsing in security_dac can be simplified.
---
  src/security/security_dac.c | 40 ++++++++++------------------------------
  1 file changed, 10 insertions(+), 30 deletions(-)

diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index a427e9d..e976144 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -95,39 +95,19 @@ int parseIds(const char *label, uid_t *uidPtr, gid_t *gidPtr)
      group = sep + 1;

      /* Parse owner */
-    if (*owner == '+') {
-        if (virStrToLong_ui(++owner, NULL, 10, &theuid) < 0) {
-            virReportError(VIR_ERR_INVALID_ARG,
-                           _("Invalid uid \"%s\" in DAC label \"%s\""),
-                           owner, label);
-            goto cleanup;
-        }
-    } else {
-        if (virGetUserID(owner, &theuid) < 0 &&
-            virStrToLong_ui(owner, NULL, 10, &theuid) < 0) {
-            virReportError(VIR_ERR_INVALID_ARG,
-                           _("Invalid owner \"%s\" in DAC label \"%s\""),
-                           owner, label);
-            goto cleanup;
-        }
+    if (virGetUserID(owner, &theuid) < 0) {
+        virReportError(VIR_ERR_INVALID_ARG,
+                       _("Invalid owner \"%s\" in DAC label \"%s\""),
+                       owner, label);
+        goto cleanup;
      }

      /* Parse group */
-    if (*group == '+') {
-        if (virStrToLong_ui(++group, NULL, 10, &thegid) < 0) {
-            virReportError(VIR_ERR_INVALID_ARG,
-                           _("Invalid gid \"%s\" in DAC label \"%s\""),
-                           group, label);
-            goto cleanup;
-        }
-    } else {
-        if (virGetGroupID(group, &thegid) < 0 &&
-            virStrToLong_ui(group, NULL, 10, &thegid) < 0) {
-            virReportError(VIR_ERR_INVALID_ARG,
-                           _("Invalid group \"%s\" in DAC label \"%s\""),
-                           group, label);
-            goto cleanup;
-        }
+    if (virGetGroupID(group, &thegid) < 0) {
+        virReportError(VIR_ERR_INVALID_ARG,
+                       _("Invalid group \"%s\" in DAC label \"%s\""),
+                       group, label);

Hm, this will mask the errors from virGetGroupID that might be useful. Should we remove this message in favor of the underlying messages or have this that has more relevant information where to find the issue but maybe mask the reason?

Any opinions?

+        goto cleanup;
      }

      if (uidPtr)



Peter


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]