Have the message data just be a raw FT_BYTES field.

If somebody wants it interpreted, they should submit code to do so,
*along with a sample capture*, so we can see how it's *supposed* to be
interpreted - the way it was being interpreted makes no sense for
several reasons.

Change-Id: I2dbcb3915fd7d7d1b4a27be627f395ac43bc4130
Reviewed-on: https://code.wireshark.org/review/33905
Petri-Dish: Guy Harris <guy@alum.mit.edu>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>

1 files changed, 21 insertions, 28 deletions

diff --git a/epan/dissectors/packet-armagetronad.c b/epan/dissectors/packet-armagetronad.c
index 87ef76b3d0..ce8234dd11 100644
--- a/epan/dissectors/packet-armagetronad.c
+++ b/epan/dissectors/packet-armagetronad.c
@@ -129,36 +129,29 @@ is_armagetronad_packet(tvbuff_t * tvb)
 static void
 add_message_data(tvbuff_t * tvb, gint offset, gint data_len, proto_tree * tree)
 {
-	gchar *data = NULL;
-	gchar  tmp;
-	int    i;
-
 	if (!tree)
 		return;
 
-	data = (gchar *)tvb_memcpy(tvb, wmem_alloc(wmem_packet_scope(), data_len + 1), offset, data_len);
-	data[data_len] = '\0';
-
-	for (i = 0; i < data_len; i += 2) {
-		/*
-		 * There must be a better way to tell
-		 * Wireshark not to stop on null bytes
-		 * as the length is known
-		 */
-		if (!data[i])
-			data[i] = ' ';
-
-		if (!data[i+1])
-			data[i+1] = ' ';
-
-		/* Armagetronad swaps unconditionally */
-		tmp = data[i];
-		data[i] = data[i+1];
-		data[i+1] = tmp;
-	}
-
-	proto_tree_add_string(tree, hf_armagetronad_data, tvb, offset,
-			      data_len, (gchar *) data);
+	/*
+	 * XXX - if these are text strings, as the original dissector
+	 * treated them as, why the byte swapping?  That would make
+	 * sense only if they're UCS-2 strings, but the rest of the
+	 * code wasn't treating them as such.
+	 *
+	 * Just treat it as a byte array.  If that's wrong, submit
+	 * a change, with a sample packet, that treats it as whatever
+	 * it is, an with a comment that *states* what it is.
+	 *
+	 * XXX - this claimed that "Armagetronad swaps unconditionally",
+	 * but I'm not seeing any obvious unconditional byte-swapping
+	 * in the code, and if somebody's never seen a big-endian
+	 * machine in their life, they may well confuse "convert to
+	 * network byte order" with "unconditionally swap".  So
+	 * if you want to dump this out as a sequence of shorts,
+	 * fetch the shorts one at a time using ENC_BIG_ENDIAN.
+	 */
+	proto_tree_add_item(tree, hf_armagetronad_data, tvb, offset,
+			      data_len, ENC_NA);
 }
 
 static gint
@@ -275,7 +268,7 @@ void proto_register_armagetronad(void)
 		 },
 		{&hf_armagetronad_data,
 		 {"Data", "armagetronad.data",
-		  FT_STRING, BASE_NONE, NULL, 0x0,
+		  FT_BYTES, BASE_NONE, NULL, 0x0,
 		  "The actual data (array of shorts in network order)", HFILL}
 		 },
 		{&hf_armagetronad_sender_id,