On Wed, Mar 13, 2013 at 15:24:01 +0000, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Introduce a local object virIdentity for managing security
attributes used to form a client application's identity.
Instances of this object are intended to be used as if they
were immutable, once created & populated with attributes
...
diff --git a/src/util/virerror.c b/src/util/virerror.c
index 40c3b25..8cb8548 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -118,6 +118,7 @@ VIR_ENUM_IMPL(virErrorDomain, VIR_ERR_DOMAIN_LAST,
"SSH transport layer", /* 50 */
"Lock Space",
"Init control",
+ "Identity",
)
@@ -1213,6 +1214,12 @@ virErrorMsg(virErrorNumber error, const char *info)
else
errmsg = _("resource busy %s");
break;
+ case VIR_ERR_INVALID_IDENTITY:
+ if (info == NULL)
+ errmsg = _("invalid identity");
+ else
+ errmsg = _("invalid identity %s");
+ break;
This doesn't seem to be used anywhere yet but would
"invalid identity: %s" result in better error messages?
}
return errmsg;
}
diff --git a/src/util/viridentity.c b/src/util/viridentity.c
new file mode 100644
index 0000000..7ebf851
--- /dev/null
+++ b/src/util/viridentity.c
...
+/**
+ * virIdentityIsEqual:
+ * @identA: the first identity
+ * @identB: the second identity
+ *
+ * Compares every attribute in @identA and @identB
+ * to determine if they refer to the same identity
+ *
+ * Returns 1 if they are equal, 0 if not equal or -1 on error
+ */
+int virIdentityIsEqual(virIdentityPtr identA,
+ virIdentityPtr identB)
+{
+ int ret = 0;
+ size_t i;
+ VIR_DEBUG("identA=%p identB=%p", identA, identB);
+
+ for (i = 0 ; i < VIR_IDENTITY_ATTR_LAST ; i++) {
+ if (STRNEQ_NULLABLE(identA->attrs[i],
+ identB->attrs[i]))
+ goto cleanup;
+ }
+
+ ret = 1;
+cleanup:
+ return ret;
+}
This API never fails, so why do we need to document unreachable -1 on
error and use int rather than bool? Especially when you use this API as
if it was just returning true/false.
diff --git a/tests/viridentitytest.c b/tests/viridentitytest.c
new file mode 100644
index 0000000..adb4f7d
--- /dev/null
+++ b/tests/viridentitytest.c
...
+static int testIdentityAttrs(const void *data ATTRIBUTE_UNUSED)
+{
+ int ret = -1;
+ virIdentityPtr ident;
+ const char *val;
+
+ if (!(ident = virIdentityNew()))
+ goto cleanup;
+
+ if (virIdentitySetAttr(ident,
+ VIR_IDENTITY_ATTR_UNIX_USER_NAME,
+ "fred") < 0)
+ goto cleanup;
+
+ if (virIdentityGetAttr(ident,
+ VIR_IDENTITY_ATTR_UNIX_USER_NAME,
+ &val) < 0)
+ goto cleanup;
+
+ if (STRNEQ_NULLABLE(val, "fred")) {
+ VIR_DEBUG("Expected 'fred' got '%s'", NULLSTR(val));
+ goto cleanup;
+ }
+
+ if (virIdentityGetAttr(ident,
+ VIR_IDENTITY_ATTR_UNIX_GROUP_NAME,
+ &val) < 0)
+ goto cleanup;
+
+ if (STRNEQ_NULLABLE(val, NULL)) {
Hmm, could be easier to just check if val is NULL :-)
+ VIR_DEBUG("Unexpected groupname attribute");
+ goto cleanup;
+ }
+
+ if (virIdentitySetAttr(ident,
+ VIR_IDENTITY_ATTR_UNIX_USER_NAME,
+ "joe") != -1) {
+ VIR_DEBUG("Unexpectedly overwrote attribute");
+ goto cleanup;
+ }
+
+ if (virIdentityGetAttr(ident,
+ VIR_IDENTITY_ATTR_UNIX_USER_NAME,
+ &val) < 0)
+ goto cleanup;
+
+ if (STRNEQ_NULLABLE(val, "fred")) {
+ VIR_DEBUG("Expected 'fred' got '%s'", NULLSTR(val));
+ goto cleanup;
+ }
+
+ ret = 0;
+cleanup:
+ virObjectUnref(ident);
+ return ret;
+}
+
+
+static int testIdentityEqual(const void *data ATTRIBUTE_UNUSED)
+{
+ int ret = -1;
+ virIdentityPtr identa = NULL;
+ virIdentityPtr identb = NULL;
+
+ if (!(identa = virIdentityNew()))
+ goto cleanup;
+ if (!(identb = virIdentityNew()))
+ goto cleanup;
+
+ if (!virIdentityIsEqual(identa, identb)) {
+ VIR_DEBUG("Empty identities were no equal");
s/no/not/
+ goto cleanup;
+ }
...
ACK with the comments addressed.
Jirka