plaintext
Random ramblings in the order I go through things...

- I git-clone'd openvpn-testing.git, went to the svn-merger branch, and
  ran "make check" (on Gentoo Linux), with my full-featured t_client.rc
  setup.

  Test ran:
   - p2mp tun udp (ipv4 + ipv6 ok)
   - p2mp tun tcp (ipv4 + ipv6 ok)
   - p2mp tun udp, "topology subnet" (ipv4 + ipv6 ok)
   - p2mp tap udp (ipv4 ok, ipv6 fails, known issue with IPv6 auto-conf
                   on TAP, not related to the svn-merger)

  so the client code (at least) is still working as well as my tests
  cover the code.

- code alignment needed: for IPv4, the "did_redirect_default_gateway"
  and "spec.remote_endpoint_defined" have been converted to flag bits in
  route_list-iflags, but for IPv6, the old structure elements
  remain - so to make the code more "in-line" for IPv4 and IPv6, this
  needs code adjustments in the IPv6 code.

- I'm not overly happy about the "default-gateway block-local" changes -
  this is less a code issue (the code might be fine) but a procedural
  issue, with a huge change to route.c coming in without any sort of
  review or discussion.  Gah.  (No response needed).

- I'm somewhat more annoyed by this one (route.c, line 1280):

#if defined(TARGET_LINUX)
#ifdef CONFIG_FEATURE_IPROUTE
  /* FIXME -- add LR_MATCH support for CONFIG_FEATURE_IPROUTE */

  this is implemented only for the "non-iproute2" case, so we have
  differing behaviour for iproute2/non-iproute2 compiles now.
  This MUST be fixed for 2.3

- implementations of LR_MATCH for most other platforms are missing,
  but this is something that can be documented in the release notes,
  and if someone thinks they need this, they can add it - but having
  support-or-not for Linux, depending on --enable-iproute2, is a no-go

- the merger has PF_INET6 blocks, and I currently don't run tests over
  IPv6 transport.  So maybe jjo could also take a look at this branch
  and see whether his stuff is still working.

- the web view of route.c, "-887,13 - +894,10" looks a bit weird,
  with "++add_routes (...", but the code in the branch is fine.

- there's a functional and potentially-fatal change here:

void
-delete_routes (struct route_list *rl, const struct tuntap *tt, unsigned int flags, the struct env_set *es)
+delete_routes (struct route_list *rl, struct route_ipv6_list *rl6,
+               const struct tuntap *tt, unsigned int flags, the struct env_set *es)
{
  - if (rl &&  rl-routes_added)
  + if (rl-iflags &  RL_ROUTES_ADDED)

  this new code does not check whether "rl" is non-NULL, but in theory
  it could very well be NULL if we only have IPv6 routes.

  So route.c line 1034 should really be:

    if ( rl &&  rl-iflags &  RL_ROUTES_ADDED)

  and the corresponding code in add_routes (route.c, line 990) should
  read:

    if ( rl &&  !(rl-iflags &  RL_ROUTES_ADDED))

  ... enhancing my tests to add a "--route-nopull --route-ipv6 test"...
  and indeed:

./t_client.sh: Zeile 200:  8787 Speicherzugriffsfehler  ./openvpn $openvpn_conf  $LOGDIR/$SUF:openvpn.log

  *bang*

redirect_default_route_to_vpn (rl=0x0, rl6=0x80f801c, tt=0x8101f90, flags=0,
   es=0x80dc860) at route.c:811
811   if (rl-flags &  RG_ENABLE)

  a proper patch is attached...

- ssl.c: it would be useful if andj or d12fk could review that - I'm not
  actually sure I understand what changed, but it seems to be some
  shuffling around of code and #ifdef ENABLE_CLIENT_CR, without actually
  changing much.

the rest looks ok-ish to me...  (but yes, I can understand that it took
you a heroic effort to merge that).

plaintext
Ok, I can't find anything horrifying in those patches. By just looking at them (that's only ssl.c and ssl.h).