Skip to content

Commit

Permalink
Make more JSON number handling improvements. (#136)
Browse files Browse the repository at this point in the history
* Make more JSON number handling improvements.

- Incorporate Ray's improvements to n_ftoa.c and n_atof.c that allow us to
support larger exponents.
- Add JAtoN and JNtoA unit tests to test these improvements.
- If NOTE_C_LOW_MEM is defined, define JNUMBER to float. On truly low memory
platforms, this likely does nothing, because those platforms often map double
and float to the same underlying type (which is single-precision). However, when
running the unit tests on a CPU with double-precision floating point support,
this allows us to deliberately run the unit tests with JNUMBER set to
single-precision. Doing this broke a couple test cases in
JSON_number_handling_test.cpp, which I've fixed here.

* Add NOTE_C_TEST_SINGLE_PRECISION.

This `#define` forces `JNUMBER` to be `float`, rather than `double`. This allows
us to run the unit tests with single precision floats, even on a machine that
supports double precision. This is useful coverage because a lot of small MCUs
don't support double precision.

* Simplify JAtoN.

Remove the code blocks guarded by `#ifndef LARGE_EXPONENT_FIX` (i.e. make the
code behave like LARGE_EXPONENT_FIX is always defined). Remove the
LARGE_EXPONENT_FIX define.
  • Loading branch information
haydenroche5 authored Jan 29, 2024
1 parent 5428d5a commit c1c6d0a
Show file tree
Hide file tree
Showing 10 changed files with 189 additions and 18 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ jobs:

- name: Run tests with NOTE_LOWMEM defined
run: |
docker run --rm --volume $(pwd):/note-c/ --workdir /note-c/ --entrypoint ./scripts/run_unit_tests.sh ghcr.io/blues/note_c_ci:latest --mem-check --low-mem
docker run --rm --volume $(pwd):/note-c/ --workdir /note-c/ --entrypoint ./scripts/run_unit_tests.sh ghcr.io/blues/note_c_ci:latest --mem-check --low-mem --single-precision
run_astyle:
runs-on: ubuntu-latest
Expand Down
9 changes: 9 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ option(NOTE_C_MEM_CHECK "Run tests with Valgrind." OFF)
option(NOTE_C_BUILD_DOCS "Build docs." OFF)
option(NOTE_C_NO_LIBC "Build the library without linking against libc, generating errors for any undefined symbols." OFF)
option(NOTE_C_LOW_MEM "Build the library tailored for low memory usage." OFF)
option(NOTE_C_TEST_SINGLE_PRECISION "Use single precision for JSON floating point numbers." OFF)

set(NOTE_C_SRC_DIR ${CMAKE_CURRENT_SOURCE_DIR})
add_library(note_c SHARED)
Expand Down Expand Up @@ -84,6 +85,14 @@ if(NOTE_C_NO_LIBC)
)
endif()

if(NOTE_C_TEST_SINGLE_PRECISION)
target_compile_definitions(
note_c
PUBLIC
NOTE_C_TEST_SINGLE_PRECISION
)
endif()

if(NOTE_C_BUILD_TESTS)
# Including CTest here rather than in test/CMakeLists.txt allows us to run
# ctest from the root build directory (e.g. build/ instead of build/test/).
Expand Down
14 changes: 6 additions & 8 deletions n_atof.c
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ char **endPtr; /* If non-NULL, store terminating character's
* address here. */
{
int sign, expSign = FALSE;
JNUMBER fraction, dblExp;
JNUMBER fraction;
register const char *p;
register int c;
int exp = 0; /* Exponent read from "EX" field. */
Expand Down Expand Up @@ -231,7 +231,6 @@ char **endPtr; /* If non-NULL, store terminating character's
if (exp > MAX_EXPONENT) {
exp = MAX_EXPONENT;
}
dblExp = 1.0;
int d;
for (d = 0; exp != 0; exp >>= 1, d += 1) {
/* Table giving binary powers of 10. Entry */
Expand Down Expand Up @@ -273,14 +272,13 @@ char **endPtr; /* If non-NULL, store terminating character's
break;
}
if (exp & 01) {
dblExp *= p10;
if (expSign) {
fraction /= p10;
} else {
fraction *= p10;
}
}
}
if (expSign) {
fraction /= dblExp;
} else {
fraction *= dblExp;
}

done:
if (endPtr != NULL) {
Expand Down
23 changes: 14 additions & 9 deletions n_ftoa.c
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ fmtflt(char *str, size_t *len, size_t size, JNUMBER fvalue, int width,
const char *infnan = NULL;
char iconvert[JNTOA_MAX];
char fconvert[JNTOA_MAX];
char econvert[4]; /* "e-12" (without nul-termination). */
char econvert[6]; /* "e-1024" (without nul-termination). */
char esign = 0;
char sign = 0;
int leadfraczeros = 0;
Expand Down Expand Up @@ -258,12 +258,17 @@ fmtflt(char *str, size_t *len, size_t size, JNUMBER fvalue, int width,
}

/*
* Convert the exponent. The sizeof(econvert) is 4. So, the
* econvert buffer can hold e.g. "e+99" and "e-99". We don't
* support an exponent which contains more than two digits.
* Therefore, the following stores are safe.
* Convert the exponent. The sizeof(econvert) is 6. So, the
* econvert buffer can hold e.g. "e+1024" and "e-1024".
*/
epos = convert(exponent, econvert, 2, 10, 0);
size_t digits = 2;
if (exponent > 99 || exponent < -99) {
digits++;
}
if (exponent > 999 || exponent < -999) {
digits++;
}
epos = convert(exponent, econvert, digits, 10, 0);
/*
* C99 says: "The exponent always contains at least two digits,
* and only as many more digits as necessary to represent the
Expand Down Expand Up @@ -424,16 +429,16 @@ static int getexponent(JNUMBER value)
int exponent = 0;

/*
* We check for 99 > exponent > -99 in order to work around possible
* We check for 1023 >= exponent >= -1022 in order to work around possible
* endless loops which could happen (at least) in the second loop (at
* least) if we're called with an infinite value. However, we checked
* for infinity before calling this function using our ISINF() macro, so
* this might be somewhat paranoid.
*/
while (tmp < 1.0 && tmp > 0.0 && --exponent > -99) {
while (tmp < 1.0 && tmp > 0.0 && --exponent >= -1022) {
tmp *= 10;
}
while (tmp >= 10.0 && ++exponent < 99) {
while (tmp >= 10.0 && ++exponent <= 1023) {
tmp /= 10;
}

Expand Down
4 changes: 4 additions & 0 deletions note.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,11 @@
#define ERRDBG
#endif

#ifdef NOTE_C_TEST_SINGLE_PRECISION
typedef float JNUMBER;
#else
typedef double JNUMBER;
#endif

typedef int64_t JINTEGER;
#define JINTEGER_MIN INT64_MIN
Expand Down
5 changes: 5 additions & 0 deletions scripts/run_unit_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@
COVERAGE=0
MEM_CHECK=0
LOW_MEM=0
SINGLE_PRECISION=0

while [[ "$#" -gt 0 ]]; do
case $1 in
--coverage) COVERAGE=1 ;;
--mem-check) MEM_CHECK=1 ;;
--low-mem) LOW_MEM=1 ;;
--single-precision) SINGLE_PRECISION=1 ;;
*) echo "Unknown parameter: $1"; exit 1 ;;
esac
shift
Expand Down Expand Up @@ -42,6 +44,9 @@ fi
if [[ $LOW_MEM -eq 1 ]]; then
CMAKE_OPTIONS="${CMAKE_OPTIONS} -DNOTE_C_LOW_MEM=1"
fi
if [[ $SINGLE_PRECISION -eq 1 ]]; then
CMAKE_OPTIONS="${CMAKE_OPTIONS} -DNOTE_C_TEST_SINGLE_PRECISION=1"
fi

cmake -B build/ $CMAKE_OPTIONS
if [[ $? -ne 0 ]]; then
Expand Down
2 changes: 2 additions & 0 deletions test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,8 @@ add_test(NoteBinaryStoreTransmit_test)
add_test(JPrintUnformatted_test)
add_test(Jtolower_test)
add_test(JSON_number_handling_test)
add_test(JAtoN_test)
add_test(JNtoA_test)

if(NOTE_C_COVERAGE)
find_program(LCOV lcov REQUIRED)
Expand Down
64 changes: 64 additions & 0 deletions test/src/JAtoN_test.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*!
* @file JAtoN_test.cpp
*
* Written by the Blues Inc. team.
*
* Copyright (c) 2024 Blues Inc. MIT License. Use of this source code is
* governed by licenses granted by the copyright holder including that found in
* the
* <a href="https://github.com/blues/note-c/blob/master/LICENSE">LICENSE</a>
* file.
*
*/

#include <catch2/catch_test_macros.hpp>
#include <cmath>

#include "n_lib.h"

namespace
{

#ifdef NOTE_C_LOW_MEM
const JNUMBER expectedNums[] = {
3.40282e+38, // Approximate largest single-precision float value
1.17549e-38, // Approximate smallest single-precision float value
};

const char *numStrs[] = {
"3.40282e+38",
"1.17549e-38"
};
#else
const JNUMBER expectedNums[] = {
1.79769e+308, // Approximate largest double-precision float value
2.22507e-308, // Approximate smallest double-precision float value
};

const char *numStrs[] = {
"1.79769e+308",
"2.22507e-308"
};
#endif // NOTE_C_LOW_MEM

const size_t NUM_TESTS = sizeof(expectedNums) / sizeof(expectedNums[0]);
const JNUMBER TOLERANCE = 1e-15;

SCENARIO("JAtoN")
{
for (size_t i = 0; i < NUM_TESTS; ++i) {
GIVEN(std::string("The string to convert to a JNUMBER is ") + numStrs[i]) {
WHEN("JAtoN is called on that string") {
JNUMBER num = JAtoN(numStrs[i], NULL);

THEN(std::string("The number returned is (approximately) ") + numStrs[i]) {
JNUMBER diff = fabs(num - expectedNums[i]);
JNUMBER err = diff / expectedNums[i];
CHECK(err < TOLERANCE);
}
}
}
}
}

}
71 changes: 71 additions & 0 deletions test/src/JNtoA_test.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*!
* @file JNtoA_test.cpp
*
* Written by the Blues Inc. team.
*
* Copyright (c) 2024 Blues Inc. MIT License. Use of this source code is
* governed by licenses granted by the copyright holder including that found in
* the
* <a href="https://github.com/blues/note-c/blob/master/LICENSE">LICENSE</a>
* file.
*
*/

#include <catch2/catch_test_macros.hpp>
#include <cmath>

#include "n_lib.h"

namespace
{

#ifdef NOTE_C_LOW_MEM
const JNUMBER nums[] = {
3.40282e+38, // Approximate largest single-precision float value
1.17549e-38, // Approximate smallest single-precision float value
};

const char *expectedNumStrs[] = {
"3.40282e+38",
"1.17549e-38"
};

const JNUMBER TOLERANCE = 1e-8;
#else
const JNUMBER nums[] = {
1.79769e+308, // Approximate largest double-precision float value
2.22507e-308, // Approximate smallest double-precision float value
};

const char *expectedNumStrs[] = {
"1.79769e+308",
"2.22507e-308"
};

const JNUMBER TOLERANCE = 1e-8;
#endif // NOTE_C_LOW_MEM

const size_t NUM_TESTS = sizeof(nums) / sizeof(nums[0]);

SCENARIO("JNtoA")
{
char numStr[JNTOA_MAX] = {0};

for (size_t i = 0; i < NUM_TESTS; ++i) {
GIVEN(std::string("The number to convert to a string is ") + expectedNumStrs[i]) {
WHEN("JNtoA is called on that number") {
JNtoA(nums[i], numStr, -1);

THEN(std::string("The string returned is (approximately)") + expectedNumStrs[i]) {
double extractedNum;
sscanf(expectedNumStrs[i], "%lf", &extractedNum);
JNUMBER diff = fabs(extractedNum - nums[i]);
JNUMBER err = diff / nums[i];
CHECK(err < TOLERANCE);
}
}
}
}
}

}
13 changes: 13 additions & 0 deletions test/src/JSON_number_handling_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -258,8 +258,14 @@ SCENARIO("Marshalling")

GIVEN("A JSON object with a numeric field with the max value of JINTEGER "
"plus 4096") {
#ifdef NOTE_C_LOW_MEM
// In the NOTE_C_LOW_MEM case, where JNUMBER is a single-precision
// float, JINTEGER_MAX_PLUS_4096 is indistinguishable from JINTEGER_MAX.
const char expected[] = "{\"" FIELD "\":" JINTEGER_MAX_STR "}";
#else
const char expected[] = "{\"" FIELD "\":" \
JINTEGER_MAX_PLUS_4096_TO_FLOAT_STR "}";
#endif
obj = JCreateObject();
REQUIRE(obj != NULL);
REQUIRE(JAddNumberToObject(obj, FIELD, JINTEGER_MAX_PLUS_4096) != NULL);
Expand Down Expand Up @@ -325,8 +331,15 @@ SCENARIO("Marshalling")

GIVEN("A J object with a numeric field with the min value of JINTEGER minus"
" 4096") {
#ifdef NOTE_C_LOW_MEM
// In the NOTE_C_LOW_MEM case, where JNUMBER is a single-precision
// float, JINTEGER_MIN_MINUS_4096 is indistinguishable from
// JINTEGER_MIN.
const char expected[] = "{\"" FIELD "\":" JINTEGER_MIN_STR "}";
#else
const char expected[] = "{\"" FIELD "\":" \
JINTEGER_MIN_MINUS_4096_TO_FLOAT_STR "}";
#endif
obj = JCreateObject();
REQUIRE(obj != NULL);
REQUIRE(JAddNumberToObject(obj, FIELD, JINTEGER_MIN_MINUS_4096) != NULL);
Expand Down

0 comments on commit c1c6d0a

Please sign in to comment.