krb5 commit: Trace log with a subset of struct conn_state

Greg Hudson ghudson at MIT.EDU
Fri Apr 12 13:24:28 EDT 2013


https://github.com/krb5/krb5/commit/e4de1f768e818911e6c035ddcbaf0127e0eccb40
commit e4de1f768e818911e6c035ddcbaf0127e0eccb40
Author: Greg Hudson <ghudson at mit.edu>
Date:   Wed Apr 10 18:10:52 2013 -0400

    Trace log with a subset of struct conn_state
    
    In struct conn_state, collect together the fields for the remote
    address and put them in a substructure.  Pass this substructure to
    trace logging macros instead of the entire conn_state structure, so
    that trace.c doesn't have to know about the whole structure.

 src/include/cm.h             |   11 +++++---
 src/include/k5-trace.h       |   54 ++++++++++++++++++++--------------------
 src/lib/krb5/os/sendto_kdc.c |   55 ++++++++++++++++++++++-------------------
 src/lib/krb5/os/t_trace.c    |   34 +++++++++++++-------------
 src/lib/krb5/os/t_trace.ref  |    8 +++---
 src/lib/krb5/os/trace.c      |   18 +++++++-------
 6 files changed, 93 insertions(+), 87 deletions(-)

diff --git a/src/include/cm.h b/src/include/cm.h
index d9c23fc..837a549 100644
--- a/src/include/cm.h
+++ b/src/include/cm.h
@@ -63,6 +63,12 @@ struct incoming_krb5_message {
     unsigned char bufsizebytes[4];
     size_t n_left;
 };
+struct remote_address {
+    int family;
+    int type;
+    socklen_t len;
+    struct sockaddr_storage saddr;
+};
 struct conn_state {
     SOCKET fd;
     krb5_error_code err;
@@ -70,10 +76,7 @@ struct conn_state {
     unsigned int is_udp : 1;
     int (*service)(krb5_context context, struct conn_state *,
                    struct select_state *, int);
-    int socktype;
-    int family;
-    size_t addrlen;
-    struct sockaddr_storage addr;
+    struct remote_address addr;
     struct {
         struct {
             sg_buf sgbuf[2];
diff --git a/src/include/k5-trace.h b/src/include/k5-trace.h
index 3b539b5..70151c6 100644
--- a/src/include/k5-trace.h
+++ b/src/include/k5-trace.h
@@ -60,7 +60,7 @@
  *   {lenstr}      size_t and const char *, as a counted string
  *   {hexlenstr}   size_t and const char *, as hex bytes
  *   {hashlenstr}  size_t and const char *, as four-character hex hash
- *   {connstate}   struct conn_state *, show socket type, address, port
+ *   {raddr}       struct remote_address *, show socket type, address, port
  *   {data}        krb5_data *, display as counted string
  *   {hexdata}     krb5_data *, display as hex bytes
  *   {errno}       int, display as number/errorstring
@@ -312,32 +312,32 @@ void krb5int_trace(krb5_context context, const char *fmt, ...);
     TRACE(c, "Response was{str} from master KDC", (master) ? "" : " not")
 #define TRACE_SENDTO_KDC_RESOLVING(c, hostname)         \
     TRACE(c, "Resolving hostname {str}", hostname)
-#define TRACE_SENDTO_KDC_RESPONSE(c, conn)              \
-    TRACE(c, "Received answer from {connstate}", conn)
-#define TRACE_SENDTO_KDC_TCP_CONNECT(c, conn)                   \
-    TRACE(c, "Initiating TCP connection to {connstate}", conn)
-#define TRACE_SENDTO_KDC_TCP_DISCONNECT(c, conn)                \
-    TRACE(c, "Terminating TCP connection to {connstate}", conn)
-#define TRACE_SENDTO_KDC_TCP_ERROR_CONNECT(c, conn, err)                \
-    TRACE(c, "TCP error connecting to {connstate}: {errno}", conn, err)
-#define TRACE_SENDTO_KDC_TCP_ERROR_RECV(c, conn, err)                   \
-    TRACE(c, "TCP error receiving from {connstate}: {errno}", conn, err)
-#define TRACE_SENDTO_KDC_TCP_ERROR_RECV_LEN(c, conn, err)               \
-    TRACE(c, "TCP error receiving from {connstate}: {errno}", conn, err)
-#define TRACE_SENDTO_KDC_TCP_ERROR_SEND(c, conn, err)                   \
-    TRACE(c, "TCP error sending to {connstate}: {errno}", conn, err)
-#define TRACE_SENDTO_KDC_TCP_SEND(c, conn)                      \
-    TRACE(c, "Sending TCP request to {connstate}", conn)
-#define TRACE_SENDTO_KDC_UDP_ERROR_RECV(c, conn, err)                   \
-    TRACE(c, "UDP error receiving from {connstate}: {errno}", conn, err)
-#define TRACE_SENDTO_KDC_UDP_ERROR_SEND_INITIAL(c, conn, err)           \
-    TRACE(c, "UDP error sending to {connstate}: {errno}", conn, err)
-#define TRACE_SENDTO_KDC_UDP_ERROR_SEND_RETRY(c, conn, err)             \
-    TRACE(c, "UDP error sending to {connstate}: {errno}", conn, err)
-#define TRACE_SENDTO_KDC_UDP_SEND_INITIAL(c, conn)                      \
-    TRACE(c, "Sending initial UDP request to {connstate}", conn)
-#define TRACE_SENDTO_KDC_UDP_SEND_RETRY(c, conn)                \
-    TRACE(c, "Sending retry UDP request to {connstate}", conn)
+#define TRACE_SENDTO_KDC_RESPONSE(c, raddr)             \
+    TRACE(c, "Received answer from {raddr}", raddr)
+#define TRACE_SENDTO_KDC_TCP_CONNECT(c, raddr)                  \
+    TRACE(c, "Initiating TCP connection to {raddr}", raddr)
+#define TRACE_SENDTO_KDC_TCP_DISCONNECT(c, raddr)               \
+    TRACE(c, "Terminating TCP connection to {raddr}", raddr)
+#define TRACE_SENDTO_KDC_TCP_ERROR_CONNECT(c, raddr, err)               \
+    TRACE(c, "TCP error connecting to {raddr}: {errno}", raddr, err)
+#define TRACE_SENDTO_KDC_TCP_ERROR_RECV(c, raddr, err)                  \
+    TRACE(c, "TCP error receiving from {raddr}: {errno}", raddr, err)
+#define TRACE_SENDTO_KDC_TCP_ERROR_RECV_LEN(c, raddr, err)              \
+    TRACE(c, "TCP error receiving from {raddr}: {errno}", raddr, err)
+#define TRACE_SENDTO_KDC_TCP_ERROR_SEND(c, raddr, err)                  \
+    TRACE(c, "TCP error sending to {raddr}: {errno}", raddr, err)
+#define TRACE_SENDTO_KDC_TCP_SEND(c, raddr)             \
+    TRACE(c, "Sending TCP request to {raddr}", raddr)
+#define TRACE_SENDTO_KDC_UDP_ERROR_RECV(c, raddr, err)                  \
+    TRACE(c, "UDP error receiving from {raddr}: {errno}", raddr, err)
+#define TRACE_SENDTO_KDC_UDP_ERROR_SEND_INITIAL(c, raddr, err)          \
+    TRACE(c, "UDP error sending to {raddr}: {errno}", raddr, err)
+#define TRACE_SENDTO_KDC_UDP_ERROR_SEND_RETRY(c, raddr, err)            \
+    TRACE(c, "UDP error sending to {raddr}: {errno}", raddr, err)
+#define TRACE_SENDTO_KDC_UDP_SEND_INITIAL(c, raddr)             \
+    TRACE(c, "Sending initial UDP request to {raddr}", raddr)
+#define TRACE_SENDTO_KDC_UDP_SEND_RETRY(c, raddr)               \
+    TRACE(c, "Sending retry UDP request to {raddr}", raddr)
 
 #define TRACE_SEND_TGS_ETYPES(c, etypes)                                \
     TRACE(c, "etypes requested in TGS request: {etypes}", etypes)
diff --git a/src/lib/krb5/os/sendto_kdc.c b/src/lib/krb5/os/sendto_kdc.c
index 040c554..04c9a7a 100644
--- a/src/lib/krb5/os/sendto_kdc.c
+++ b/src/lib/krb5/os/sendto_kdc.c
@@ -605,10 +605,10 @@ add_connection(struct conn_state **conns, struct addrinfo *ai,
     state->state = INITIALIZING;
     state->err = 0;
     state->x.out.sgp = state->x.out.sgbuf;
-    state->socktype = ai->ai_socktype;
-    state->family = ai->ai_family;
-    state->addrlen = ai->ai_addrlen;
-    memcpy(&state->addr, ai->ai_addr, ai->ai_addrlen);
+    state->addr.type = ai->ai_socktype;
+    state->addr.family = ai->ai_family;
+    state->addr.len = ai->ai_addrlen;
+    memcpy(&state->addr.saddr, ai->ai_addr, ai->ai_addrlen);
     state->fd = INVALID_SOCKET;
     state->server_index = server_index;
     SG_SET(&state->x.out.sgbuf[1], 0, 0);
@@ -766,25 +766,27 @@ start_connection(krb5_context context, struct conn_state *state,
     static const struct linger lopt = { 0, 0 };
 
     dprint("start_connection(@%p)\ngetting %s socket in family %d...", state,
-           state->socktype == SOCK_STREAM ? "stream" : "dgram", state->family);
-    fd = socket(state->family, state->socktype, 0);
+           state->addr.type == SOCK_STREAM ? "stream" : "dgram",
+           state->addr.family);
+    fd = socket(state->addr.family, state->addr.type, 0);
     if (fd == INVALID_SOCKET) {
         state->err = SOCKET_ERRNO;
-        dprint("socket: %m creating with af %d\n", state->err, state->family);
+        dprint("socket: %m creating with af %d\n", state->err,
+               state->addr.family);
         return -1;              /* try other hosts */
     }
     set_cloexec_fd(fd);
     /* Make it non-blocking.  */
     if (ioctlsocket(fd, FIONBIO, (const void *) &one))
         dperror("sendto_kdc: ioctl(FIONBIO)");
-    if (state->socktype == SOCK_STREAM) {
+    if (state->addr.type == SOCK_STREAM) {
         if (setsockopt(fd, SOL_SOCKET, SO_LINGER, &lopt, sizeof(lopt)))
             dperror("sendto_kdc: setsockopt(SO_LINGER)");
-        TRACE_SENDTO_KDC_TCP_CONNECT(context, state);
+        TRACE_SENDTO_KDC_TCP_CONNECT(context, &state->addr);
     }
 
     /* Start connecting to KDC.  */
-    e = connect(fd, (struct sockaddr *)&state->addr, state->addrlen);
+    e = connect(fd, (struct sockaddr *)&state->addr.saddr, state->addr.len);
     if (e != 0) {
         /*
          * This is the path that should be followed for non-blocking
@@ -832,16 +834,16 @@ start_connection(krb5_context context, struct conn_state *state,
         set_conn_state_msg_length(state, &state->callback_buffer);
     }
 
-    if (state->socktype == SOCK_DGRAM) {
+    if (state->addr.type == SOCK_DGRAM) {
         /* Send it now.  */
         ssize_t ret;
         sg_buf *sg = &state->x.out.sgbuf[0];
 
-        TRACE_SENDTO_KDC_UDP_SEND_INITIAL(context, state);
+        TRACE_SENDTO_KDC_UDP_SEND_INITIAL(context, &state->addr);
         dprint("sending %d bytes on fd %d\n", SG_LEN(sg), state->fd);
         ret = send(state->fd, SG_BUF(sg), SG_LEN(sg), 0);
         if (ret < 0 || (size_t) ret != SG_LEN(sg)) {
-            TRACE_SENDTO_KDC_UDP_ERROR_SEND_INITIAL(context, state,
+            TRACE_SENDTO_KDC_UDP_ERROR_SEND_INITIAL(context, &state->addr,
                                                     SOCKET_ERRNO);
             dperror("sendto");
             (void) closesocket(state->fd);
@@ -889,7 +891,7 @@ maybe_send(krb5_context context, struct conn_state *conn,
         return -1;
     }
 
-    if (conn->socktype == SOCK_STREAM) {
+    if (conn->addr.type == SOCK_STREAM) {
         dprint("skipping stream socket\n");
         /* The select callback will handle flushing any data we
            haven't written yet, and we only write it once.  */
@@ -898,11 +900,12 @@ maybe_send(krb5_context context, struct conn_state *conn,
 
     /* UDP - retransmit after a previous attempt timed out. */
     sg = &conn->x.out.sgbuf[0];
-    TRACE_SENDTO_KDC_UDP_SEND_RETRY(context, conn);
+    TRACE_SENDTO_KDC_UDP_SEND_RETRY(context, &conn->addr);
     dprint("sending %d bytes on fd %d\n", SG_LEN(sg), conn->fd);
     ret = send(conn->fd, SG_BUF(sg), SG_LEN(sg), 0);
     if (ret < 0 || (size_t) ret != SG_LEN(sg)) {
-        TRACE_SENDTO_KDC_UDP_ERROR_SEND_RETRY(context, conn, SOCKET_ERRNO);
+        TRACE_SENDTO_KDC_UDP_ERROR_SEND_RETRY(context, &conn->addr,
+                                              SOCKET_ERRNO);
         dperror("send");
         /* Keep connection alive, we'll try again next pass.
 
@@ -965,7 +968,7 @@ service_tcp_fd(krb5_context context, struct conn_state *conn,
             /* Bad -- the KDC shouldn't be sending to us first.  */
             e = EINVAL /* ?? */;
         kill_conn:
-            TRACE_SENDTO_KDC_TCP_DISCONNECT(context, conn);
+            TRACE_SENDTO_KDC_TCP_DISCONNECT(context, &conn->addr);
             kill_conn(conn, selstate, e);
             return e == 0;
         }
@@ -990,7 +993,7 @@ service_tcp_fd(krb5_context context, struct conn_state *conn,
          */
         e = get_so_error(conn->fd);
         if (e) {
-            TRACE_SENDTO_KDC_TCP_ERROR_CONNECT(context, conn, e);
+            TRACE_SENDTO_KDC_TCP_ERROR_CONNECT(context, &conn->addr, e);
             dprint("socket error on write fd: %m", e);
             goto kill_conn;
         }
@@ -1012,12 +1015,12 @@ service_tcp_fd(krb5_context context, struct conn_state *conn,
                ((conn->x.out.sg_count == 2 ? SG_LEN(&conn->x.out.sgp[1]) : 0)
                 + SG_LEN(&conn->x.out.sgp[0])),
                conn->fd);
-        TRACE_SENDTO_KDC_TCP_SEND(context, conn);
+        TRACE_SENDTO_KDC_TCP_SEND(context, &conn->addr);
         nwritten = SOCKET_WRITEV(conn->fd, conn->x.out.sgp,
                                  conn->x.out.sg_count, tmp);
         if (nwritten < 0) {
             e = SOCKET_ERRNO;
-            TRACE_SENDTO_KDC_TCP_ERROR_SEND(context, conn, e);
+            TRACE_SENDTO_KDC_TCP_ERROR_SEND(context, &conn->addr, e);
             dprint("failed: %m\n", e);
             goto kill_conn;
         }
@@ -1072,7 +1075,7 @@ service_tcp_fd(krb5_context context, struct conn_state *conn,
                 e = nread ? SOCKET_ERRNO : ECONNRESET;
                 free(conn->x.in.buf);
                 conn->x.in.buf = 0;
-                TRACE_SENDTO_KDC_TCP_ERROR_RECV(context, conn, e);
+                TRACE_SENDTO_KDC_TCP_ERROR_RECV(context, &conn->addr, e);
                 goto kill_conn;
             }
             conn->x.in.n_left -= nread;
@@ -1087,7 +1090,7 @@ service_tcp_fd(krb5_context context, struct conn_state *conn,
                                 conn->x.in.bufsizebytes + conn->x.in.bufsizebytes_read,
                                 4 - conn->x.in.bufsizebytes_read);
             if (nread < 0) {
-                TRACE_SENDTO_KDC_TCP_ERROR_RECV_LEN(context, conn, e);
+                TRACE_SENDTO_KDC_TCP_ERROR_RECV_LEN(context, &conn->addr, e);
                 e = SOCKET_ERRNO;
                 goto kill_conn;
             }
@@ -1132,7 +1135,7 @@ service_udp_fd(krb5_context context, struct conn_state *conn,
 
     nread = recv(conn->fd, conn->x.in.buf, conn->x.in.bufsize, 0);
     if (nread < 0) {
-        TRACE_SENDTO_KDC_UDP_ERROR_RECV(context, conn, SOCKET_ERRNO);
+        TRACE_SENDTO_KDC_UDP_ERROR_RECV(context, &conn->addr, SOCKET_ERRNO);
         kill_conn(conn, selstate, SOCKET_ERRNO);
         return 0;
     }
@@ -1271,7 +1274,7 @@ k5_sendto(krb5_context context, const krb5_data *message,
             goto cleanup;
         for (state = *tailptr; state != NULL && !done; state = state->next) {
             /* Contact each new connection whose socktype matches socktype1. */
-            if (state->socktype != socktype1)
+            if (state->addr.type != socktype1)
                 continue;
             if (maybe_send(context, state, sel_state, callback_info))
                 continue;
@@ -1283,7 +1286,7 @@ k5_sendto(krb5_context context, const krb5_data *message,
     /* Complete the first pass by contacting servers of the non-preferred
      * socktype (if given), waiting 1s for an answer from each. */
     for (state = conns; state != NULL && !done; state = state->next) {
-        if (state->socktype != socktype2)
+        if (state->addr.type != socktype2)
             continue;
         if (maybe_send(context, state, sel_state, callback_info))
             continue;
@@ -1323,7 +1326,7 @@ k5_sendto(krb5_context context, const krb5_data *message,
         goto cleanup;
     }
     /* Success!  */
-    TRACE_SENDTO_KDC_RESPONSE(context, winner);
+    TRACE_SENDTO_KDC_RESPONSE(context, &winner->addr);
     reply->data = winner->x.in.buf;
     reply->length = winner->x.in.pos - winner->x.in.buf;
     retval = 0;
diff --git a/src/lib/krb5/os/t_trace.c b/src/lib/krb5/os/t_trace.c
index 746dbea..ed53181 100644
--- a/src/lib/krb5/os/t_trace.c
+++ b/src/lib/krb5/os/t_trace.c
@@ -61,7 +61,7 @@ main (int argc, char *argv[])
     char *str = "example.data";
     krb5_octet *oct = (krb5_octet *) str;
     unsigned int oct_length = strlen(str);
-    struct conn_state conn;
+    struct remote_address ra;
     struct sockaddr_in *addr_in;
     krb5_data data;
     struct krb5_key_st key;
@@ -112,26 +112,26 @@ main (int argc, char *argv[])
     TRACE(ctx, "size_t and const char *, as four-character hex hash: "
           "{hashlenstr}", 1, NULL);
 
-    conn.socktype = SOCK_STREAM;
-    addr_in = (struct sockaddr_in *) &conn.addr;
+    ra.type = SOCK_STREAM;
+    addr_in = (struct sockaddr_in *)&ra.saddr;
     addr_in->sin_family = AF_INET;
     addr_in->sin_addr.s_addr = INADDR_ANY;
     addr_in->sin_port = htons(88);
-    conn.addrlen = sizeof(struct sockaddr_in);
-    conn.family = AF_INET;
-    TRACE(ctx, "struct conn_state *, show socket type, address, port: "
-          "{connstate}", &conn);
-    conn.socktype = SOCK_DGRAM;
-    TRACE(ctx, "struct conn_state *, show socket type, address, port: "
-          "{connstate}", &conn);
-    conn.socktype = 1234;
+    ra.len = sizeof(struct sockaddr_in);
+    ra.family = AF_INET;
+    TRACE(ctx, "struct remote_address *, show socket type, address, port: "
+          "{raddr}", &ra);
+    ra.type = SOCK_DGRAM;
+    TRACE(ctx, "struct remote_address *, show socket type, address, port: "
+          "{raddr}", &ra);
+    ra.type = 1234;
     addr_in->sin_family = AF_UNSPEC;
-    conn.family = AF_UNSPEC;
-    TRACE(ctx, "struct conn_state *, show socket type, address, port: "
-          "{connstate}", &conn);
-    conn.family = 5678;
-    TRACE(ctx, "struct conn_state *, show socket type, address, port: "
-          "{connstate}", &conn);
+    ra.family = AF_UNSPEC;
+    TRACE(ctx, "struct remote_address *, show socket type, address, port: "
+          "{raddr}", &ra);
+    ra.family = 5678;
+    TRACE(ctx, "struct remote_address *, show socket type, address, port: "
+          "{raddr}", &ra);
 
     data.magic = 0;
     data.length = strlen(str);
diff --git a/src/lib/krb5/os/t_trace.ref b/src/lib/krb5/os/t_trace.ref
index 4922b89..749d9c9 100644
--- a/src/lib/krb5/os/t_trace.ref
+++ b/src/lib/krb5/os/t_trace.ref
@@ -8,10 +8,10 @@ size_t and const char *, as hex bytes: 6578616D706C652E64617461
 size_t and const char *, as hex bytes: (null)
 size_t and const char *, as four-character hex hash: 7B9A
 size_t and const char *, as four-character hex hash: (null)
-struct conn_state *, show socket type, address, port: stream 0.0.0.0:88
-struct conn_state *, show socket type, address, port: dgram 0.0.0.0:88
-struct conn_state *, show socket type, address, port: socktype1234 AF_UNSPEC
-struct conn_state *, show socket type, address, port: socktype1234 af5678
+struct remote_address *, show socket type, address, port: stream 0.0.0.0:88
+struct remote_address *, show socket type, address, port: dgram 0.0.0.0:88
+struct remote_address *, show socket type, address, port: socktype1234 AF_UNSPEC
+struct remote_address *, show socket type, address, port: socktype1234 af5678
 krb5_data *, display as counted string: example.data
 krb5_data *, display as counted string: (null)
 krb5_data *, display as hex bytes: 6578616D706C652E64617461
diff --git a/src/lib/krb5/os/trace.c b/src/lib/krb5/os/trace.c
index 97e1c06..fcfc70b 100644
--- a/src/lib/krb5/os/trace.c
+++ b/src/lib/krb5/os/trace.c
@@ -131,7 +131,7 @@ trace_format(krb5_context context, const char *fmt, va_list ap)
     krb5_error_code kerr;
     size_t len, i;
     int err;
-    struct conn_state *cs;
+    struct remote_address *ra;
     const krb5_data *d;
     krb5_data data;
     char addrbuf[NI_MAXHOST], portbuf[NI_MAXSERV], tmpbuf[200], *str;
@@ -196,22 +196,22 @@ trace_format(krb5_context context, const char *fmt, va_list ap)
                     k5_buf_add(&buf, str);
                 free(str);
             }
-        } else if (strcmp(tmpbuf, "connstate") == 0) {
-            cs = va_arg(ap, struct conn_state *);
-            if (cs->socktype == SOCK_DGRAM)
+        } else if (strcmp(tmpbuf, "raddr") == 0) {
+            ra = va_arg(ap, struct remote_address *);
+            if (ra->type == SOCK_DGRAM)
                 k5_buf_add(&buf, "dgram");
-            else if (cs->socktype == SOCK_STREAM)
+            else if (ra->type == SOCK_STREAM)
                 k5_buf_add(&buf, "stream");
             else
-                k5_buf_add_fmt(&buf, "socktype%d", cs->socktype);
+                k5_buf_add_fmt(&buf, "socktype%d", ra->type);
 
-            if (getnameinfo((struct sockaddr *)&cs->addr, cs->addrlen,
+            if (getnameinfo((struct sockaddr *)&ra->saddr, ra->len,
                             addrbuf, sizeof(addrbuf), portbuf, sizeof(portbuf),
                             NI_NUMERICHOST|NI_NUMERICSERV) != 0) {
-                if (cs->family == AF_UNSPEC)
+                if (ra->family == AF_UNSPEC)
                     k5_buf_add(&buf, " AF_UNSPEC");
                 else
-                    k5_buf_add_fmt(&buf, " af%d", cs->family);
+                    k5_buf_add_fmt(&buf, " af%d", ra->family);
             } else
                 k5_buf_add_fmt(&buf, " %s:%s", addrbuf, portbuf);
         } else if (strcmp(tmpbuf, "data") == 0) {


More information about the cvs-krb5 mailing list