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

Re: [libvirt] [PATCH v2 2/6] Define internal APIs for managing identities



On Tue, Mar 19, 2013 at 12:20:51PM +0100, Jiri Denemark wrote:
> On Wed, Mar 13, 2013 at 15:24:01 +0000, Daniel P. Berrange wrote:
> > From: "Daniel P. Berrange" <berrange 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?

Oh this is left over from earlier versions fo the patch. I'll
kill it now.

> 
> >      }
> >      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.

Again, left over cruft from earlier versions.


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


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