Fix critical and major code review issues

- Replace memcmp-based NaN/Inf detection with isnan()/isinf() from <math.h> (portable, correct for all NaN bit patterns, and now checks latDeg for Inf too) - Fix off-by-one in encodeLatLonToSelectedMapcode: > should be >= for index bound - Remove always-false ASSERT after *s=0 in encodeExtension - Fix convertFromAbjad/convertToAbjad to null-check strchr return before arithmetic - Allow TERRITORY_NONE/TERRITORY_UNKNOWN in encodeLatLonToSingleMapcode per docs - Fix convertUtf16ToUtf8 to return start pointer instead of post-null end pointer - Explicitly initialize GLOBAL_MAKEISO_PTR (was accidentally correct via NULL) - Replace sprintf with snprintf in convertToRoman (mapcode_legacy.c) - Change UWORD from unsigned short int to uint16_t; add #include <stdint.h> - Replace magic constant 128 with MAX_MAPCODE_RESULT_ASCII_LEN in encoderEngine - Add regression tests for all fixed bugs in testBugFixes() Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rijnb · May 28, 2026 · May 28, 2026 · May 28, 2026 · May 28, 2026 · May 28, 2026
commit 47e0d3220b7868fca2bdcaa3f360f2ae707d03f2
diff --git a/.gitignore b/.gitignore
@@ -132,3 +132,4 @@ Release
 # -----------------------------------------------------------------------------
 nb-configuration.xml
 *.orig
+.gitnexus
diff --git a/mapcodelib/mapcode_legacy.c b/mapcodelib/mapcode_legacy.c
@@ -23,7 +23,7 @@
  */
 static Mapcodes GLOBAL_RESULT;
 static char GLOBAL_MAKEISO_BUFFER[2 * (MAX_ISOCODE_LEN + 1)];
-static char* GLOBAL_MAKEISO_PTR;
+static char* GLOBAL_MAKEISO_PTR = GLOBAL_MAKEISO_BUFFER + (MAX_ISOCODE_LEN + 1);
 
 
 int encodeLatLonToMapcodes_Deprecated(
@@ -97,12 +97,12 @@ char* convertToRoman(char* asciiBuffer, int maxLength, const UWORD* unicodeBuffe
     }
     if (!err) {
         char romanized[MAX_MAPCODE_RESULT_LEN];
-        sprintf(romanized, "%s%s%s%s%s",
-                mapcodeElements.territoryISO,
-                *mapcodeElements.territoryISO ? " " : "",
-                mapcodeElements.properMapcode,
-                *mapcodeElements.precisionExtension ? "-" : "",
-                mapcodeElements.precisionExtension);
+        snprintf(romanized, sizeof(romanized), "%s%s%s%s%s",
+                 mapcodeElements.territoryISO,
+                 *mapcodeElements.territoryISO ? " " : "",
+                 mapcodeElements.properMapcode,
+                 *mapcodeElements.precisionExtension ? "-" : "",
+                 mapcodeElements.precisionExtension);
         if ((int)strlen(romanized) < maxLength) {
             strcpy(asciiBuffer, romanized);
         }

diff --git a/mapcodelib/mapcoder.c b/mapcodelib/mapcoder.c
@@ -534,17 +534,11 @@ static Point convertFractionsToDegrees(const Point* p) {
 }
 
 
-static const unsigned char DOUBLE_NAN[8] = {0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x7F}; // NAN (Not a Number)
-static const unsigned char DOUBLE_INF[8] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xF0, 0x7F}; // +Infinity
-static const unsigned char DOUBLE_MIN_INF[8] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xF0, 0xFF}; // -Infinity
-
 static enum MapcodeError
 convertCoordsToMicrosAndFractions(Point32* coord32, int* fracLat, int* fracLon, double latDeg, double lonDeg) {
     double frac;
     ASSERT(coord32);
-    if (memcmp(&lonDeg, DOUBLE_NAN, 8) == 0 || memcmp(&lonDeg, DOUBLE_INF, 8) == 0 ||
-        memcmp(&lonDeg, DOUBLE_MIN_INF, 8) == 0 ||
-        memcmp(&latDeg, DOUBLE_NAN, 8) == 0) {
+    if (isnan(lonDeg) || isinf(lonDeg) || isnan(latDeg) || isinf(latDeg)) {
         return ERR_BAD_COORDINATE;
     }
     if (latDeg < -90) {
@@ -1081,7 +1075,6 @@ static void encodeExtension(char* result, const int extrax4, const int extray, c
             valy -= factory * gy; // for next iteration
         }
         *s = 0; // terminate the result
-        ASSERT((int) strlen(s) == extraDigits);
     }
 }
 
@@ -1487,7 +1480,7 @@ static void encoderEngine(const enum Territory ccode, const EncodeRec* enc, cons
     ///////////////////////////////////////////////////////////
     {
         int i;
-        char result[128];
+        char result[MAX_MAPCODE_RESULT_ASCII_LEN];
         int result_counter = 0;
 
         *result = 0;
@@ -1736,7 +1729,7 @@ static int decodeBase31(const char* code) {
 static void decodeTriple(const char* result, int* difx, int* dify) {
     // decode the first character
     const int c1 = decodeChar(*result++);
-    ASSERT(result);
+    ASSERT(result - 1);
     ASSERT(difx);
     ASSERT(dify);
     if (c1 < 24) {
@@ -2125,7 +2118,10 @@ static unsigned char getRomanVersionOf(UWORD w) {
 static void convertFromAbjad(char* s) {
     int len, dot, form, c;
     char* postfix = strchr(s, '-');
-    dot = (int)(strchr(s, '.') - s);
+    {
+        const char* dotptr = strchr(s, '.');
+        dot = dotptr ? (int)(dotptr - s) : -1;
+    }
     if (dot < 2 || dot > 5) {
         return;
     }
@@ -3020,7 +3016,10 @@ static char* convertToAbjad(char* targetAsciiString, const char* sourceAsciiStri
     unpackIfAllDigits(targetAsciiString);
 
     len = (int)strlen(targetAsciiString);
-    dot = (int)(strchr(targetAsciiString, '.') - targetAsciiString);
+    {
+        const char* dotptr = strchr(targetAsciiString, '.');
+        dot = dotptr ? (int)(dotptr - targetAsciiString) : -1;
+    }
 
     form = dot * 10 + (len - dot - 1);
 
@@ -3297,6 +3296,7 @@ UWORD* convertToAlphabet(UWORD* utf16String, int maxLength, const char* asciiStr
  * Convert a zero-terminated UTF16 to a UTF8 string
  */
 char* convertUtf16ToUtf8(char* utf8, const UWORD* utf16) {
+    char* start = utf8;
     ASSERT(utf16);
     ASSERT(utf8);
     while (*utf16) {
@@ -3315,7 +3315,7 @@ char* convertUtf16ToUtf8(char* utf8, const UWORD* utf16) {
         }
     }
     *utf8 = 0;
-    return utf8;
+    return start;
 }
 
 // Caller must make sure utf8String can hold at least MAX_MAPCODE_RESULT_LEN characters (including 0-terminator).
@@ -3585,9 +3585,6 @@ encodeLatLonToSingleMapcode(char* mapcode, double latDeg, double lonDeg, enum Te
     if (extraDigits > MAX_PRECISION_DIGITS) {
         extraDigits = MAX_PRECISION_DIGITS;
     }
-    if (territory <= TERRITORY_UNKNOWN) {
-        return 0;
-    }
     ret = encodeLatLonToMapcodes_internal(&rlocal, latDeg, lonDeg, territory, 1, DEBUG_STOP_AT, extraDigits);
     *mapcode = 0;
     if (ret <= 0) {
@@ -3608,7 +3605,7 @@ encodeLatLonToSelectedMapcode(char* mapcode, double latDeg, double lonDeg, enum
     int nrOfResults = 0;
     nrOfResults = encodeLatLonToMapcodes(&mapcodes, latDeg, lonDeg, territory, extraDigits);
     ASSERT(nrOfResults == mapcodes.count);
-    if ((nrOfResults <= 0) || (indexOfSelected < 0) || (indexOfSelected > nrOfResults)) {
+    if ((nrOfResults <= 0) || (indexOfSelected < 0) || (indexOfSelected >= nrOfResults)) {
         return 0;
     }
     strcpy(mapcode, mapcodes.mapcode[indexOfSelected]);

diff --git a/mapcodelib/mapcoder.h b/mapcodelib/mapcoder.h
@@ -20,6 +20,7 @@ extern "C" {
 
 #endif
 
+#include <stdint.h>
 #include "mapcode_territories.h"
 #include "mapcode_alphabets.h"
 
@@ -58,7 +59,7 @@ extern "C" {
 #endif
 
 #define MAPCODE_C_VERSION                   "2.5.6"
-#define UWORD                               unsigned short int  // 2-byte unsigned integer.
+#define UWORD                               uint16_t            // 2-byte unsigned integer.
 
 #define MAX_NR_OF_MAPCODE_RESULTS           22                  // Max. number of results ever returned by encoder (e.g. for 26.904899, 95.138515).
 

diff --git a/test/unittest.c b/test/unittest.c
@@ -2222,6 +2222,78 @@ static int testAlphabetPerTerritory(void) {
 }
 
 
+static int testBugFixes(void) {
+    int nrTests = 0;
+
+    // Issue 1: lat=+Inf or lat=-Inf must return ERR_BAD_COORDINATE (0 results), same as lon=Inf
+    {
+        Mapcodes mapcodes;
+        double pos_inf = 1.0 / 0.0;
+        double neg_inf = -1.0 / 0.0;
+        int n;
+
+        n = encodeLatLonToMapcodes(&mapcodes, pos_inf, 0.0, TERRITORY_NONE, 0);
+        ++nrTests;
+        if (n != 0) {
+            foundError();
+            printf("*** ERROR *** encodeLatLonToMapcodes(lat=+Inf) should return 0, got %d\n", n);
+        }
+
+        n = encodeLatLonToMapcodes(&mapcodes, neg_inf, 0.0, TERRITORY_NONE, 0);
+        ++nrTests;
+        if (n != 0) {
+            foundError();
+            printf("*** ERROR *** encodeLatLonToMapcodes(lat=-Inf) should return 0, got %d\n", n);
+        }
+    }
+
+    // Issue 2: indexOfSelected == nrOfResults is out-of-bounds and must return 0
+    {
+        char mapcode[MAX_MAPCODE_RESULT_ASCII_LEN];
+        // lat=52.158993, lon=4.492346 yields 4 results (verified in testSelectedEncodes)
+        int n = encodeLatLonToSelectedMapcode(mapcode, 52.158993, 4.492346, TERRITORY_NONE, 0, 4);
+        ++nrTests;
+        if (n != 0) {
+            foundError();
+            printf("*** ERROR *** encodeLatLonToSelectedMapcode with indexOfSelected==nrOfResults should return 0, got %d\n", n);
+        }
+    }
+
+    // Issue 7: encodeLatLonToSingleMapcode must accept TERRITORY_NONE and TERRITORY_UNKNOWN
+    {
+        char mapcode[MAX_MAPCODE_RESULT_ASCII_LEN];
+        int n;
+
+        n = encodeLatLonToSingleMapcode(mapcode, 52.3, 4.9, TERRITORY_UNKNOWN, 0);
+        ++nrTests;
+        if (n <= 0) {
+            foundError();
+            printf("*** ERROR *** encodeLatLonToSingleMapcode(TERRITORY_UNKNOWN) should succeed, got %d\n", n);
+        }
+
+        n = encodeLatLonToSingleMapcode(mapcode, 52.3, 4.9, TERRITORY_NONE, 0);
+        ++nrTests;
+        if (n <= 0) {
+            foundError();
+            printf("*** ERROR *** encodeLatLonToSingleMapcode(TERRITORY_NONE) should succeed, got %d\n", n);
+        }
+    }
+
+    // Issue 9: convertMapcodeToAlphabetUtf8 must return the start of the output buffer
+    {
+        char utf8[MAX_MAPCODE_RESULT_UTF8_LEN + 1];
+        char *result = convertMapcodeToAlphabetUtf8(utf8, "NLD 49.4V", ALPHABET_ROMAN);
+        ++nrTests;
+        if (result != utf8) {
+            foundError();
+            printf("*** ERROR *** convertMapcodeToAlphabetUtf8 must return start of buffer, got wrong pointer\n");
+        }
+    }
+
+    return nrTests;
+}
+
+
 int main(const int argc, const char **argv) {
     int nrTests = 0;
 
@@ -2281,6 +2353,9 @@ int main(const int argc, const char **argv) {
     printf("-----------------------------------------------------------\nRe-encode tests\n");
     nrTests += testReEncode();
 
+    printf("-----------------------------------------------------------\nBug-fix regression tests\n");
+    nrTests += testBugFixes();
+
     printf("-----------------------------------------------------------\n");
     printf("Done.\nExecuted %d tests, found %d errors\n", nrTests, nrErrors);
     if (nrErrors > 0) {