From a21ef29759e110e6134ed7953c409cc7a89c9316 Mon Sep 17 00:00:00 2001 From: Rijn Buve Date: Thu, 28 May 2026 08:16:57 +0200 Subject: [PATCH 1/3] Fix critical and major code review issues - Replace memcmp-based NaN/Inf detection with isnan()/isinf() from (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 - 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 --- .gitignore | 1 + mapcodelib/mapcode_legacy.c | 14 +++---- mapcodelib/mapcoder.c | 31 +++++++-------- mapcodelib/mapcoder.h | 3 +- test/unittest.c | 75 +++++++++++++++++++++++++++++++++++++ 5 files changed, 99 insertions(+), 25 deletions(-) diff --git a/.gitignore b/.gitignore index 05788f6..63fdf13 100644 --- 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 index b897e8a..6587ece 100644 --- 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 index a495957..e7b0a4d 100644 --- 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 index 63e68b1..48d41f8 100644 --- a/mapcodelib/mapcoder.h +++ b/mapcodelib/mapcoder.h @@ -20,6 +20,7 @@ extern "C" { #endif +#include #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 index 7d88cbf..b5b9f4f 100644 --- 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) { From 794e31f71ec6ec0ee9e2ea024b1dc60fe4b12e84 Mon Sep 17 00:00:00 2001 From: Rijn Buve Date: Thu, 28 May 2026 08:48:08 +0200 Subject: [PATCH 2/3] updagted version --- mapcodelib/mapcoder.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mapcodelib/mapcoder.h b/mapcodelib/mapcoder.h index 48d41f8..918360d 100644 --- a/mapcodelib/mapcoder.h +++ b/mapcodelib/mapcoder.h @@ -58,7 +58,7 @@ extern "C" { #define MAPCODE_SUPPORT_LANGUAGE_UK #endif -#define MAPCODE_C_VERSION "2.5.6" +#define MAPCODE_C_VERSION "2.5.7" #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). From 85b4d82a0ea655d6b7d2db9dc42eaea306ad34d0 Mon Sep 17 00:00:00 2001 From: Rijn Buve Date: Thu, 28 May 2026 13:27:49 +0200 Subject: [PATCH 3/3] updated README --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 8e178f7..6f0b46d 100644 --- a/README.md +++ b/README.md @@ -3,7 +3,7 @@ [![Codacy Badge](https://api.codacy.com/project/badge/Grade/d2e3d7a469484bfd8b801ce94d3f1737)](https://www.codacy.com/app/rijnb/mapcode-cpp?utm_source=github.com&utm_medium=referral&utm_content=mapcode-foundation/mapcode-cpp&utm_campaign=Badge_Grade) [![License](http://img.shields.io/badge/license-APACHE2-blue.svg)]() -**Copyright (C) 2014-2025 Stichting Mapcode Foundation (http://www.mapcode.com)** +**Copyright (C) 2014-2026 Stichting Mapcode Foundation (http://www.mapcode.com)** **Online documentation: http://mapcode-foundation.github.io/mapcode-cpp/**