Improve JSON number handling (continued). (#133)

* Improve JSON number handling (continued).

- Change NOTE_LOWMEM to NOTE_C_LOW_MEM. If NOTE_C_LOW_MEM is defined, the old
macro NOTE_LOWMEM is automatically defined. Same for NOTE_FLOAT, which was also
removed.
- Change JItoA, JAtoI, NoteSetEnvDefaultInt, and NoteGetEnvInt to use JINTEGER
instead of long int.
- Make JINTEGER a typedef for int64_t. Don't bother trying to save a few bytes
in the NOTE_C_LOW_MEM case.
- Make JNUMBER a typedef for double. On low memory platforms, double is often
the same as float anyway (e.g. AVR).
- Improve comment explaining the JINTEGER_MIN case in JItoA.
- Simplify JSON_number_handling_test.cpp.

* Make more fixes to number handling and NOTE_C_LOW_MEM code.

- Remove #ifdef ERRDBG guard in i2cNoteReset. This was causing us to not call
_I2CReset when we should have been.
- Make JTIME a typedef for JUINTEGER.
- Fix run_unit_tests.sh script so that it actually runs the tests with
NOTE_C_LOW_MEM defined.
- Use JAddIntToObject to add Unix timestamps to J objects in unit tests. This is
necessary to avoid loss of precision when using JAddNumberToObject.

CMakeLists.txt

@@ -61,10 +61,10 @@ if(NOTE_C_LOW_MEM)
  target_compile_definitions(
  note_c
  PUBLIC
- NOTE_LOWMEM
+ NOTE_C_LOW_MEM
  )
 else()
- # This file is empty if NOTE_LOWMEM is defined, which leads to a warning
+ # This file is empty if NOTE_C_LOW_MEM is defined, which leads to a warning
  # about an empty translation unit, so we only add it to the build if
  # NOTE_C_LOW_MEM is false.
  target_sources(

n_atof.c

@@ -257,7 +257,7 @@ char **endPtr; /* If non-NULL, store terminating character's
  case 5:
  p10 = 1.0e32;
  break;
-#ifndef NOTE_LOWMEM
+#ifndef NOTE_C_LOW_MEM
  case 6:
  p10 = 1.0e64;
  break;

n_cjson_helpers.c

@@ -428,12 +428,15 @@ const char *JGetItemName(const J * item)
  @note The buffer must be large enough because no bounds checking is done.
 */
 /**************************************************************************/
-void JItoA(long int n, char *s)
+void JItoA(JINTEGER n, char *s)
 {
  char c;
  // Conversion to unsigned is required to handle the case where n is
- // LONG_MIN.
- unsigned long int unsignedN = n;
+ // JINTEGER_MIN. In that case, applying the unary minus operator to the
+ // signed version of n overflows and the behavior is undefined. By changing
+ // n to be unsigned, the unary minus operator behaves differently, and there
+ // is no overflow. See https://stackoverflow.com/q/8026694.
+ JUINTEGER unsignedN = n;
  long int i, j;
  if (n < 0) {
  unsignedN = -unsignedN;
@@ -460,9 +463,9 @@ void JItoA(long int n, char *s)
  @returns An integer, or 0 if invalid
 */
 /**************************************************************************/
-long int JAtoI(const char *string)
+JINTEGER JAtoI(const char *string)
 {
- long int result = 0;
+ JINTEGER result = 0;
  unsigned int digit;
  int sign;
  while (*string == ' ') {

n_helpers.c

@@ -1207,7 +1207,7 @@ bool NoteSetEnvDefault(const char *variable, char *buf)
  @returns boolean indicating if variable was set.
 */
 /**************************************************************************/
-bool NoteSetEnvDefaultInt(const char *variable, long int defaultVal)
+bool NoteSetEnvDefaultInt(const char *variable, JINTEGER defaultVal)
 {
  char buf[32];
  JItoA(defaultVal, buf);
@@ -1253,7 +1253,7 @@ JNUMBER NoteGetEnvNumber(const char *variable, JNUMBER defaultVal)
  @returns environment variable value.
 */
 /**************************************************************************/
-long int NoteGetEnvInt(const char *variable, long int defaultVal)
+JINTEGER NoteGetEnvInt(const char *variable, JINTEGER defaultVal)
 {
  char buf[32], buf2[32];
  JItoA(defaultVal, buf2);

n_hooks.c

@@ -491,7 +491,7 @@ void NoteDelayMs(uint32_t ms)
  }
 }
 
-#if NOTE_SHOW_MALLOC || !defined(NOTE_LOWMEM)
+#if NOTE_SHOW_MALLOC || !defined(NOTE_C_LOW_MEM)
 //**************************************************************************/
 /*!
  @brief Convert number to a hex string

n_i2c.c

@@ -302,7 +302,6 @@ bool i2cNoteReset(void)
  // then the Notecard has been successfully reset.
  if (!somethingFound || nonControlCharFound) {
  notecardReady = false;
-#ifdef ERRDBG
  if (somethingFound) {
  NOTE_C_LOG_WARN(ERRSTR("unrecognized data from notecard", c_iobad));
  } else {
@@ -315,7 +314,6 @@ bool i2cNoteReset(void)
  }
  delayIO();
  }
-#endif
  } else {
  notecardReady = true;
  break;

n_lib.h

@@ -94,13 +94,13 @@ extern "C" {
  @brief Memory allocation chunk size.
 */
 /**************************************************************************/
-#ifdef NOTE_LOWMEM
+#ifdef NOTE_C_LOW_MEM
 #define ALLOC_CHUNK 64
 #else
 #define ALLOC_CHUNK 128
 #endif
 
-#ifdef NOTE_LOWMEM
+#ifdef NOTE_C_LOW_MEM
 #define NOTE_DISABLE_USER_AGENT
 #endif
 

n_request.c

@@ -25,7 +25,7 @@ static int suppressShowTransactions = 0;
 static bool resetRequired = true;
 
 // CRC data
-#ifndef NOTE_LOWMEM
+#ifndef NOTE_C_LOW_MEM
 static uint16_t lastRequestSeqno = 0;
 #define CRC_FIELD_LENGTH 22 // ,"crc":"SSSS:CCCCCCCC"
 #define CRC_FIELD_NAME_OFFSET 1
@@ -451,7 +451,7 @@ J *noteTransactionShouldLock(J *req, bool lockNotecard)
  // as part of the CRC data so that two identical requests occurring within the
  // modulus of seqno never are mistaken as being the same request being retried.
  uint8_t lastRequestRetries = 0;
-#ifndef NOTE_LOWMEM
+#ifndef NOTE_C_LOW_MEM
  bool lastRequestCrcAdded = false;
  if (!noResponseExpected) {
  char *newJson = crcAdd(json, lastRequestSeqno);
@@ -461,7 +461,7 @@ J *noteTransactionShouldLock(J *req, bool lockNotecard)
  lastRequestCrcAdded = true;
  }
  }
-#endif // !NOTE_LOWMEM
+#endif // !NOTE_C_LOW_MEM
 
  // When note.add or web.* requests are used to transfer binary data, the
  // time to complete the transaction can vary depending on the size of
@@ -550,7 +550,7 @@ J *noteTransactionShouldLock(J *req, bool lockNotecard)
  continue;
  }
 
-#ifndef NOTE_LOWMEM
+#ifndef NOTE_C_LOW_MEM
  // If we sent a CRC in the request, examine the response JSON to see if
  // it has a CRC error. Note that the CRC is stripped from the
  // responseJSON as a side-effect of this method.
@@ -562,7 +562,7 @@ J *noteTransactionShouldLock(J *req, bool lockNotecard)
  _DelayMs(500);
  continue;
  }
-#endif // !NOTE_LOWMEM
+#endif // !NOTE_C_LOW_MEM
 
  // See if the response JSON can't be unmarshaled, or if it contains an {io} error
  rsp = JParse(responseJSON);
@@ -576,7 +576,7 @@ J *noteTransactionShouldLock(J *req, bool lockNotecard)
  if (responseJSON == NULL) {
  NOTE_C_LOG_ERROR(ERRSTR("response expected, but response is NULL.", c_ioerr));
  } else {
-#ifndef NOTE_LOWMEM
+#ifndef NOTE_C_LOW_MEM
  _DebugWithLevel(NOTE_C_LOG_LEVEL_ERROR, "[ERROR] ");
  _DebugWithLevel(NOTE_C_LOG_LEVEL_ERROR, "invalid JSON: ");
  _DebugWithLevel(NOTE_C_LOG_LEVEL_ERROR, responseJSON);
@@ -608,7 +608,7 @@ J *noteTransactionShouldLock(J *req, bool lockNotecard)
  }
 
  // Bump the request sequence number now that we've processed this request, success or error
-#ifndef NOTE_LOWMEM
+#ifndef NOTE_C_LOW_MEM
  lastRequestSeqno++;
 #endif
 
@@ -723,7 +723,7 @@ void NoteErrorClean(char *begin)
  }
 }
 
-#ifndef NOTE_LOWMEM
+#ifndef NOTE_C_LOW_MEM
 
 /*!
  @brief Convert a hex string to a 64-bit unsigned integer.
@@ -887,4 +887,4 @@ NOTE_C_STATIC bool crcError(char *json, uint16_t shouldBeSeqno)
  return (shouldBeSeqno != actualSeqno || shouldBeCrc32 != actualCrc32);
 }
 
-#endif // !NOTE_LOWMEM
+#endif // !NOTE_C_LOW_MEM

n_ua.c

@@ -11,7 +11,7 @@
  *
  */
 
-#ifndef NOTE_LOWMEM
+#ifndef NOTE_C_LOW_MEM
 
 #include "n_lib.h"
 

note.h

@@ -26,50 +26,47 @@
 #define NOTE_C_VERSION_PATCH 1
 
 // If double and float are the same size, then we must be on a small MCU. Turn
-// on NOTE_LOWMEM to conserve memory.
+// on NOTE_C_LOW_MEM to conserve memory.
 #if defined(FLT_MAX_EXP) && defined(DBL_MAX_EXP)
 #if (FLT_MAX_EXP == DBL_MAX_EXP)
-#define NOTE_LOWMEM
+#define NOTE_C_LOW_MEM
 #endif
 #elif defined(__FLT_MAX_EXP__) && defined(__DBL_MAX_EXP__)
 #if (__FLT_MAX_EXP__ == __DBL_MAX_EXP__)
-#define NOTE_LOWMEM
+#define NOTE_C_LOW_MEM
 #endif
 #else
 #error What are floating point exponent length symbols for this compiler?
 #endif
 
-#ifndef JNUMBER
+// NOTE_LOWMEM is the old name of NOTE_C_LOW_MEM. If NOTE_LOWMEM is defined,
+// we define NOTE_C_LOW_MEM as well, for backwards compatibility. NOTE_FLOAT is
+// also no longer used internally, but used to be defined when NOTE_LOWMEM was
+// defined. It's also preserved here for backwards compatibility.
 #ifdef NOTE_LOWMEM
-#define JNUMBER float
-#else
-#define JNUMBER double
-#endif
+#define NOTE_C_LOW_MEM
+#define NOTE_FLOAT
 #endif
 
-#ifndef JINTEGER
-#ifdef NOTE_LOWMEM
-#define JINTEGER int32_t
-#define JINTEGER_MIN INT32_MIN
-#define JINTEGER_MAX INT32_MAX
-#else
-#define JINTEGER int64_t
-#define JINTEGER_MIN INT64_MIN
-#define JINTEGER_MAX INT64_MAX
-#endif
-#endif
-
-#ifdef NOTE_LOWMEM
+#ifdef NOTE_C_LOW_MEM
 #define ERRSTR(x,y) (y)
 #else
 #define ERRSTR(x,y) (x)
 #define ERRDBG
 #endif
 
+typedef double JNUMBER;
+
+typedef int64_t JINTEGER;
+#define JINTEGER_MIN INT64_MIN
+#define JINTEGER_MAX INT64_MAX
+
+typedef uint64_t JUINTEGER;
+
 // UNIX Epoch time (also known as POSIX time) is the number of seconds that have elapsed since
 // 00:00:00 Thursday, 1 January 1970, Coordinated Universal Time (UTC). In this project, it always
 // originates from the Notecard, which synchronizes the time from both the cell network and GPS.
-typedef unsigned long int JTIME;
+typedef JUINTEGER JTIME;
 
 // C-callable functions
 #ifdef __cplusplus
@@ -433,8 +430,8 @@ int JBaseItemType(int type);
 #define JNTOA_MAX (44)
 char * JNtoA(JNUMBER f, char * buf, int precision);
 JNUMBER JAtoN(const char *string, char **endPtr);
-void JItoA(long int n, char *s);
-long int JAtoI(const char *s);
+void JItoA(JINTEGER n, char *s);
+JINTEGER JAtoI(const char *s);
 int JB64EncodeLen(int len);
 int JB64Encode(char * coded_dst, const char *plain_src,int len_plain_src);
 int JB64DecodeLen(const char * coded_src);
@@ -483,12 +480,12 @@ bool NoteRegion(char **retCountry, char **retArea, char **retZone, int *retZoneO
 bool NoteLocationValid(char *errbuf, uint32_t errbuflen);
 bool NoteLocationValidST(char *errbuf, uint32_t errbuflen);
 void NoteTurboIO(bool enable);
-long int NoteGetEnvInt(const char *variable, long int defaultVal);
+JINTEGER NoteGetEnvInt(const char *variable, JINTEGER defaultVal);
 JNUMBER NoteGetEnvNumber(const char *variable, JNUMBER defaultVal);
 bool NoteGetEnv(const char *variable, const char *defaultVal, char *buf, uint32_t buflen);
 bool NoteSetEnvDefault(const char *variable, char *buf);
 bool NoteSetEnvDefaultNumber(const char *variable, JNUMBER defaultVal);
-bool NoteSetEnvDefaultInt(const char *variable, long int defaultVal);
+bool NoteSetEnvDefaultInt(const char *variable, JINTEGER defaultVal);
 bool NoteIsConnected(void);
 bool NoteIsConnectedST(void);
 bool NoteGetNetStatus(char *statusBuf, int statusBufLen);

scripts/run_unit_tests.sh

@@ -39,7 +39,7 @@ if [[ $MEM_CHECK -eq 1 ]]; then
  # host machine is running Fedora. See https://stackoverflow.com/a/75293014.
  ulimit -n 1024
 fi
-if [[ $NOTE_C_LOW_MEM -eq 1 ]]; then
+if [[ $LOW_MEM -eq 1 ]]; then
  CMAKE_OPTIONS="${CMAKE_OPTIONS} -DNOTE_C_LOW_MEM=1"
 fi
 

test/src/JAtoI_test.cpp

@@ -11,8 +11,6 @@
  *
  */
 
-
-
 #include <catch2/catch_test_macros.hpp>
 
 #include "n_lib.h"
@@ -36,7 +34,7 @@ SCENARIO("JAtoI")
  " -50",
  "-50 "
  };
- long int nums[] = {
+ JINTEGER nums[] = {
  0,
  1,
  1000,
@@ -57,5 +55,3 @@ SCENARIO("JAtoI")
 }
 
 }
-
-