Address bug #46802: AIVDM to CSV is broken in some weird cases

While trying the regression tests on a MacBook (PowerPC), I ran across
some failures in the "Testing AIVDM decoding w/ CSV format" test. The
failing type/dac/fid cases were as follows:

6,1,14
6,1,18
6,1,30
6,1,32
8,200,10

The proximate cause is endian dependence in the CSV output, but the
real problem turns out to be that the code is broken on all platforms,
but only fails on big-endian platforms because the
incorrect-but-presumed-correct output was captured on a little-endian
processor.

The problem arises when the type 6 or type 8 case is supported by the
AIVDM parser but not by the CSV formatter. When the parser doesn't
support the case, it falls back to capturing the raw binary
bitdata. When the formatter doesn't support the case, it falls back to
dumping the raw binary bitdata. But in the offending cases, the raw
data was never correctly captured, and is instead an overlaid view of
the structured data, which is incorrect (as raw data) in all cases as
well as being endian-sensitive due to the presence of multibyte
quantities in host byte order.

The only way to make the CSV fallback work as intended would be to
change the struct ais_t type6 and type8 substructure definitions to
move the bitdata field out of the union and populate it
unconditionally. But this would increase the memory footprint and add
to the core code for the sole purpose of providing more useful output
in these cases of missing CSV formatting support, which doesn't seem
very justifiable.

The alternative is simply not to report the unavailable raw bitdata at
all in these cases, but instead to report something indicating that
the raw data is unknown. I've implemented a version of this where the
formatting remains the same (including the bit count) but the hex data
is replaced by "x" characters (which seemed safer than using "?",
given the absence of the latter in any existing cases.

After implementing the code change and before updating the test data,
I saw one additional miscompare that I'd not seen on the PPC. This was
for the 6,1,12 case, where the existing test data is too incomplete to
populate the endian-sensitive fields in the decoded form, but which
would have failed with a better version of the test case.

After updating the test data as well as the code, the test passes on
both x86_64 and PPC.

P.S.:

There are three additional uses of gpsd_hexdump in the CSV output, for
types 17, 25, and 26, but those all seem to populate the bidata field
unconditionally (and type 17 doesn't even have a "structured" flag),
so I left them alone.

Clearly the right fix would be to implement the missing CSV formatting
cases, which would render the fallback code unreachable, but it could
become reachable again if new cases were implemented in the parser and
not in the CSV formatter, so having a well-behaved fallback seems like
a good idea.

1 files changed, 28 insertions, 6 deletions

diff --git a/gpsdecode.c b/gpsdecode.c
index 17114c59..23c7684b 100644
--- a/gpsdecode.c
+++ b/gpsdecode.c
@@ -31,6 +31,26 @@ static struct gps_context_t context;
  **************************************************************************/
 
 #ifdef AIVDM_ENABLE
+static const char *raw_hexdump(char *scbuf, size_t scbuflen, int structured,
+					  char *binbuf, size_t binbuflen)
+{
+    if (!structured)
+	return gpsd_hexdump(scbuf, scbuflen, binbuf, binbuflen);
+/* Data parsed as structured doesn't have correct raw data */
+#ifndef SQUELCH_ENABLE
+    size_t len =
+	(size_t) ((binbuflen >
+		   MAX_PACKET_LENGTH) ? MAX_PACKET_LENGTH : binbuflen) * 2;
+    if (len > scbuflen - 1) len = scbuflen - 1;
+
+    memset(scbuf, 'x', len);
+    scbuf[len] = '\0';
+#else /* SQUELCH defined */
+    scbuf[0] = '\0';
+#endif /* SQUELCH_ENABLE */
+    return scbuf;
+}
+
 static void aivdm_csv_dump(struct ais_t *ais, char *buf, size_t buflen)
 {
     char scratchbuf[MAX_PACKET_LENGTH*2+1];
@@ -124,9 +144,10 @@ static void aivdm_csv_dump(struct ais_t *ais, char *buf, size_t buflen)
 	    str_appendf(buf, buflen,
 			   "|%zd:%s",
 			   ais->type6.bitcount,
-			   gpsd_hexdump(scratchbuf, sizeof(scratchbuf),
-					ais->type6.bitdata,
-					BITS_TO_BYTES(ais->type6.bitcount)));
+			   raw_hexdump(scratchbuf, sizeof(scratchbuf),
+				       ais->type6.structured,
+				       ais->type6.bitdata,
+				       BITS_TO_BYTES(ais->type6.bitcount)));
 	break;
     case 7:			/* Binary Acknowledge */
     case 13:			/* Safety Related Acknowledge */
@@ -229,9 +250,10 @@ static void aivdm_csv_dump(struct ais_t *ais, char *buf, size_t buflen)
 	    str_appendf(buf, buflen,
 			   "|%zd:%s",
 			   ais->type8.bitcount,
-			   gpsd_hexdump(scratchbuf, sizeof(scratchbuf),
-					ais->type8.bitdata,
-					BITS_TO_BYTES(ais->type8.bitcount)));
+			   raw_hexdump(scratchbuf, sizeof(scratchbuf),
+				       ais->type8.structured,
+				       ais->type8.bitdata,
+				       BITS_TO_BYTES(ais->type8.bitcount)));
 	break;
     case 9:
 	str_appendf(buf, buflen,