Created attachment 289933 [details] [review]
goadaemon: free presentation identity when finished with it

The update_account_object function reads an account's presentation
identity from the accounts configuration.  GKeyFile returns a
duped string and so the presentation identity needs to be freed.

Created attachment 289934 [details] [review]
identity: plug leak in identity service

When the identity service first starts up, it gets a list
of preexisting kerberos principals and then exports them
over the bus.

The object path used for the exported objects is getting leaked.
This commit fixes that.

Created attachment 289935 [details] [review]
kerberos: plug leak in error handling

It's a little tedious to convert a kerberos error into a GError,
so there's a helper function to do it: set_error_from_krb5_error_code

That helper function leaks the format string use for constructing the
GError's message.

This commit fixes that.

Review of attachment 289933 [details] [review]:

Good catch!

Review of attachment 289934 [details] [review]:

Thanks for the patch, Ray.

::: src/goaidentity/goaidentityservice.c
@@ +1602,3 @@
       const char  *principal;
       GoaObject   *object;
+      char *object_path;

Don't you want to align the '*object_path' with the previous lines? :)

@@ +1606,1 @@
+      object_path = export_identity (self, identity);

Would it be overkill to mark export_identity as G_GNUC_WARN_UNUSED_RESULT ?

Review of attachment 289935 [details] [review]:

Looks good to me.

(In reply to comment #5)
> Review of attachment 289934 [details] [review]:
> 
> Thanks for the patch, Ray.
> 
> ::: src/goaidentity/goaidentityservice.c
> @@ +1602,3 @@
>        const char  *principal;
>        GoaObject   *object;
> +      char *object_path;
> 
> Don't you want to align the '*object_path' with the previous lines? :)
I do, indeed. Thanks.

> @@ +1606,1 @@
> +      object_path = export_identity (self, identity);
> 
> Would it be overkill to mark export_identity as G_GNUC_WARN_UNUSED_RESULT ?

I don't think it would be overkill, but it would be a bit out of character for the codebase, as it stands (which doesn't otherwise use that notation).  I think having a flag day adding that to the various functions that need it would be a useful (though not critical) change, but I don't think we should introduce it in this commit, imo.

Attachment 289934 [details] pushed as 924d41f - identity: plug leak in identity service

(In reply to comment #7)

> I don't think it would be overkill, but it would be a bit out of character for
> the codebase, as it stands (which doesn't otherwise use that notation).  I
> think having a flag day adding that to the various functions that need it would
> be a useful (though not critical) change, but I don't think we should introduce
> it in this commit, imo.

Ok.