[RFC][PATCH] krb5 => libverto main loop

[Greg Hudson]

On Tue, 2011-08-23 at 15:58 -0400, Nathaniel McCallum wrote:
> > at some point the code should change
> > to not set up the routing socket if IPv4 and IPv6 pktinfo [...]

> If we make IPv6 non-conditional that should essentially fix this
> problem, right?

No; IPv4 pktinfo support is still pretty sparse, so we'll still want to
conditionalize setting up the routing socket on what happened during
network setup (or on the same set of preprocessor symbols).

> I did not see either of these... Perhaps my grep-fu isn't as good as
> yours?

I think they were mostly in the worker process code which has been
reworked.

In my review of the patches so far:

* I don't understand patch 4.  In the KDC (in patch 5) you set up a
SIGHUP handler to preserve the behavior that a SIGHUP reopens the
logfile, but in kadmind you appear to have eliminated this behavior,
which seems wrong.

* The new signal handling for the worker process monitor isn't
completely correct; using a single variable (signal_received) for HUP
and termination signals could cause one or the other to get lost before
it is processed.  There's also a race where two precisely timed HUPs
could incorrectly abort the monitor loop.  Also, variables like
signal_received should be volatile.

* kadmind should set up the routing socket but doesn't.  The KDC sets up
the routing socket exactly when it shouldn't (when it's running as a
worker process) and not when it should.

* The SIGHUP handler in kdc/main.c doesn't need to call
get_context(NULL); it can just refer to kdc_context.

* routing_update_needed has a cuddled opening brace.

Also, in libverto itself (some of these have been raised before; just
making sure they don't get lost):

* On Unix platforms, pdlmsuffix is hardcoded to ".so", which isn't
correct on all platforms.  HPUX uses .sl, Darwin uses .dylib, and AIX
uses .a.  (We don't try terribly hard to support HPUX and AIX, but we do
have shared library linking support for those platforms, and the Darwin
issue is significant.)

* Some prototypes in the headers aren't actually prototypes because they
don't use (void):

        verto-glib.h:verto_ctx *verto_new_glib();
        verto-glib.h:verto_ctx *verto_default_glib();
        verto-libevent.h:verto_ctx *verto_new_libevent();
        verto-libevent.h:verto_ctx *verto_default_libevent();
        verto-libev.h:verto_ctx *verto_new_libev();
        verto-libev.h:verto_ctx *verto_default_libev();
        verto-module.h:typedef verto_ctx *(*verto_ctx_constructor)();
        verto-tevent.h:verto_ctx *verto_new_tevent();
        verto-tevent.h:verto_ctx *verto_default_tevent();

* The pkg-config input files need to contain "exec_prefix=@exec_prefix@"
or they won't work.

* I believe it would be normal practice to include @LIBS@ in the
libverto pkg-config input file so that -ldl is included.  Otherwise,
static linking against libverto using the pkg-config output will fail.
(An unfortunate side effect of including -ldl there is that every
libverto consumer gets a direct dependency on libdl whether it uses
dlopen or not; this is a long-standing problem with no clear resolution
as far as I know.)
h