niobserver: some work.

Put all the TLV stuff together.  *If* some TLVs are only in the file
header and others are only in packets, thot should be the split; it
appears that the TLVS with a type with the 0x01 bit clear are for the
file header, so perhaps they can be split based on that.

Don't include the TLV header in the structure for the time_info TLV;
that matches other TLV structures.  Write the time_info TLV in two
parts, as we do with the comment TLV.

Consistently use _TO_LE macros in our _TO_LE_IN_PLACE macros.

Add _FROM_LE_IN_PLACE and _TO_LE_IN_PLACE macros for the network_load
TLV.

Use %z, now that we require C99-or-later.

Check the length of TLVs.

Note some things found in files while reverse engineering.

2 files changed, 144 insertions, 85 deletions

diff --git a/wiretap/network_instruments.c b/wiretap/network_instruments.c
index 4179c9478d..4a6635dae0 100644
--- a/wiretap/network_instruments.c
+++ b/wiretap/network_instruments.c
@@ -193,8 +193,8 @@ wtap_open_return_val network_instruments_open(wtap *wth, int *err, gchar **err_i
 
         if (tlvh.length < sizeof tlvh) {
             *err = WTAP_ERR_BAD_FILE;
-            *err_info = g_strdup_printf("Observer: bad record (TLV length %u < %lu)",
-                tlvh.length, (unsigned long)sizeof tlvh);
+            *err_info = g_strdup_printf("Observer: bad record (TLV length %u < %zu)",
+                tlvh.length, sizeof tlvh);
             return WTAP_OPEN_ERROR;
         }
 
@@ -217,11 +217,11 @@ wtap_open_return_val network_instruments_open(wtap *wth, int *err, gchar **err_i
         /* process (or skip over) the current TLV */
         switch (tlvh.type) {
         case INFORMATION_TYPE_TIME_INFO:
-            if (tlvh.length != sizeof tlvh + sizeof private_state->time_format) {
+            if (tlv_data_length != sizeof private_state->time_format) {
                 *err = WTAP_ERR_BAD_FILE;
-                *err_info = g_strdup_printf("Observer: bad record (time information TLV length %u != %lu)",
+                *err_info = g_strdup_printf("Observer: bad record (time information TLV length %u != %zu)",
                     tlvh.length,
-                    (unsigned long)(sizeof tlvh + sizeof private_state->time_format));
+                    sizeof tlvh + sizeof private_state->time_format);
                 return WTAP_OPEN_ERROR;
             }
             if (!wtap_read_bytes(wth->fh, &private_state->time_format,
@@ -232,8 +232,7 @@ wtap_open_return_val network_instruments_open(wtap *wth, int *err, gchar **err_i
             offset += (guint)sizeof private_state->time_format;
             break;
         default:
-            tlv_data_length = tlvh.length - (guint)sizeof tlvh;
-            if (tlv_data_length > 0) {
+            if (tlv_data_length != 0) {
                 if (!wtap_read_bytes(wth->fh, NULL, tlv_data_length, err, err_info))
                     return WTAP_OPEN_ERROR;
             }
@@ -408,7 +407,6 @@ read_packet_header(wtap *wth, FILE_T fh, union wtap_pseudo_header *pseudo_header
     int offset;
     guint i;
     tlv_header tlvh;
-    int seek_increment;
     tlv_wireless_info wireless_header;
 
     offset = 0;
@@ -466,6 +464,8 @@ read_packet_header(wtap *wth, FILE_T fh, union wtap_pseudo_header *pseudo_header
 
     /* process extra information */
     for (i = 0; i < packet_header->number_of_information_elements; i++) {
+        guint tlv_data_length;
+
         /* read the TLV header */
         if (!wtap_read_bytes(fh, &tlvh, sizeof tlvh, err, err_info))
             return -1;
@@ -474,15 +474,21 @@ read_packet_header(wtap *wth, FILE_T fh, union wtap_pseudo_header *pseudo_header
 
         if (tlvh.length < sizeof tlvh) {
             *err = WTAP_ERR_BAD_FILE;
-            *err_info = g_strdup_printf("Observer: bad record (TLV length %u < %lu)",
-                tlvh.length, (unsigned long)sizeof tlvh);
+            *err_info = g_strdup_printf("Observer: bad record (TLV length %u < %zu)",
+                tlvh.length, sizeof tlvh);
             return -1;
         }
+        tlv_data_length = tlvh.length - (guint)sizeof tlvh;
 
         /* process (or skip over) the current TLV */
         switch (tlvh.type) {
         case INFORMATION_TYPE_WIRELESS:
-            /* XXX - what if tlvh.length != sizeof wireless_header? */
+            if (tlv_data_length != sizeof wireless_header) {
+                *err = WTAP_ERR_BAD_FILE;
+                *err_info = g_strdup_printf("Observer: bad record (wireless TLV length %u != %zu)",
+                    tlvh.length, sizeof tlvh + sizeof wireless_header);
+                return -1;
+            }
             if (!wtap_read_bytes(fh, &wireless_header, sizeof wireless_header,
                                  err, err_info))
                 return -1;
@@ -499,12 +505,11 @@ read_packet_header(wtap *wth, FILE_T fh, union wtap_pseudo_header *pseudo_header
             break;
         default:
             /* skip the TLV data */
-            seek_increment = tlvh.length - (int)sizeof tlvh;
-            if (seek_increment > 0) {
-                if (!wtap_read_bytes(fh, NULL, seek_increment, err, err_info))
+            if (tlv_data_length != 0) {
+                if (!wtap_read_bytes(fh, NULL, tlv_data_length, err, err_info))
                     return -1;
             }
-            offset += seek_increment;
+            offset += tlv_data_length;
         }
     }
 
@@ -672,9 +677,10 @@ static gboolean network_instruments_dump_open(wtap_dumper *wdh, int *err,
     guint header_offset;
     const gchar *err_str;
     tlv_header comment_header;
-    tlv_time_info time_header;
     char comment[64];
     size_t comment_length;
+    tlv_header time_info_header;
+    tlv_time_info time_info;
     struct tm * current_time;
     time_t system_time;
 
@@ -714,13 +720,13 @@ static gboolean network_instruments_dump_open(wtap_dumper *wdh, int *err,
 
     /* create the timestamp encoding TLV */
     {
-        time_header.type = INFORMATION_TYPE_TIME_INFO;
-        time_header.length = (guint16) (sizeof(time_header));
-        time_header.time_format = TIME_INFO_GMT;
+        time_info_header.type = INFORMATION_TYPE_TIME_INFO;
+        time_info_header.length = (guint16) (sizeof(time_info_header) + sizeof(time_info));
+        time_info.time_format = TIME_INFO_GMT;
 
         /* update the file header to account for the timestamp encoding TLV */
         file_header.number_of_information_elements++;
-        header_offset += time_header.length;
+        header_offset += time_info_header.length;
     }
 
     /* Store the offset to the first packet */
@@ -750,11 +756,17 @@ static gboolean network_instruments_dump_open(wtap_dumper *wdh, int *err,
 
     /* write the time info TLV */
     {
-        TLV_TIME_INFO_TO_LE_IN_PLACE(time_header);
-        if (!wtap_dump_file_write(wdh, &time_header, sizeof(time_header), err)) {
+        TLV_HEADER_TO_LE_IN_PLACE(time_info_header);
+        if (!wtap_dump_file_write(wdh, &time_info_header, sizeof(time_info_header), err)) {
+            return FALSE;
+        }
+        wdh->bytes_dumped += sizeof(time_info_header);
+
+        TLV_TIME_INFO_TO_LE_IN_PLACE(time_info);
+        if (!wtap_dump_file_write(wdh, &time_info, sizeof(time_info), err)) {
             return FALSE;
         }
-        wdh->bytes_dumped += sizeof(time_header);
+        wdh->bytes_dumped += sizeof(time_info);
     }
 
     err_str = init_gmt_to_localtime_offset();
diff --git a/wiretap/network_instruments.h b/wiretap/network_instruments.h
index 7651fff493..c30bc47266 100644
--- a/wiretap/network_instruments.h
+++ b/wiretap/network_instruments.h
@@ -54,22 +54,118 @@ typedef struct tlv_header
     (_tlv_header).type   = GUINT16_TO_LE((_tlv_header).type); \
     (_tlv_header).length = GUINT16_TO_LE((_tlv_header).length)
 
+/*
+ * TLV type values.
+ *
+ * Do TLVs without the 0x0100 bit set show up in packets, and
+ * do TLVs with that set show up in the file header, or are
+ * there two separate types of TLV?
+ *
+ * ALIAS_LIST contains an ASCII string (null-terminated, but
+ * we can't trust that, of course) that is the pathname of
+ * a file containing the alias list.  Not much use to us.
+ *
+ * COMMENT contains an ASCII string (null-terminated, but
+ * we can't trust that, of course); in all the captures
+ * I've seen, it appears to be a note about the file added
+ * by Observer, not by a user.  It appears to end with 0x0a
+ * 0x2e, i.e. '\n' '.'.
+ *
+ * REMOTE_PROBE contains, in all the captures I've seen, an
+ * ASCII string (null-terminated, but we cna't trust that,
+ * of course) of the form "Remote Probe [hex string]".  THe
+ * hex string has 8 characters, i.e. 4 octets.
+ *
+ * The Observer document indicates that the types of expert information
+ * packets are:
+ *
+ *    Network Load (markers used by Expert Time Interval and What If
+ *    analysis modes)
+ *
+ *    Start/Stop Packet Capture marker frames (with time stamps when
+ *    captures start and stop)
+ *
+ *    Wireless Channel Change (markers showing what channel was being
+ *    currently listened to)
+ *
+ * That information appears to be contained in TLVs.
+ */
+#define INFORMATION_TYPE_ALIAS_LIST         0x0001
+#define INFORMATION_TYPE_COMMENT            0x0002 /* ASCII text */
+#define INFORMATION_TYPE_TIME_INFO          0x0004
+#define INFORMATION_TYPE_REMOTE_PROBE       0x0005
+#define INFORMATION_TYPE_NETWORK_LOAD       0x0100
+#define INFORMATION_TYPE_WIRELESS           0x0101
+#define INFORMATION_TYPE_CAPTURE_START_STOP 0x0104
+
+/*
+ * See in Fibre Channel captures; not seen elsewhere.
+ *
+ * It has 4 bytes of data in all captures I've seen.
+ */
+/*                                          0x0106 */
+
 typedef struct tlv_time_info {
     guint16 type;
     guint16 length;
     guint32 time_format;
 } tlv_time_info;
 
+/*
+ * TIME_INFO time_format values.
+ */
+#define TIME_INFO_LOCAL 0
+#define TIME_INFO_GMT   1
+
 #define TLV_TIME_INFO_FROM_LE_IN_PLACE(_tlv_time_info) \
-    (_tlv_time_info).type   = GUINT16_FROM_LE((_tlv_time_info).type); \
-    (_tlv_time_info).length = GUINT16_FROM_LE((_tlv_time_info).length); \
     (_tlv_time_info).time_format = GUINT32_FROM_LE((_tlv_time_info).time_format)
 
 #define TLV_TIME_INFO_TO_LE_IN_PLACE(_tlv_time_info) \
-    (_tlv_time_info).type   = GUINT16_TO_LE((_tlv_time_info).type); \
-    (_tlv_time_info).length = GUINT16_TO_LE((_tlv_time_info).length); \
-    (_tlv_time_info).time_format = GUINT32_FROM_LE((_tlv_time_info).time_format)
+    (_tlv_time_info).time_format = GUINT32_TO_LE((_tlv_time_info).time_format)
 
+/*
+ * Might some of these be broadecast and multicast packet counts, or
+ * error counts, or both?
+ */
+typedef struct tlv_network_load
+{
+    guint32 utilization;        /* network utilization, in .1% units */
+    guint32 unknown1;           /* zero in all captures I've seen */
+    guint32 unknown2;           /* zero in all captures I've seen */
+    guint32 packets_per_second;
+    guint32 unknown3;           /* zero in all captures I've seen */
+    guint32 bytes_per_second;
+    guint32 unknown4;           /* zero in all captures I've seen */
+} tlv_network_load;
+
+#define TLV_NETWORK_LOAD_FROM_LE_IN_PLACE(_tlv_network_load) \
+    (_tlv_network_load).utilization = GUINT32_FROM_LE((_tlv_network_load).utilization); \
+    (_tlv_network_load).unknown1 = GUINT32_FROM_LE((_tlv_network_load).unknown1); \
+    (_tlv_network_load).unknown2 = GUINT32_FROM_LE((_tlv_network_load).unknown2); \
+    (_tlv_network_load).packets_per_second = GUINT32_FROM_LE((_tlv_network_load).packets_per_second); \
+    (_tlv_network_load).unknown3 = GUINT32_FROM_LE((_tlv_network_load).unknown3); \
+    (_tlv_network_load).bytes_per_second = GUINT32_FROM_LE((_tlv_network_load).bytes_per_second); \
+    (_tlv_network_load).unknown4 = GUINT32_FROM_LE((_tlv_network_load).unknown4) \
+
+#define TLV_NETWORK_LOAD_TO_LE_IN_PLACE(_tlv_network_load) \
+    (_tlv_network_load).utilization = GUINT32_TO_LE((_tlv_network_load).utilization); \
+    (_tlv_network_load).unknown1 = GUINT32_TO_LE((_tlv_network_load).unknown1); \
+    (_tlv_network_load).unknown2 = GUINT32_TO_LE((_tlv_network_load).unknown2); \
+    (_tlv_network_load).packets_per_second = GUINT32_TO_LE((_tlv_network_load).packets_per_second); \
+    (_tlv_network_load).unknown3 = GUINT32_TO_LE((_tlv_network_load).unknown3); \
+    (_tlv_network_load).bytes_per_second = GUINT32_TO_LE((_tlv_network_load).bytes_per_second); \
+    (_tlv_network_load).unknown4 = GUINT32_TO_LE((_tlv_network_load).unknown4) \
+
+/*
+ * quality is presumably some measure of signal quality; in
+ * the captures I've seen, it has values of 15, 20-27, 50-54,
+ * 208, and 213.
+ *
+ * conditions has values of 0x00, 0x02, and 0x90.
+ *
+ * reserved is either 0x00 or 0x80; the 0x80 values
+ * are for TLVs where conditions is 0x90.
+ */
 typedef struct tlv_wireless_info {
     guint8 quality;
     guint8 signalStrength;
@@ -85,20 +181,16 @@ typedef struct tlv_wireless_info {
  * Wireless conditions
  */
 #define WIRELESS_WEP_SUCCESS		0x80
+/*                                      0x10 */
+/*                                      0x02 */
 
-/*
- * TLV type values.
- */
-#define INFORMATION_TYPE_ALIAS_LIST 0x01
-#define INFORMATION_TYPE_COMMENT    0x02 /* ASCII text */
-#define INFORMATION_TYPE_TIME_INFO  0x04
-#define INFORMATION_TYPE_WIRELESS   0x101
+typedef struct tlv_capture_start_stop
+{
+    guint32 start_stop;
+} tlv_capture_start_stop;
 
-/*
- * TVL TIME_INFO values.
- */
-#define TIME_INFO_LOCAL 0
-#define TIME_INFO_GMT   1
+#define START_STOP_TYPE_STOP   0
+#define START_STOP_TYPE_START  1
 
 typedef struct packet_entry_header
 {
@@ -160,49 +252,4 @@ typedef struct packet_entry_header
 #define PACKET_TYPE_DATA_PACKET               0
 #define PACKET_TYPE_EXPERT_INFORMATION_PACKET 1
 
-/*
- * The Observer document indicates that the types of expert information
- * packets are:
- *
- *    Network Load (markers used by Expert Time Interval and What If
- *    analysis modes)
- *
- *    Start/Stop Packet Capture marker frames (with time stamps when
- *    captures start and stop)
- *
- *    Wireless Channel Change (markers showing what channel was being
- *    currently listened to)
- *
- * That information appears to be contained in TLVs.
- */
-
-/*
- * TLV type values.
- */
-#define INFORMATION_TYPE_NETWORK_LOAD       0x0100
-#define INFORMATION_TYPE_CAPTURE_START_STOP 0x0104
-
-/*
- * Might some of these be broadcast and multicast packet counts?
- */
-typedef struct tlv_network_load
-{
-    guint32 utilization;        /* network utilization, in .1% units */
-    guint32 unknown1;
-    guint32 unknown2;
-    guint32 packets_per_second;
-    guint32 unknown3;
-    guint32 bytes_per_second;
-    guint32 unknown4;
-} tlv_network_load;
-
-typedef struct tlv_capture_start_stop
-{
-    guint32 start_stop;
-} tlv_capture_start_stop;
-
-#define START_STOP_TYPE_STOP   0
-#define START_STOP_TYPE_START  1
-
 #endif
-