libflux: make flux_msg_fprint() output clearer

src/common/libflux/message.c

@@ -1522,53 +1522,178 @@ static const char *msgtype_shortstr (int type)
     return "?";
 }
 
+struct flagmap {
+    const char *name;
+    int flag;
+};
+
+static struct flagmap flagmap[] = {
+    { "topic", FLUX_MSGFLAG_TOPIC},
+    { "payload", FLUX_MSGFLAG_PAYLOAD},
+    { "noresponse", FLUX_MSGFLAG_NORESPONSE},
+    { "route", FLUX_MSGFLAG_ROUTE},
+    { "upstream", FLUX_MSGFLAG_UPSTREAM},
+    { "private", FLUX_MSGFLAG_PRIVATE},
+    { "streaming", FLUX_MSGFLAG_STREAMING},
+};
+static const int flagmap_len = sizeof (flagmap) / sizeof (flagmap[0]);
+
+static void flags2str (uint8_t flags, char *buf, int buflen)
+{
+    int i, len = 0;
+    buf[0] = '\0';
+    for (i = 0; i < flagmap_len; i++) {
+        if ((flags & flagmap[i].flag)) {
+            if (len) {
+                assert (len < (buflen - 1));
+                strcat (buf, ",");
+                len++;
+            }
+            assert ((len + strlen (flagmap[i].name)) < (buflen - 1));
+            strcat (buf, flagmap[i].name);
+            len += strlen (flagmap[i].name);
+        }
+    }
+}
+
+static void userid2str (uint32_t userid, char *buf, int buflen)
+{
+    int n;
+    if (userid == FLUX_USERID_UNKNOWN)
+        n = snprintf (buf, buflen, "unknown");
+    else
+        n = snprintf (buf, buflen, "%u", userid);
+    assert (n < buflen);
+}
+
+static void rolemask2str (uint32_t rolemask, char *buf, int buflen)
+{
+    int n;
+    switch (rolemask) {
+        case FLUX_ROLE_NONE:
+            n = snprintf (buf, buflen, "none");
+            break;
+        case FLUX_ROLE_OWNER:
+            n = snprintf (buf, buflen, "owner");
+            break;
+        case FLUX_ROLE_USER:
+            n = snprintf (buf, buflen, "user");
+            break;
+        case FLUX_ROLE_ALL:
+            n = snprintf (buf, buflen, "all");
+            break;
+        default:
+            n = snprintf (buf, buflen, "unknown");
+    }
+    assert (n < buflen);
+}
+
+static void nodeid2str (uint32_t nodeid, char *buf, int buflen)
+{
+    int n;
+    if (nodeid == FLUX_NODEID_ANY)
+        n = snprintf (buf, buflen, "any");
+    else if (nodeid == FLUX_NODEID_UPSTREAM)
+        n = snprintf (buf, buflen, "upstream");
+    else
+        n = snprintf (buf, buflen, "%u", nodeid);
+    assert (n < buflen);
+}
+
 void flux_msg_fprint (FILE *f, const flux_msg_t *msg)
 {
     int hops;
     const char *prefix;
-    uint8_t proto[PROTO_SIZE];
-    int i;
+    char flagsstr[128];
+    char useridstr[32];
+    char rolemaskstr[32];
+    char nodeidstr[32];
 
     fprintf (f, "--------------------------------------\n");
     if (!msg) {
         fprintf (f, "NULL");
         return;
     }
     prefix = msgtype_shortstr (msg->type);
+    /* Topic (keepalive has none)
+     */
+    if (msg->topic)
+        fprintf (f, "%s %s\n", prefix, msg->topic);
+    /* Proto info
+     */
+    flags2str (msg->flags, flagsstr, sizeof (flagsstr));
+    userid2str (msg->userid, useridstr, sizeof (useridstr));
+    rolemask2str (msg->rolemask, rolemaskstr, sizeof (rolemaskstr));
+    fprintf (f, "%s flags=%s userid=%s rolemask=%s ",
+             prefix, flagsstr, useridstr, rolemaskstr);
+    switch (msg->type) {
+        case FLUX_MSGTYPE_REQUEST:
+            nodeid2str (msg->nodeid, nodeidstr, sizeof (nodeidstr));
+            fprintf (f, "nodeid=%s matchtag=%u\n",
+                     nodeidstr,
+                     msg->matchtag);
+            break;
+        case FLUX_MSGTYPE_RESPONSE:
+            fprintf (f, "errnum=%u matchtag=%u\n",
+                     msg->errnum,
+                     msg->matchtag);
+            break;
+        case FLUX_MSGTYPE_EVENT:
+            fprintf (f, "sequence=%u\n",
+                     msg->sequence);
+            break;
+        case FLUX_MSGTYPE_KEEPALIVE:
+            fprintf (f, "errnum=%u status=%u\n",
+                     msg->errnum,
+                     msg->status);
+            break;
+        default:
+            fprintf (f, "aux1=0x%X aux2=0x%X\n",
+                     msg->aux1,
+                     msg->aux2);
+            break;
+    }
     /* Route stack
      */
     hops = flux_msg_get_route_count (msg); /* -1 if no route stack */
-    if (hops >= 0) {
-        int len = flux_msg_get_route_size (msg);
+    if (hops > 0) {
         char *rte = flux_msg_get_route_string (msg);
         assert (rte != NULL);
-        fprintf (f, "%s[%3.3d] |%s|\n", prefix, len, rte);
+        fprintf (f, "%s |%s|\n", prefix, rte);
         free (rte);
     };
-    /* Topic (keepalive has none)
-     */
-    if (msg->topic)
-        fprintf (f, "%s[%3.3zu] %s\n", prefix, strlen (msg->topic), msg->topic);
     /* Payload
      */
     if (flux_msg_has_payload (msg)) {
         const char *s;
         const void *buf;
         int size;
         if (flux_msg_get_string (msg, &s) == 0)
-            fprintf (f, "%s[%3.3zu] %s\n", prefix, strlen (s), s);
-        else if (flux_msg_get_payload (msg, &buf, &size) == 0)
-            fprintf (f, "%s[%3.3d] ...\n", prefix, size);
+            fprintf (f, "%s %s\n", prefix, s);
+        else if (flux_msg_get_payload (msg, &buf, &size) == 0) {
+            /* output at max 80 cols worth of info.  We subtract 2 and
+             * set 'max' to 78 b/c of the prefix taking 2 bytes.
+             */
+            int i, iter, max = 78;
+            bool ellipses = false;
+            fprintf (f, "%s ", prefix);
+            if ((size * 2) > max) {
+                /* -3 for ellipses, divide by 2 b/c 2 chars of output
+                 * per byte */
+                iter = (max - 3) / 2;
+                ellipses = true;
+            }
+            else
+                iter = size;
+            for (i = 0; i < iter; i++)
+                fprintf (f, "%02X", ((uint8_t *)buf)[i]);
+            if (ellipses)
+                fprintf (f, "...");
+            fprintf (f, "\n");
+        }
         else
             fprintf (f, "malformed payload\n");
     }
-    /* Proto block
-     */
-    msg_proto_setup (msg, proto, PROTO_SIZE);
-    fprintf (f, "%s[%03d] ", prefix, PROTO_SIZE);
-    for (i = 0; i < PROTO_SIZE; i++)
-        fprintf (f, "%02X", proto[i]);
-    fprintf (f, "\n");
 }
 
 static zmsg_t *msg_to_zmsg (const flux_msg_t *msg)

src/common/libflux/test/message.c

@@ -11,6 +11,7 @@
 #if HAVE_CONFIG_H
 #include "config.h"
 #endif
+#include <stdbool.h>
 #include <czmq.h>
 #include <errno.h>
 #include <stdio.h>
@@ -19,6 +20,8 @@
 #include "src/common/libflux/message.h"
 #include "src/common/libtap/tap.h"
 
+static bool verbose = false;
+
 void check_cornercase (void)
 {
     flux_msg_t *msg;
@@ -1075,8 +1078,10 @@ void check_copy (void)
 void check_print (void)
 {
     flux_msg_t *msg;
+    char *strpayload = "a.special.payload";
     char buf[] = "xxxxxxxx";
-    FILE *f = fopen ("/dev/null", "w");
+    char buf_long[] = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
+    FILE *f = verbose ? stderr : fopen ("/dev/null", "w");
     if (!f)
         BAIL_OUT ("cannot open /dev/null for writing");
 
@@ -1108,6 +1113,48 @@ void check_print (void)
         "flux_msg_fprint doesn't segfault on fully loaded request");
     flux_msg_destroy (msg);
 
+    ok ((msg = flux_msg_create (FLUX_MSGTYPE_REQUEST)) != NULL,
+        "created test message");
+    ok (flux_msg_set_userid (msg, 42) == 0,
+        "set userid");
+    ok (flux_msg_set_rolemask (msg, FLUX_ROLE_OWNER) == 0,
+        "set rolemask");
+    ok (flux_msg_set_nodeid (msg, 42) == 0,
+        "set nodeid");
+    ok (flux_msg_set_string (msg, strpayload) == 0,
+        "added payload");
+    lives_ok ({flux_msg_fprint (f, msg);},
+        "flux_msg_fprint doesn't segfault on request settings #1");
+    flux_msg_destroy (msg);
+
+    ok ((msg = flux_msg_create (FLUX_MSGTYPE_REQUEST)) != NULL,
+        "created test message");
+    ok (flux_msg_set_rolemask (msg, FLUX_ROLE_USER) == 0,
+        "set rolemask");
+    ok (flux_msg_set_flags (msg, FLUX_MSGFLAG_NORESPONSE | FLUX_MSGFLAG_UPSTREAM) == 0,
+        "set new flags");
+    lives_ok ({flux_msg_fprint (f, msg);},
+        "flux_msg_fprint doesn't segfault on request settings #2");
+    flux_msg_destroy (msg);
+
+    ok ((msg = flux_msg_create (FLUX_MSGTYPE_REQUEST)) != NULL,
+        "created test message");
+    ok (flux_msg_set_rolemask (msg, FLUX_ROLE_ALL) == 0,
+        "set rolemask");
+    ok (flux_msg_set_flags (msg, FLUX_MSGFLAG_PRIVATE | FLUX_MSGFLAG_STREAMING) == 0,
+        "set new flags");
+    lives_ok ({flux_msg_fprint (f, msg);},
+        "flux_msg_fprint doesn't segfault on request settings #3");
+    flux_msg_destroy (msg);
+
+    ok ((msg = flux_msg_create (FLUX_MSGTYPE_REQUEST)) != NULL,
+        "created test message");
+    ok (flux_msg_set_payload (msg, buf_long, strlen (buf_long)) == 0,
+        "added long payload");
+    lives_ok ({flux_msg_fprint (f, msg);},
+        "flux_msg_fprint doesn't segfault on long payload");
+    flux_msg_destroy (msg);
+
     ok ((msg = flux_msg_create (FLUX_MSGTYPE_RESPONSE)) != NULL,
         "created test message");
     ok (flux_msg_enable_route (msg) == 0,
@@ -1204,6 +1251,13 @@ void check_refcount (void)
 
 int main (int argc, char *argv[])
 {
+    int opt;
+
+    while ((opt = getopt (argc, argv, "v")) != -1) {
+        if (opt == 'v')
+            verbose = true;
+    }
+
     plan (NO_PLAN);
 
     check_cornercase ();