Sanitise locale code, comments and documentation

4 files changed, 71 insertions, 41 deletions

diff --git a/fpconv.c b/fpconv.c
index fd4deb0..3bcef41 100644
--- a/fpconv.c
+++ b/fpconv.c
@@ -1,3 +1,9 @@
+/* JSON uses a '.' decimal separator. strtod() / sprintf() under C libraries
+ * with locale support will break when the decimal separator is a comma.
+ *
+ * fpconv_* will around these issues with a translation buffer if required.
+ */
+
 #include <stdio.h>
 #include <stdlib.h>
 #include <assert.h>
@@ -5,15 +11,21 @@
 
 #include "fpconv.h"
 
+/* Lua CJSON assumes the locale is the same for all threads within a
+ * process and doesn't change after initialisation.
+ *
+ * This avoids the need for per thread storage or expensive checks
+ * for call. */
 static char locale_decimal_point = '.';
 
 /* In theory multibyte decimal_points are possible, but
  * Lua CJSON only supports UTF-8 and known locales only have
  * single byte decimal points ([.,]).
  *
- * localconv() may not be thread safe, and nl_langinfo() is not
- * supported on some platforms. Use sprintf() instead. */
-void fpconv_update_locale()
+ * localconv() may not be thread safe (=>crash), and nl_langinfo() is
+ * not supported on some platforms. Use sprintf() instead - if the
+ * locale does change, at least Lua CJSON won't crash. */
+static void fpconv_update_locale()
 {
     char buf[8];
 
@@ -29,11 +41,14 @@ void fpconv_update_locale()
     locale_decimal_point = buf[1];
 }
 
-/* Check for a valid number character: [-+0-9a-fA-FpPxX.]
- * It doesn't matter if actual invalid characters are counted - strtod()
- * will find the valid number if it exists. The risk is that slightly more
- * memory might be allocated before a parse error occurs. */
-static int valid_number_character(char ch)
+/* Check for a valid number character: [-+0-9a-yA-Y.]
+ * Eg: -0.6e+5, infinity, 0xF0.F0pF0
+ *
+ * Used to find the probable end of a number. It doesn't matter if
+ * invalid characters are counted - strtod() will find the valid
+ * number if it exists.  The risk is that slightly more memory might
+ * be allocated before a parse error occurs. */
+static inline int valid_number_character(char ch)
 {
     char lower_ch;
 
@@ -42,9 +57,7 @@ static int valid_number_character(char ch)
     if (ch == '-' || ch == '+' || ch == '.')
         return 1;
 
-    /* Hex digits, exponent (e), base (p), "infinity",..
-     * The main purpose is to not include a "comma". If any other invalid
-     * characters are included, the will only generate a parse error later. */
+    /* Hex digits, exponent (e), base (p), "infinity",.. */
     lower_ch = ch | 0x20;
     if ('a' <= lower_ch && lower_ch <= 'y')
         return 1;
@@ -52,8 +65,8 @@ static int valid_number_character(char ch)
     return 0;
 }
 
-/* Calculate the size of the buffer required for a locale
- * conversion. Returns 0 if conversion is not required */
+/* Calculate the size of the buffer required for a strtod locale
+ * conversion. */
 static int strtod_buffer_size(const char *s)
 {
     const char *p = s;
@@ -68,38 +81,38 @@ static int strtod_buffer_size(const char *s)
  * character. Guaranteed to be called at the start of any valid number in a string */
 double fpconv_strtod(const char *nptr, char **endptr)
 {
-    char *num, *endnum, *dp;
-    int numlen;
+    char *buf, *endbuf, *dp;
+    int buflen;
     double value;
 
     /* System strtod() is fine when decimal point is '.' */
     if (locale_decimal_point == '.')
         return strtod(nptr, endptr);
 
-    numlen = strtod_buffer_size(nptr);
-    if (!numlen) {
+    buflen = strtod_buffer_size(nptr);
+    if (!buflen) {
         /* No valid characters found, standard strtod() return */
         *endptr = (char *)nptr;
         return 0;
     }
 
     /* Duplicate number into buffer */
-    num = malloc(numlen + 1);
-    if (!num) {
+    buf = malloc(buflen + 1);
+    if (!buf) {
         fprintf(stderr, "Out of memory");
         abort();
     }
-    memcpy(num, nptr, numlen);
-    num[numlen] = 0;
+    memcpy(buf, nptr, buflen);
+    buf[buflen] = 0;
 
     /* Update decimal point character if found */
-    dp = strchr(num, '.');
+    dp = strchr(buf, '.');
     if (dp)
         *dp = locale_decimal_point;
 
-    value = strtod(num, &endnum);
-    *endptr = (char *)&nptr[endnum - num];
-    free(num);
+    value = strtod(buf, &endbuf);
+    *endptr = (char *)&nptr[endbuf - buf];
+    free(buf);
 
     return value;
 }
@@ -142,7 +155,7 @@ int fpconv_g_fmt(char *str, double num, int precision)
     /* snprintf() to a buffer then translate for other decimal point characters */
     len = snprintf(buf, FPCONV_G_FMT_BUFSIZE, fmt, num);
 
-    /* Returned 'len' includes the null terminator */
+    /* Copy into target location. Translate decimal point if required */
     b = buf;
     do {
         *str++ = (*b == locale_decimal_point ? '.' : *b);
@@ -151,5 +164,10 @@ int fpconv_g_fmt(char *str, double num, int precision)
     return len;
 }
 
+void fpconv_init()
+{
+    fpconv_update_locale();
+}
+
 /* vi:ai et sw=4 ts=4:
  */
diff --git a/fpconv.h b/fpconv.h
index b8a6469..ea875c0 100644
--- a/fpconv.h
+++ b/fpconv.h
@@ -1,9 +1,12 @@
 /* Lua CJSON floating point conversion routines */
 
-/* Buffer larger than required to store the largest %.14g number */
+/* Buffer required to store the largest string representation of a double.
+ *
+ * Longest double printed with %.14g is 21 characters long:
+ * -1.7976931348623e+308 */
 # define FPCONV_G_FMT_BUFSIZE   32
 
-extern void fpconv_update_locale();
+extern void fpconv_init();
 extern int fpconv_g_fmt(char*, double, int);
 extern double fpconv_strtod(const char*, char**);
 
diff --git a/lua_cjson.c b/lua_cjson.c
index fe6d41a..175433a 100644
--- a/lua_cjson.c
+++ b/lua_cjson.c
@@ -571,8 +571,6 @@ static void json_append_number(lua_State *l, json_config_t *cfg,
         /* Some platforms may print -nan, just hard code it */
         strbuf_append_mem(json, "nan", 3);
     } else {
-        /* Longest double printed with %.14g is 21 characters long:
-         * -1.7976931348623e+308 */
         strbuf_ensure_empty_length(json, FPCONV_G_FMT_BUFSIZE);
         len = fpconv_g_fmt(strbuf_empty_ptr(json), num, cfg->encode_number_precision);
         strbuf_extend_length(json, len);
@@ -1283,9 +1281,8 @@ static int lua_cjson_new(lua_State *l)
         { NULL, NULL }
     };
 
-    /* Update the current locale for g_fmt/strtod.
-     * Using different locales per-thread is not supported. */
-    fpconv_update_locale();
+    /* Initialise number conversions */
+    fpconv_init();
 
     /* cjson module table */
     lua_newtable(l);
diff --git a/manual.txt b/manual.txt
index 810d3d1..3ccbce3 100644
--- a/manual.txt
+++ b/manual.txt
@@ -48,7 +48,7 @@ Build Options (#define)
 [horizontal]
 USE_INTERNAL_ISINF:: Workaround for Solaris platforms missing isinf().
 DISABLE_CJSON_GLOBAL:: Do not store module table in global "cjson"
-  variable.
+  variable. Redundant from Lua 5.2 onwards.
 
 
 Make
@@ -150,10 +150,10 @@ local cjson = require "cjson"
 local cjson2 = cjson.new()
 ------------
 
-Lua CJSON can be loaded via +require+. A global +cjson+ table is
-registered under Lua 5.1 to maintain backward compatibility. Lua CJSON
-does not register a global table under Lua 5.2 since this practice is
-discouraged.
+Lua CJSON can be loaded via the Lua +require+ function. A global
++cjson+ module table is registered under Lua 5.1 to maintain backward
+compatibility. Lua CJSON does not register a global table under Lua
+5.2 since this practice is discouraged.
 
 +cjson.new+ can be used to instantiate an independent copy of the Lua
 CJSON module. The new module has a separate persistent encoding
@@ -164,9 +164,21 @@ threads within a single Lua state provided the persistent encoding
 buffer is not shared. This can be achieved by one of the following
 methods:
 
-- Disabling the persistent encoding buffer with +encode_keep_buffer+.
-- Ensuring only a single thread calls +encode+ at a time.
-- Using a separate +cjson+ instantiation per pre-emptive thread.
+- Disabling the persistent encoding buffer with
+  +cjson.encode_keep_buffer+.
+- Ensuring each thread calls +cjson.encode+ at a time.
+- Using a separate +cjson+ module table per pre-emptive thread
+  (+cjson.new+).
+
+[NOTE]
+Lua CJSON uses ++strtod++(3) and ++snprintf++(3) to perform numeric
+conversion as they are usually well supported, fast and bug free.
+However, these functions require a workaround for JSON
+encoding/parsing under locales using a comma decimal separator. Lua
+CJSON detects the current locale during instantiation to determine
+whether a workaround is required. CJSON should be reinitialised via
++cjson.new+ if the locale of the current process changes. Different
+locales per thread are not supported.
 
 
 decode