Coding style and other cleanups for Ruby bindings

Correct the README somewhat on terminology and on which exceptions are
thrown.  Remove (required) notation for arguments when all arguments are
required.  Standardize (hopefully) on correct Ruby notation for methods.

Remove gcc warning options from the CFLAGS in the Ruby extension.  We'll
deal with that another way.

Declare all functions except Init_remctl static and rename Init_remctl
to Init_Remctl to match the name of the class.  Handle a few more places
out of memory errors are possible.

3 files changed, 272 insertions, 208 deletions

diff --git a/ruby/README b/ruby/README
index ff50dc3..7c7fc48 100644
--- a/ruby/README
+++ b/ruby/README
@@ -8,13 +8,14 @@ OVERVIEW
 
   This module provides two interfaces: a simple interface in Ruby that
   performs a single call to a remctl server and returns the result as an
-  object; and a full interface in Ruby which provides more control over the
-  connection, returns individual output tokens, and allows multiple commands
-  to be sent via the same connection.
+  object; and a full interface in Ruby which provides more control over
+  the connection, returns individual output tokens, and allows multiple
+  commands to be sent via the same connection.
 
 REQUIREMENTS
 
-  The module has been tested with Ruby 1.8 and Remctl 2.15.
+  The module has been tested with Ruby 1.8 and may not work with older
+  (or newer) versions of Ruby.
 
 SIMPLIFIED INTERFACE
 
@@ -23,16 +24,16 @@ SIMPLIFIED INTERFACE
       the results.
 
       Arguments:
-      * host (required): string, the host to connect to
-      * command (required): array containing strings making up the command
+      * host: string, the host to connect to
+      * args: array containing strings making up the command
 
       In order to change the port or principal used, assign to
-      Remctl.default_port and Remctl.default_principal. These default to 0
-      and nil, meaning to respect the library defaults: port 4373
-      then fall back to attempting the legacy 4444 port, and a principal of
+      Remctl.default_port and Remctl.default_principal.  These default to
+      0 and nil, meaning to respect the library defaults: port 4373 then
+      fall back to attempting the legacy 4444 port, and a principal of
       host/<host>, with the realm determined by the domain-realm mapping.
 
-      command must be an array of things which respond to #to_s.
+      args must be an array of things which respond to #to_s.
 
       Returns an object of type Remctl::Result with the following
       attributes:
@@ -41,10 +42,11 @@ SIMPLIFIED INTERFACE
       * status: integer, the exit status of the command
 
       Exceptions:
-      * ArgumentError, TypeError: an invalid argument was supplied
+      * ArgError, TypeError: an invalid argument was supplied
+      * NoMemError: memory allocation failed while making the call
       * Remctl::Error: an error occurred in the remctl communication
 
-      The message attribute (or string value) of the RemctlProtocolError
+      The message attribute (or string value) of the Remctl::Error
       exception contains the string returned by the remctl library.  This
       may be an internal error (such as a network error) or an error
       returned by the remote server (such as an unknown command error).
@@ -64,75 +66,76 @@ FULL INTERFACE
 
   The full remctl interface requires the user to do more bookkeeping, but
   provides more flexibility and visibility into what is happening at a
-  protocol level.  It allows issuing multiple commands on the same persistent
-  connection (provided that the remote server supports protocol version two;
-  if it doesn't, the library will transparently fall back to opening a
-  connection for each command).
+  protocol level.  It allows issuing multiple commands on the same
+  persistent connection (provided that the remote server supports protocol
+  version two; if it doesn't, the library will transparently fall back to
+  opening a connection for each command).
 
-  To use the full interface, first create a connection object with Remctl.new
-  and then call the Remctl#command method to issue a command.  Read output
-  tokens with Remctl#output until a status token has been received.  Then the
-  command is complete and another command can be issued.
+  To use the full interface, first create a connection object with
+  Remctl.new and then call the #command method to issue a command.  Read
+  output tokens with #output until a status token has been received.  Then
+  the command is complete and another command can be issued.
 
   Remctl.new(host, port, principal)
       The constructor.  Create a new Remctl object.  The host argument is
-      required; port and principal default to Remctl.default_port and
+      required.  port and principal default to Remctl.default_port and
       Remctl.default_principal respectively.
 
       Arguments:
       * host (required): string, the host to connect to
       * port (optional): unsigned short, the port to connect to
       * principal (optional): string, authentication identity of host
-      
-      This method immediately connects to the server.  As such it raises
-      ArgumentError in the event that the port is out of range, and
-      Remctl::Error in case of a protocol error.
+
+      This method immediately connects to the server.  It raises ArgError
+      in the event that the port is out of range and Remctl::Error in case
+      of a protocol error.
 
       If port is not given, the library default is used (try 4373 first
       and fall back to attempting the legacy 4444 port).  If principal is
       not given, the library default (host/<host> with the realm
-      determined by the domain-realm mapping) is used.      
-      
+      determined by the domain-realm mapping) is used.
+
       Exceptions:
-      * ArgumentError, TypeError: an invalid argument was supplied
+      * ArgError, TypeError: an invalid argument was supplied
+      * NoMemError: memory allocation failed while allocating the object
       * Remctl::Error: a network or authentication error occurred
 
-      The message attribute (or string value) of the Remctl::Error exception
-      contains the string returned by the remctl library remctl_error()
-      function.
+      The message attribute (or string value) of the Remctl::Error
+      exception contains the string returned by the remctl library
+      remctl_error() function.
 
   All further methods below must be called on a Remctl object as returned
   or yielded by the constructor.
 
-  Remctl.command(*args)
+  r.command(*args)
       Send a command to the remote host.  There is no return value; an
       exception is thrown on any error.
 
       The Remctl object must already be connected.  The command may, under
       the remctl protocol, contain any character, but be aware that most
       remctl servers will reject commands or arguments containing ASCII 0
-      (NUL).  This currently therefore cannot be used for upload of
-      arbitrary unencoded binary data.
+      (NUL) unless that argument is configured on the server to be passed
+      via standard input.
 
       Arguments:
-      * args (required): An array of arguments. Each must be convertible to
-        a string.
+      * args (required): An array of arguments.  Each must respond to
+        #to_s.
 
       Exceptions:
       * TypeError: an invalid argument was supplied
       * Remctl::Error: a network or authentication error occurred
       * Remctl::NotOpen: no connection currently open
 
-      The message attribute (or string value) of the Remctl::Error exception
-      contains the string returned by the remctl library.
+      The message attribute (or string value) of the Remctl::Error
+      exception contains the string returned by the remctl library.
 
-  remctl.output()
+  r.output()
       Reads an output token from the server and returns it as an array.  A
       command will result in either one error token or zero or more output
-      tokens followed by a status token.  The output is complete as soon as
-      any token other than an output token has been received, but the module
-      will keep returning done tokens to the caller for as long as
-      Remctl#output is called without another call to Remctl#command.
+      tokens followed by a status token.  The output is complete as soon
+      as any token other than an output token has been received, but the
+      module will keep returning done tokens to the caller for as long as
+      #output is called without another call to #command.
 
       The members of the returned array are, in order:
       * type: symbol, :output, :status, :error, or :done
@@ -142,10 +145,10 @@ FULL INTERFACE
       * error: integer, remctl protocol error for an error token
 
       The array always has 5 elements.  The other members of the array may
-      be nil depending on the type of token.  Output tokens will have output
-      and stream information, error tokens will have output and error
-      information, and status tokens will have status information.  Done
-      tokens will return nil for all other elements.
+      be nil depending on the type of token.  Output tokens will have
+      output and stream information, error tokens will have output and
+      error information, and status tokens will have status information.
+      Done tokens will return nil for all other elements.
 
       For error tokens, error holds the numeric error code (see the remctl
       protocol specification), which is the recommended value for programs
@@ -156,17 +159,22 @@ FULL INTERFACE
       Exceptions:
       * Remctl::NotOpen: no connection currently open
 
-  remctl.close()
-      Close a connection.  After calling this method, Remctl#reopen must be
-      called for this object before sending any further commands.  The
+  r.close()
+      Close a connection.  After calling this method, #reopen must
+      be called for this object before sending any further commands.  The
       connection will also be automatically closed when the object is
       destroyed, so calling this method is often not necessary.
 
-  remctl.reopen
+  r.reopen()
       Close the connection, reopen it, and authenticate.  Returns self.
       Raises Remctl::Error in the event that remctl_open() fails.
 
+      Exceptions:
+      * NoMemError: memory allocation failed while making the call
+      * Remctl::Error: a network or authentication error occurred
 
 HISTORY
 
-  Written by Anthony Martinez <twopir@nmt.edu> in 2010.
+  The original implementation was written by Anthony Martinez
+  <twopir@nmt.edu> in 2010.  As of remctl 2.16 it is part of the stock
+  remctl distribution.
diff --git a/ruby/extconf.rb.in b/ruby/extconf.rb.in
index eaf09fb..dba8cc0 100644
--- a/ruby/extconf.rb.in
+++ b/ruby/extconf.rb.in
@@ -1,14 +1,23 @@
-# I'm pretty sure this is an ugly hack.
+# Extension setup for libremctl Ruby bindings.  -*- ruby -*-
+#
+# Original implementation by Anthony M. Martinez <twopir@nmt.edu>
+# Some subsequent updates by Russ Allbery <rra@stanford.edu>
+# Copyright 2010 Anthony M. Martinez <twopir@nmt.edu>
+# Copyright 2010 Board of Trustees, Leland Stanford Jr. University
+#
+# See LICENSE for licensing terms.
+
+# This hack is to force the extension to build against the local client
+# library rather than some other library already installed on the system.
 require 'rbconfig'
-old_libdir = Config::CONFIG["libdir"]
+old_libdir = Config::CONFIG['libdir']
 require 'mkmf'
-Config::MAKEFILE_CONFIG["libdir"] = "@abs_top_srcdir@/client/.libs"
-
-Extension = "remctl"
+Config::MAKEFILE_CONFIG['libdir'] = '@abs_top_srcdir@/client/.libs'
 
-$CFLAGS  << " -I@abs_top_srcdir@/client @CFLAGS@ -Wall -Wpointer-arith"
+$CFLAGS  << ' -I@abs_top_srcdir@ @CFLAGS@'
 $LDFLAGS << " -L@abs_top_srcdir@/client/.libs -L#{old_libdir}"
 
-fail "Couldn't find libremctl" unless have_library("remctl", "remctl_open", "remctl.h")
+fail "Couldn't find libremctl"\
+    unless have_library('remctl', 'remctl_open', 'remctl.h')
 
-create_makefile Extension
+create_makefile('Remctl')
diff --git a/ruby/remctl.c b/ruby/remctl.c
index f2ac689..70155b6 100644
--- a/ruby/remctl.c
+++ b/ruby/remctl.c
@@ -1,10 +1,12 @@
 /*
- * An interface between Ruby and libremctl. Implements both the simple and
- * complex forms of the API.
+ * Ruby interface to remctl.
  *
- * Original implementation by Anthony M. Martinez <twopir@nmt.edu>
+ * Converts the libremctl C API into a Ruby interface.  Implements both the
+ * simple and complex forms of the API.
  *
+ * Original implementation by Anthony M. Martinez <twopir@nmt.edu>
  * Copyright 2010 Anthony M. Martinez <twopir@nmt.edu>
+ * Copyright 2010 Board of Trustees, Leland Stanford Jr. University
  *
  * Permission to use, copy, modify, and distribute this software and its
  * documentation for any purpose and without fee is hereby granted, provided
@@ -21,21 +23,21 @@
  * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE.
  */
 
-#include <ruby.h>
+#include <config.h>
+#include <portable/system.h>
 
 #include <errno.h>
-#include <stdlib.h>
-#include <string.h>
-#include <unistd.h>
+#include <ruby.h>
 #include <sys/uio.h>
 
-#include <remctl.h>
+#include <client/remctl.h>
 
 static VALUE cRemctl, cRemctlResult, eRemctlError, eRemctlNotOpen;
+
 /* Since we can't have @ in our names here... */
 static ID AAdefault_port, AAdefault_principal, Ahost, Aport, Aprincipal;
 
-/* Stolen from the Python */
+/* Map the remctl_output type constants to strings. */
 const struct {
     enum remctl_output_type type;
     const char *name;
@@ -47,51 +49,76 @@ const struct {
     { 0,                 NULL     }
 };
 
-#define GET_REMCTL_OR_RAISE(obj, var) do { \
-    Data_Get_Struct(obj, struct remctl, var); \
-    if(var == NULL) \
-        rb_raise(eRemctlNotOpen, "Connection is no longer open."); \
+/*
+ * Used for the complex interface when a method that requires the remctl
+ * connection be open is called.  Retrieves the underlying struct remctl
+ * object and raises an exception if it is NULL.
+ */
+#define GET_REMCTL_OR_RAISE(obj, var)                                   \
+    do {                                                                \
+        Data_Get_Struct((obj), struct remctl, (var));                   \
+        if ((var) == NULL)                                              \
+            rb_raise(eRemctlNotOpen, "Connection is no longer open.");  \
     } while(0)
 
-/* Given a remctl_result pointer, return a Ruby Remctl::Result, unless the
- * remctl call had an error, in which case raise a Remctl::Error
+
+/*
+ * Given a remctl_result pointer, return a Ruby Remctl::Result, unless the
+ * remctl call had an error, in which case raise a Remctl::Error.  This is
+ * used only internally by the extension and isn't exported as a method.
  */
-VALUE
+static VALUE
 rb_remctl_result_new(struct remctl_result *rr)
 {
-    VALUE result = rb_class_new_instance(0, NULL, cRemctlResult);
-    if(rr->error)
-        rb_raise(eRemctlError, "%s", rr->error);
+    VALUE result;
 
+    if (rr->error)
+        rb_raise(eRemctlError, "%s", rr->error);
+    result = rb_class_new_instance(0, NULL, cRemctlResult);
     rb_iv_set(result, "@stderr", rb_str_new(rr->stderr_buf, rr->stderr_len));
     rb_iv_set(result, "@stdout", rb_str_new(rr->stdout_buf, rr->stdout_len));
     rb_iv_set(result, "@status", INT2FIX(rr->status));
-
     remctl_result_free(rr);
     return result;
 }
 
+
+/* call-seq:
+ * Remctl::Result.new() -> #&lt;Remctl::Result&gt;
+ *
+ * Initialize a Remctl::Result object.
+ */
+static VALUE
+rb_remctl_result_initialize(VALUE self)
+{
+    rb_define_attr(cRemctlResult, "stderr", 1, 0);
+    rb_define_attr(cRemctlResult, "stdout", 1, 0);
+    rb_define_attr(cRemctlResult, "status", 1, 0);
+    return Qtrue;
+}
+
+
 /* call-seq:
  * Remctl.remctl(host, *args)   -> Remctl::Result
  *
  * Place a single Remctl call to the host, using the current default port and
  * principal.
  */
-VALUE
+static VALUE
 rb_remctl_remctl(int argc, VALUE argv[], VALUE self)
 {
     VALUE vhost, vport, vprinc, vargs, tmp;
     unsigned int port;
     char *host, *princ;
     const char **args;
-    struct remctl_result *rc_res;
+    struct remctl_result *result;
     int i, rc_argc;
 
-    /* Take the port and princ from the class instead of demanding that the
+    /*
+     * Take the port and princ from the class instead of demanding that the
      * user specify "nil, nil" so often.
      */
     rb_scan_args(argc, argv, "1*", &vhost, &vargs);
-
     host   = StringValuePtr(vhost);
     vport  = rb_cvar_get(cRemctl, AAdefault_port);
     vprinc = rb_cvar_get(cRemctl, AAdefault_principal);
@@ -100,226 +127,242 @@ rb_remctl_remctl(int argc, VALUE argv[], VALUE self)
 
     /* Convert the remaining arguments to their underlying pointers. */
     rc_argc = RARRAY_LEN(vargs);
-    args = ALLOC_N(const char *, rc_argc+1);
-    for(i = 0; i < rc_argc; i++) {
+    args = ALLOC_N(const char *, rc_argc + 1);
+    for (i = 0; i < rc_argc; i++) {
         tmp = rb_ary_entry(vargs, i);
         args[i] = StringValuePtr(tmp);
     }
     args[rc_argc] = NULL;
 
-    rc_res = remctl(host, port, princ, args);
-    if (rc_res == NULL)
+    /* Make the actual call. */
+    result = remctl(host, port, princ, args);
+    if (result == NULL)
         rb_raise(rb_eNoMemError, "remctl");
-
-    return rb_remctl_result_new(rc_res);
+    return rb_remctl_result_new(result);
 }
 
-VALUE
-rb_remctl_result_initialize(VALUE self)
-{
-    rb_define_attr(cRemctlResult, "stderr", 1, 0);
-    rb_define_attr(cRemctlResult, "stdout", 1, 0);
-    rb_define_attr(cRemctlResult, "status", 1, 0);
-    return Qtrue;
-}
 
 /* call-seq:
  * Remctl.default_port  -> 0
  *
  * Return the default port used for a Remctl simple connection or new instance
- * of a complex connection. A value of 0 indicates the default port.
+ * of a complex connection.  A value of 0 indicates the default port.
  */
-VALUE
+static VALUE
 rb_remctl_default_port_get(VALUE self)
 {
     return rb_cvar_get(cRemctl, AAdefault_port);
 }
 
+
 /* call-seq:
  * Remctl.default_port = 0       -> 0
  * Remctl.default_port = 2617    -> 2617
- * Remctl.default_port = 65539   -> ArgumentError
+ * Remctl.default_port = 65539   -> ArgError
  *
  * Change the default port used for a Remctl simple connection or new instance
- * of a complex connection. A value of 0 indicates the default port. Raises
- * +ArgumentError+ if the port number isn't sane.
+ * of a complex connection.  A value of 0 indicates the default port.  Raises
+ * +ArgError+ if the port number isn't sane.
  */
-VALUE
+static VALUE
 rb_remctl_default_port_set(VALUE self, VALUE new)
 {
-    unsigned int port = FIX2UINT(new);
-    if(port > 65535)
+    unsigned int port;
+
+    port = FIX2UINT(new);
+    if (port > 65535)
         rb_raise(rb_eArgError, "Port number %u out of range", port);
     else
         rb_cvar_set(cRemctl, AAdefault_port, new, 0);
     return rb_cvar_get(cRemctl, AAdefault_port);
 }
 
+
 /* call-seq:
  * Remctl.default_principal  -> nil
  *
  * Return the default principal used for a Remctl simple connection or new
  * instance of a complex connection.
  */
-VALUE
+static VALUE
 rb_remctl_default_principal_get(VALUE self)
 {
     return rb_cvar_get(cRemctl, AAdefault_principal);
 }
 
+
 /* call-seq:
  * Remctl.default_principal = "root@REALM"  -> "root@REALM"
  *
- * Set the principal used by default for a Remctl simple connection. A value of
- * nil requests the library default.
+ * Set the principal used by default for a Remctl simple connection.  A value
+ * of nil requests the library default.
  */
-VALUE
+static VALUE
 rb_remctl_default_principal_set(VALUE self, VALUE new)
 {
     rb_cvar_set(cRemctl, AAdefault_principal, StringValue(new), 0);
     return rb_cvar_get(cRemctl, AAdefault_principal);
 }
 
-void
-rb_remctl_destroy(void *rc)
+
+/*
+ * Destructor for a Remctl object.  Closes the connection and frees the
+ * underlying memory.
+ */
+static void
+rb_remctl_destroy(void *r)
 {
-    if(rc)
-        remctl_close(rc);
+    if (r != NULL)
+        remctl_close(r);
 }
 
-VALUE
+
+/*
+ * Alloate a new object of the Remctl class and register it with the
+ * destructor.
+ */
+static VALUE
 rb_remctl_alloc(VALUE klass)
 {
-    struct remctl *rem = NULL;
-    return Data_Wrap_Struct(klass, NULL, rb_remctl_destroy, rem);
+    struct remctl *r = NULL;
+
+    return Data_Wrap_Struct(klass, NULL, rb_remctl_destroy, r);
 }
 
+
 /* call-seq:
- * rc.close  -> nil
+ * r.close  -> nil
  *
- * Close a Remctl connection. Any further operations (besides reopen) on the
+ * Close a Remctl connection.  Any further operations (besides reopen) on the
  * object will raise Remctl::NotOpen.
  */
-VALUE
+static VALUE
 rb_remctl_close(VALUE self)
 {
-    struct remctl *rem;
+    struct remctl *r;
 
-    GET_REMCTL_OR_RAISE(self, rem);
-    remctl_close(rem);
+    GET_REMCTL_OR_RAISE(self, r);
+    remctl_close(r);
     DATA_PTR(self) = NULL;
     return Qnil;
 }
 
-/* Reopen a Remctl connection to the stored host, port, and principal.
+
+/* call-seq:
+ * r.reopen  -> nil
  *
- * Raises Remctl::Error if the connection fails.
+ * Reopen a Remctl connection to the stored host, port, and principal.  Raises
+ * Remctl::Error if the connection fails.
  */
-VALUE
+static VALUE
 rb_remctl_reopen(VALUE self)
 {
-    struct remctl *rc;
+    struct remctl *r;
     VALUE vhost, vport, vprinc;
     char *host, *princ;
     unsigned int port;
 
-    Data_Get_Struct(self, struct remctl, rc);
-    if(rc != NULL) {
-        remctl_close(rc);
-    }
-    rc = remctl_new();
+    Data_Get_Struct(self, struct remctl, r);
+    if (r != NULL)
+        remctl_close(r);
+    r = remctl_new();
+    if (r == NULL)
+        rb_raise(rb_eNoMemError, "remctl");
 
-    /* This is kinda ugly :( */
+    /* Retrieve the stored host, port, and principal values. */
     vhost  = rb_ivar_get(self, Ahost);
     vport  = rb_ivar_get(self, Aport);
     vprinc = rb_ivar_get(self, Aprincipal);
+    host   = StringValuePtr(vhost);
+    port   = NIL_P(vport)  ? 0    : FIX2UINT(vport);
+    princ  = NIL_P(vprinc) ? NULL : StringValuePtr(vprinc);
 
-    host  = StringValuePtr(vhost);
-    port  = NIL_P(vport)  ? 0    : FIX2UINT(vport);
-    princ = NIL_P(vprinc) ? NULL : StringValuePtr(vprinc);
-
-    if(!remctl_open(rc, host, port, princ)) {
-        rb_raise(eRemctlError, "%s", rb_str_new2(remctl_error(rc)));
-    }
-    DATA_PTR(self) = rc;
+    /* Reopen the connection. */
+    if (!remctl_open(r, host, port, princ))
+        rb_raise(eRemctlError, "%s", rb_str_new2(remctl_error(r)));
+    DATA_PTR(self) = r;
     return self;
 }
 
-/* Call a remote command. Returns nil.
+
+/* call-seq:
+ * r.command(*args)  -> nil
+ *
+ * Call a remote command.  Returns nil.
  *
  * Raises Remctl::Error in the event of failure, and Remctl::NotOpen if the
  * connection has been closed.
  */
-VALUE
+static VALUE
 rb_remctl_command(int argc, VALUE argv[], VALUE self)
 {
-    struct remctl *rc;
+    struct remctl *r;
     struct iovec *iov;
     int i;
     VALUE s;
 
     GET_REMCTL_OR_RAISE(self, rc);
     iov = ALLOC_N(struct iovec, argc);
-    for(i = 0; i < argc; i++) {
+    for (i = 0; i < argc; i++) {
         s = StringValue(argv[i]);
         iov[i].iov_base = RSTRING_PTR(s);
         iov[i].iov_len  = RSTRING_LEN(s);
     }
-
-    if(!remctl_commandv(rc, iov, argc)) {
-        rb_raise(eRemctlError, "%s", rb_str_new2(remctl_error(rc)));
-    }
-
+    if (!remctl_commandv(rc, iov, argc))
+        rb_raise(eRemctlError, "%s", rb_str_new2(remctl_error(r)));
     return Qnil;
 }
 
-VALUE
-rb_remctl_type_intern(enum remctl_output_type t)
+
+/*
+ * Convert an enum remctl_output_type argument to a Ruby symbol.
+ */
+static VALUE
+rb_remctl_type_intern(enum remctl_output_type type)
 {
     int i;
-    for(i = 0; OUTPUT_TYPE[i].name != NULL; i++) {
-        if(OUTPUT_TYPE[i].type == t) {
+
+    for (i = 0; OUTPUT_TYPE[i].name != NULL; i++)
+        if (OUTPUT_TYPE[i].type == type)
             return ID2SYM(rb_intern(OUTPUT_TYPE[i].name));
-        }
-    }
-    rb_bug("Fell off the end while looking up remctl output type %d!\n", t);
+    rb_bug("Fell off the end while looking up remctl output type %d!\n", type);
 }
 
+
 /* call-seq:
  * rc.command -> [type, output, stream, status, error]
  *
- * Retrieve the output tokens from the remote command.
- *
- * Raises Remctl::Error in the event of failure, and Remctl::NotOpen if the
- * connection has been closed.
+ * Retrieve the output tokens from the remote command.  Raises Remctl::Error
+ * in the event of failure, and Remctl::NotOpen if the connection has been
+ * closed.
  */
-VALUE
+static VALUE
 rb_remctl_output(VALUE self)
 {
-    struct remctl *rc;
-    struct remctl_output *o;
-
-    GET_REMCTL_OR_RAISE(self, rc);
-    if((o = remctl_output(rc)) == NULL) {
-        rb_raise(eRemctlError, "%s", rb_str_new2(remctl_error(rc)));
-    }
-    return rb_ary_new3(5, rb_remctl_type_intern(o->type),
-                          rb_str_new(o->data, o->length),
-                          INT2FIX(o->stream),
-                          INT2FIX(o->status),
-                          INT2FIX(o->error));
+    struct remctl *r;
+    struct remctl_output *output;
+
+    GET_REMCTL_OR_RAISE(self, r);
+    output = remctl_output(r);
+    if (output == NULL)
+        rb_raise(eRemctlError, "%s", rb_str_new2(remctl_error(r)));
+    return rb_ary_new3(5, rb_remctl_type_intern(output->type),
+                       rb_str_new(output->data, output->length),
+                       INT2FIX(output->stream), INT2FIX(output->status),
+                       INT2FIX(output->error));
 }
 
+
 /* call-seq:
  * Remctl.new(host, port=Remctl.default_port, princ=Remctl.default_principal) -> #&lt;Remctl&gt;
- * Remctl.new(host, port, princ) {|rc| ...} -> nil
+ * Remctl.new(host, port, princ) {|r| ...} -> nil
  *
- * Create and open a new Remctl complex instance to +host+. Raises
- * ArgumentError if the port number is out of range, and Remctl::Error if the
- * underlying library raises one. With a block, yield the instance, and ensure
- * the connection is closed at block exit.
+ * Create and open a new Remctl complex instance to +host+.  Raises ArgError
+ * if the port number is out of range, and Remctl::Error if the underlying
+ * library raises one.  With a block, yield the instance, and ensure the
+ * connection is closed at block exit.
  */
-VALUE
+static VALUE
 rb_remctl_initialize(int argc, VALUE argv[], VALUE self)
 {
     VALUE vhost, vport, vprinc, vdefport, vdefprinc;
@@ -329,83 +372,87 @@ rb_remctl_initialize(int argc, VALUE argv[], VALUE self)
     rb_define_attr(cRemctl, "host", 1, 0);
     rb_define_attr(cRemctl, "port", 1, 0);
     rb_define_attr(cRemctl, "principal", 1, 0);
-
     vdefport  = rb_cvar_get(cRemctl, AAdefault_port);
     vdefprinc = rb_cvar_get(cRemctl, AAdefault_principal);
-
     rb_scan_args(argc, argv, "12", &vhost, &vport, &vprinc);
-
-    if(NIL_P(vport))  vport  = vdefport;
-    if(NIL_P(vprinc)) vprinc = vdefprinc;
-
+    if (NIL_P(vport))
+        vport  = vdefport;
+    if (NIL_P(vprinc))
+        vprinc = vdefprinc;
     host  = StringValuePtr(vhost);
     port  = NIL_P(vport)  ? 0    : FIX2UINT(vport);
     princ = NIL_P(vprinc) ? NULL : StringValuePtr(vprinc);
-
-    if(port > 65535) {
+    if (port > 65535)
         rb_raise(rb_eArgError, "Port number %u out of range", port);
-    }
 
-    /* Hold these in instance variables for reopen */
+    /* Hold these in instance variables for reopen. */
     rb_ivar_set(self, Ahost, vhost);
     rb_ivar_set(self, Aport, vport);
     rb_ivar_set(self, Aprincipal, vprinc);
     rb_remctl_reopen(self);
-    if(rb_block_given_p()) {
+    if (rb_block_given_p()) {
         rb_ensure(rb_yield, self, rb_remctl_close, self);
         return Qnil;
-    }
-    else {
+    } else {
         return self;
     }
 }
 
-/* Ruby interface to the Remctl library for remote Kerberized command
- * execution.
+
+/*
+ * Ruby interface to the Remctl library for remote Kerberized command
+ * execution.  This function does class setup and registers the methods and
+ * variables.
  */
 void
-Init_remctl()
+Init_Remctl(void)
 {
     cRemctl = rb_define_class("Remctl", rb_cObject);
     rb_define_singleton_method(cRemctl, "remctl", rb_remctl_remctl, -1);
 
+    /*
+     * Allocate string constants used to refer to our class and instance
+     * variables.
+     */
     AAdefault_port      = rb_intern("@@default_port");
     AAdefault_principal = rb_intern("@@default_principal");
+    Ahost               = rb_intern("@host");
+    Aport               = rb_intern("@port");
+    Aprincipal          = rb_intern("@principal");
 
-    Ahost      = rb_intern("@host");
-    Aport      = rb_intern("@port");
-    Aprincipal = rb_intern("@principal");
-
+    /* Default values for class variables. */
     rb_cvar_set(cRemctl, AAdefault_port, UINT2NUM(0), 0);
     rb_cvar_set(cRemctl, AAdefault_principal, Qnil, 0);
 
+    /* Getter and setter methods for class variables. */
     rb_define_singleton_method(cRemctl, "default_port",
-            rb_remctl_default_port_get, 0);
+                               rb_remctl_default_port_get, 0);
     rb_define_singleton_method(cRemctl, "default_port=",
-            rb_remctl_default_port_set, 1);
+                               rb_remctl_default_port_set, 1);
     rb_define_singleton_method(cRemctl, "default_principal",
-            rb_remctl_default_principal_get, 0);
+                               rb_remctl_default_principal_get, 0);
     rb_define_singleton_method(cRemctl, "default_principal=",
-            rb_remctl_default_principal_set, 1);
+                               rb_remctl_default_principal_set, 1);
 
+    /* Create the Remctl class. */
     rb_define_alloc_func(cRemctl, rb_remctl_alloc);
     rb_define_method(cRemctl, "initialize", rb_remctl_initialize, -1);
     rb_define_method(cRemctl, "close", rb_remctl_close, 0);
     rb_define_method(cRemctl, "reopen", rb_remctl_reopen, 0);
     rb_define_method(cRemctl, "command", rb_remctl_command, -1);
     rb_define_method(cRemctl, "output", rb_remctl_output, 0);
-    
+
     /* Document-class: Remctl::Result
      *
-     * Returned from a simple Remctl.remctl call. Attributes:
+     * Returned from a simple Remctl.remctl call.  Attributes:
      *
      * +stdout+:: String containing the returned standard output
      * +stderr+:: Same, but standard error.
      * +status+:: Fixnum of the command's exit status.
      */
     cRemctlResult = rb_define_class_under(cRemctl, "Result", rb_cObject);
-    rb_define_method(cRemctlResult, "initialize", rb_remctl_result_initialize,
-            0);
+    rb_define_method(cRemctlResult, "initialize",
+                     rb_remctl_result_initialize, 0);
 
     /* Document-class: Remctl::Error
      *