Move krb5_principal wrapping to the utility code

To prepare for returning Authen::Kerberos::Principal objects from
Authen::Kerberos::Kadmin methods, move the wrapping function to
the separate utility code.

diff --git a/lib/Authen/Kerberos.xs b/lib/Authen/Kerberos.xs
index 95bfb0f68ce03749f80420d12a546617b0c1f68f..dd9eac8c6d7dadfa99ecb11cd5b7641a22717daa 100644 (file)
--- a/lib/Authen/Kerberos.xs
+++ b/lib/Authen/Kerberos.xs
@@ -33,6 +33,8 @@
 #include <portable/krb5.h>
 #include <portable/system.h>
 
+#include <stdlib.h>
+
 #include <EXTERN.h>
 #include <perl.h>
 #include <XSUB.h>
@@ -73,38 +75,6 @@ typedef struct {
     krb5_keytab_entry entry;
 } *Authen__Kerberos__KeytabEntry;
 
-typedef struct {
-    SV *ctx;
-    krb5_principal principal;
-} *Authen__Kerberos__Principal;
-
-
-/*
- * Given a krb5_principal, convert it to an Authen::Kerberos::Principal
- * object.  Make a copy of the principal so that it can be stored and used
- * independently of whatever object we generated it from.  Takes both the
- * context and the underlying Authen::Kerberos object so that we can stash the
- * latter in the created struct.
- */
-static Authen__Kerberos__Principal
-wrap_principal(krb5_context ctx, SV *krb5, krb5_principal princ)
-{
-    krb5_principal copy;
-    krb5_error_code code;
-    Authen__Kerberos__Principal principal;
-
-    code = krb5_copy_principal(ctx, princ, &copy);
-    if (code != 0)
-        krb5_croak(ctx, code, "krb5_copy_principal", FALSE);
-    principal = malloc(sizeof(*principal));
-    if (principal == NULL)
-        croak("cannot allocate memory");
-    principal->ctx = krb5;
-    SvREFCNT_inc_simple_void_NN(principal->ctx);
-    principal->principal = copy;
-    return principal;
-}
-
 
 /*
  * Helper function to convert an argument to a keytab.  Accept either an
@@ -120,9 +90,9 @@ sv_to_keytab(SV *krb5, SV *sv)
 
     /*
      * If the SV is not already an Authen::Kerberos::Keytab object, we have to
-     * create a new krb5_keytab.  Do so by calling the normal constructor and
-     * then making the resulting object mortal.  This allows us to return its
-     * contents and rely on Perl garbage collection to free it later.
+     * create a new krb5_keytab.  Do so by calling the normal constructor.
+     * This allows us to return its contents and rely on Perl garbage
+     * collection to free it later.
      *
      * Temporaries will be freed by the caller's FREETMPS/LEAVE.  Stay within
      * the caller's temporary frame.
@@ -131,11 +101,16 @@ sv_to_keytab(SV *krb5, SV *sv)
         int count;
         dSP;
 
+        /* Set up the stack for the Perl call. */
         PUSHMARK(sp);
         XPUSHs(krb5);
         XPUSHs(sv);
         PUTBACK;
+
+        /* Turn the scalar into a keytab using the regular method. */
         count = call_method("keytab", G_SCALAR);
+
+        /* Retrieve the returned object from the stack. */
         SPAGAIN;
         if (count != 1)
             croak("Authen::Kerberos::keytab returned %d values", count);
@@ -375,7 +350,7 @@ client(self)
 {
     CROAK_NULL_SELF(self, "Authen::Kerberos::Creds", "client");
     ctx = krb5_context_from_sv(self->ctx, "Authen::Kerberos::Creds");
-    RETVAL = wrap_principal(ctx, self->ctx, self->creds.client);
+    RETVAL = krb5_wrap_principal(ctx, self->ctx, self->creds.client);
 }
   OUTPUT:
     RETVAL
@@ -393,7 +368,7 @@ server(self)
 {
     CROAK_NULL_SELF(self, "Authen::Kerberos::Creds", "server");
     ctx = krb5_context_from_sv(self->ctx, "Authen::Kerberos::Creds");
-    RETVAL = wrap_principal(ctx, self->ctx, self->creds.server);
+    RETVAL = krb5_wrap_principal(ctx, self->ctx, self->creds.server);
 }
   OUTPUT:
     RETVAL

diff --git a/util/util.h b/util/util.h
index 027f62f6ad4f1c89c92fcd5a19c91e91cb31cbbf..c0209ea5aa09a6d7d46f53c0b8f954f2a629bd35 100644 (file)
--- a/util/util.h
+++ b/util/util.h
@@ -2,7 +2,8 @@
  * Extra utility functions for Kerberos Perl bindings.
  *
  * Prototypes for various utility functions used by multiple Kerberos XS
- * modules, collected together to avoid code duplication.
+ * modules, collected together to avoid code duplication and so that the
+ * modules can use the same internal representation for some objects.
  *
  * Written by Russ Allbery <rra@cpan.org>
  * Copyright 2014
@@ -43,6 +44,21 @@
     } while (0);
 #define CROAK_NULL_SELF(o, t, f) CROAK_NULL((o), t, t "::" f)
 
+/*
+ * Our internal representation of a krb5_principal, which both
+ * Authen::Kerberos and Authen::Kerberos::Kadmin methods need to be able to
+ * generate, interoperably.
+ *
+ * We wrap the API data structure so that we can store a reference to the
+ * Authen::Kerberos object (the Kerberos context), ensuring that the Kerberos
+ * context is not freed before the secondary object and that the same Kerberos
+ * context is used for all operations on that object.
+ */
+typedef struct {
+    SV *ctx;
+    krb5_principal principal;
+} *Authen__Kerberos__Principal;
+
 BEGIN_DECLS
 
 /* Default to a hidden visibility for all util functions. */
@@ -65,6 +81,14 @@ void krb5_croak(krb5_context, krb5_error_code, const char *function,
                 bool destroy)
     __attribute__((__noreturn__));
 
+/*
+ * Given a Kerberos context, an Authen::Kerberos SV, and a krb5_principal,
+ * copy the latter and create a wrapped Authen__Kerberos__Principal object
+ * suitable for returning to let XS bless into an Authen::Kerberos::Principal.
+ */
+Authen__Kerberos__Principal krb5_wrap_principal(krb5_context, SV *,
+                                                krb5_principal);
+
 /* Undo default visibility change. */
 #pragma GCC visibility pop
 

diff --git a/util/wrap.c b/util/wrap.c
new file mode 100644 (file)
index 0000000..15e1dd6
--- /dev/null
+++ b/util/wrap.c
@@ -0,0 +1,63 @@
+/*
+ * Utility functions for XS code to wrap Kerberos data structures.
+ *
+ * Provides helper functions to wrap Kerberos data structures in internal
+ * representations so that we can easily generate appropriate Perl objects.
+ *
+ * Written by Russ Allbery <rra@cpan.org>
+ * Copyright 2014
+ *     The Board of Trustees of the Leland Stanford Junior University
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#include <EXTERN.h>
+#include <perl.h>
+#include <XSUB.h>
+
+#include <portable/krb5.h>
+
+#include <util/util.h>
+
+
+/*
+ * Given a krb5_principal, convert it to an Authen::Kerberos::Principal
+ * object.  Make a copy of the principal so that it can be stored and used
+ * independently of whatever object we generated it from.  Takes both the
+ * context and the underlying Authen::Kerberos object so that we can stash the
+ * latter in the created struct.
+ */
+Authen__Kerberos__Principal
+krb5_wrap_principal(krb5_context ctx, SV *krb5, krb5_principal princ)
+{
+    krb5_principal copy;
+    krb5_error_code code;
+    Authen__Kerberos__Principal principal;
+
+    code = krb5_copy_principal(ctx, princ, &copy);
+    if (code != 0)
+        krb5_croak(ctx, code, "krb5_copy_principal", FALSE);
+    principal = malloc(sizeof(*principal));
+    if (principal == NULL)
+        croak("cannot allocate memory");
+    principal->ctx = krb5;
+    SvREFCNT_inc_simple_void_NN(principal->ctx);
+    principal->principal = copy;
+    return principal;
+}