From 81af4460f352aa4a1b5d4cbbe50ef48dcb68b527 Mon Sep 17 00:00:00 2001
From: Andrew Bettison <andrew@servalproject.org>
Date: Fri, 12 Apr 2013 17:18:25 +0930
Subject: [PATCH] Fix broken "ping" command arg parsing

---
 cli.c         | 22 +++++++++++++---------
 commandline.c | 11 +++++++----
 tests/routing |  6 ++++--
 3 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/cli.c b/cli.c
index 0477bcf6..300c25de 100644
--- a/cli.c
+++ b/cli.c
@@ -52,7 +52,7 @@ int cli_usage_args(const int argc, const char *const *args, const struct cli_sch
     if (matched) {
       matched_any = 1;
       for (opt = 0; (word = commands[cmd].words[opt]); ++opt) {
-	if (word[0] == '\\')
+	if (word[0] == '|')
 	  ++word;
 	xprintf(xpf, " %s", word);
       }
@@ -116,8 +116,8 @@ int cli_parse(const int argc, const char *const *args, const struct cli_schema *
        * or "wordN", labels it with the matched word (an empty alternative, eg "|word" does not
        * match an empty argument)
        *
-       * (as a result of the above rule, "|word" or "word|" or "|word|" consumes one argument that
-       * exactly matches "word" and labels it "word")
+       * (as a special case of the above rule, "|word" consumes one argument that exactly matches
+       * "word" and labels it "word", but it appears in the help description as "word")
        *
        * "<label>" consumes exactly one argument "ANY", records it with label "label"
        *
@@ -170,16 +170,20 @@ int cli_parse(const int argc, const char *const *args, const struct cli_schema *
 	const char *text = NULL;
 	const char *label = NULL;
 	unsigned labellen = 0;
-	const char *word;
+	const char *word = pattern;
 	unsigned wordlen = 0;
 	char simple = 0;
 	unsigned alt = 0;
-	for (word = pattern; word < &pattern[patlen]; word += wordlen + 1, ++alt) {
+	if (patlen && *word == '|') {
+	  ++alt;
+	  ++word;
+	}
+	if (patlen == 0)
+	  return WHYF("Internal error: commands[%d].word[%d]=\"%s\" - empty words not allowed", cmd, opt, commands[cmd].words[opt]);
+	for (; word < &pattern[patlen]; word += wordlen + 1, ++alt) {
 	  // Skip over empty "||word" alternative (but still count it).
-	  if (*word == '|') {
-	    wordlen = 0;
-	    continue;
-	  }
+	  if (*word == '|')
+	    return WHYF("Internal error: commands[%d].word[%d]=\"%s\" - empty alternatives not allowed", cmd, opt, commands[cmd].words[opt]);
 	  // Find end of "word|" alternative.
 	  wordlen = 1;
 	  while (&word[wordlen] < &pattern[patlen] && word[wordlen] != '|')
diff --git a/commandline.c b/commandline.c
index 3b5441e6..a5db9655 100644
--- a/commandline.c
+++ b/commandline.c
@@ -937,13 +937,13 @@ int app_mdp_ping(const struct cli_parsed *parsed, void *context)
   if (config.debug.verbose)
     DEBUG_cli_parsed(parsed);
   const char *sidhex, *count;
-  if (cli_arg(parsed, "SID|broadcast", &sidhex, str_is_subscriber_id, "broadcast") == -1)
+  if (cli_arg(parsed, "SID", &sidhex, str_is_subscriber_id, "broadcast") == -1)
     return -1;
   if (cli_arg(parsed, "count", &count, NULL, "0") == -1)
     return -1;
   
   // assume we wont hear any responses
-  int ret=-1;
+  int ret=1;
   int icount=atoi(count);
 
   overlay_mdp_frame mdp;
@@ -968,6 +968,7 @@ int app_mdp_ping(const struct cli_parsed *parsed, void *context)
 
   /* TODO Eventually we should try to resolve SID to phone number and vice versa */
   printf("MDP PING %s (%s): 12 data bytes\n", alloca_tohex_sid_t(ping_sid), alloca_tohex_sid_t(ping_sid));
+  fflush(stdout);
 
   time_ms_t rx_mintime=-1;
   time_ms_t rx_maxtime=-1;
@@ -1028,6 +1029,7 @@ int app_mdp_ping(const struct cli_parsed *parsed, void *context)
 		     hop_count,
 		     mdp.packetTypeAndFlags&MDP_NOCRYPT?"":" ENCRYPTED",
 		     mdp.packetTypeAndFlags&MDP_NOSIGN?"":" SIGNED");
+	      fflush(stdout);
 	      // TODO Put duplicate pong detection here so that stats work properly.
 	      rx_count++;
 	      ret=0;
@@ -1069,6 +1071,7 @@ int app_mdp_ping(const struct cli_parsed *parsed, void *context)
     printf("round-trip min/avg/max/stddev%s = %lld/%.3f/%lld/%.3f ms\n",
 	   (samples<rx_count)?" (stddev calculated from last 1024 samples)":"",
 	   rx_mintime,rx_mean,rx_maxtime,rx_stddev);
+    fflush(stdout);
   }
   overlay_mdp_client_done();
   return ret;
@@ -2349,7 +2352,7 @@ struct cli_schema command_line_options[]={
    "Stop a running daemon with instance path from SERVALINSTANCE_PATH environment variable."},
   {app_server_status,{"status",NULL},CLIFLAG_PERMISSIVE_CONFIG,
    "Display information about running daemon."},
-  {app_mdp_ping,{"mdp","ping","<SID|broadcast>","[<count>]",NULL}, 0,
+  {app_mdp_ping,{"mdp","ping","<SID>|broadcast","[<count>]",NULL}, 0,
    "Attempts to ping specified node via Mesh Datagram Protocol (MDP)."},
   {app_trace,{"mdp","trace","<SID>",NULL}, 0,
    "Trace through the network to the specified node via MDP."},
@@ -2392,7 +2395,7 @@ struct cli_schema command_line_options[]={
         "Extract and decrypt a file from Rhizome and write it to the given path"},
   {app_rhizome_delete,{"rhizome","delete","manifest|payload|bundle","<manifestid>",NULL}, 0,
 	"Remove the manifest, or payload, or both for the given Bundle ID from the Rhizome store"},
-  {app_rhizome_delete,{"rhizome","delete","file|","<fileid>",NULL}, 0,
+  {app_rhizome_delete,{"rhizome","delete","|file","<fileid>",NULL}, 0,
 	"Remove the file with the given hash from the Rhizome store"},
   {app_rhizome_direct_sync,{"rhizome","direct","sync","[<url>]",NULL}, 0,
 	"Synchronise with the specified Rhizome Direct server. Return when done."},
diff --git a/tests/routing b/tests/routing
index b43f76bf..fdaa07d9 100755
--- a/tests/routing
+++ b/tests/routing
@@ -39,9 +39,11 @@ start_routing_instance() {
       set server.interface_path "$SERVALD_VAR" \
       set monitor.socket "org.servalproject.servald.monitor.socket.$TFWUNIQUE.$instance_name" \
       set mdp.socket "org.servalproject.servald.mdp.socket.$TFWUNIQUE.$instance_name" \
+      set log.console.level debug \
       set log.console.show_pid on \
       set log.console.show_time on \
-      set debug.mdprequests Yes \
+      set debug.mdprequests yes \
+      set debug.verbose yes \
       set rhizome.enable no
    start_servald_server
    wait_until interface_up
@@ -261,7 +263,7 @@ test_multihop_linear() {
    foreach_instance +A +B +C +D \
       wait_until has_seen_instances +A +B +C +D
    set_instance +A
-   executeOk_servald mdp ping $SIDD 1
+   executeOk_servald --stdout --stderr mdp ping $SIDD 1
    tfw_cat --stdout --stderr
    executeOk_servald mdp trace $SIDD
    tfw_cat --stdout --stderr