Merge pull request #26 from filip-stenstrom/fix_locale

Set LC_NUMERIC during parsing

.gitignore

@@ -4,3 +4,7 @@ testlogs-*/
 CMakeFiles/
 ThirdParty/winflexbison/
 doc/
+CMakeSettings.json
+.vs/
+out/
+enc_temp_folder/

CMakeLists.txt

@@ -62,6 +62,7 @@ option(FMILIB_LINK_TEST_TO_SHAREDLIB "Link the tests to fmilib_shared (if built)
 option(FMILIB_GENERATE_BUILD_STAMP "Generate a build time stamp and include in into the library" OFF)
 option(FMILIB_ENABLE_LOG_LEVEL_DEBUG "Enable log level 'debug'. If the option is of then the debug level is not compiled in." OFF)
 option(FMILIB_PRINT_DEBUG_MESSAGES "Enable printing of status messages from the build script. Intended for debugging." OFF)
+option(FMILIB_TEST_LOCALE          "Perform testing related to setting locales (requires certain language packs)" OFF)
 mark_as_advanced(FMILIB_PRINT_DEBUG_MESSAGES FMILIB_DEBUG_TRACE)
 
 if(NOT FMILIB_BUILD_SHARED_LIB AND NOT FMILIB_BUILD_STATIC_LIB)

Config.cmake/runtime_test.cmake

@@ -13,16 +13,25 @@
 
 set(RTTESTDIR ${FMILIBRARYHOME}/Test)
 
+# Test: jm_vector
 add_executable (jm_vector_test ${RTTESTDIR}/jm_vector_test.c)
 target_link_libraries (jm_vector_test ${JMUTIL_LIBRARIES})
 
+# Test: jm locale
+add_executable (jm_locale_test ${RTTESTDIR}/jm_locale_test.c)
+target_link_libraries (jm_locale_test ${JMUTIL_LIBRARIES})
+if(FMILIB_TEST_LOCALE)
+    target_compile_definitions(jm_locale_test PRIVATE -DFMILIB_TEST_LOCALE)
+endif()
+
 #Create function that zipz the dummy FMUs 
 add_executable (compress_test_fmu_zip ${RTTESTDIR}/compress_test_fmu_zip.c)
 target_link_libraries (compress_test_fmu_zip ${FMIZIP_LIBRARIES})
 
 set_target_properties(
-	jm_vector_test  compress_test_fmu_zip
+	jm_vector_test jm_locale_test compress_test_fmu_zip
     PROPERTIES FOLDER "Test")
+
 #Path to the executable
 get_property(COMPRESS_EXECUTABLE TARGET compress_test_fmu_zip PROPERTY LOCATION)
 
@@ -153,6 +162,8 @@ if(FMILIB_BUILD_BEFORE_TESTS)
 		COMMAND "${CMAKE_COMMAND}" --build ${FMILIBRARYBUILD} --config $<CONFIGURATION>)
 endif()
 
+add_test(ctest_jm_locale_test jm_locale_test)
+
 ADD_TEST(ctest_fmi_zip_unzip_test fmi_zip_unzip_test)
 ADD_TEST(ctest_fmi_zip_zip_test fmi_zip_zip_test)
 
@@ -177,4 +188,4 @@ if(FMILIB_BUILD_BEFORE_TESTS)
 		ctest_fmi_zip_zip_test
 		PROPERTIES DEPENDS ctest_build_all)
 endif()
-SET_TESTS_PROPERTIES ( ctest_fmi_import_test_no_xml PROPERTIES DEPENDS ctest_fmi_zip_unzip_test) 
+SET_TESTS_PROPERTIES ( ctest_fmi_import_test_no_xml PROPERTIES DEPENDS ctest_fmi_zip_unzip_test) 

Config.cmake/test_fmi2.cmake

@@ -77,6 +77,14 @@ to_native_c_path("${TEST_OUTPUT_FOLDER}/${FMU2_DUMMY_CS_MODEL_IDENTIFIER}_mf.fmu
 
 add_executable (fmi2_xml_parsing_test ${RTTESTDIR}/FMI2/fmi2_xml_parsing_test.c)
 target_link_libraries (fmi2_xml_parsing_test  ${FMILIBFORTEST})
+if(FMILIB_TEST_LOCALE)
+    set(FMI2_XML_PARSING_TEST_DEFINITIONS -DFMILIB_TEST_LOCALE)
+    if(UNIX)
+        list(APPEND FMI2_XML_PARSING_TEST_DEFINITIONS -D_GNU_SOURCE)
+    endif()
+    target_compile_definitions(fmi2_xml_parsing_test PRIVATE ${FMI2_XML_PARSING_TEST_DEFINITIONS})
+endif()
+
 add_executable (fmi2_import_xml_test ${RTTESTDIR}/FMI2/fmi2_import_xml_test.cc)
 target_link_libraries (fmi2_import_xml_test  ${FMILIBFORTEST})
 add_executable (fmi2_import_me_test ${RTTESTDIR}/FMI2/fmi2_import_me_test.c)

Makefile

@@ -25,6 +25,7 @@ generate:
 			-DFMILIB_GENERATE_DOXYGEN_DOC=$(GENERATE_DOXYGEN_DOC) \
 			-DFMILIB_BUILD_WITH_STATIC_RTLIB=$(BUILD_WITH_STATIC_RTLIB) \
 			-DFMILIB_BUILD_TESTS=$(BUILD_TESTS) \
+			-DFMILIB_TEST_LOCALE=$(TEST_LOCALE) \
 			-G $(GENERATOR) \
 			../$(SRC_DIR)
 

RELEASE_NOTES.md

@@ -4,6 +4,12 @@ The release notes are typically a highlighting subset of all changes made. For f
 
 Note that version 2.1 is the first version with release notes. Please see the commit history for older versions.
 
+## 2.2.1
+
+### Minor
+- Bug fix: Correctly parse doubles when locale is not using decimal point
+- Check `variability != continuous` for non-Real variables
+
 ## 2.2
 
 ### Bug fixes

Test/FMI2/fmi2_xml_parsing_test.c

@@ -2,6 +2,7 @@
 #include <fmilib.h>
 #include <stdio.h>
 #include "config_test.h"
+#include <locale.h>
 
 static const int SHOULD_NOT_LOG_EXPECTED_MSG = 0;
 static const int SHOULD_LOG_EXPECTED_MSG = 1;
@@ -10,6 +11,17 @@ static int did_not_log_expected_msg;
 static char *expected_message = "Invalid structured ScalarVariable name";
 static char *name_check_test_directory;
 
+static void fail(const char* fmt, ...) {
+    va_list args;
+    va_start(args, fmt);
+    printf("Test failure: ");
+    vprintf(fmt, args);
+    printf("\n");
+    va_end(args);
+
+    exit(CTEST_RETURN_FAIL);
+}
+
 char *concat(char *s1, char *s2)
 {
     size_t len1 = strlen(s1);
@@ -35,20 +47,19 @@ void importlogger(jm_callbacks* c, jm_string module,
 
 void test_parser(char *xml_dir, int should_not_log_expected_msg, int configuration)
 {
-    jm_callbacks *callbacks;
+    jm_callbacks cb;
     fmi_import_context_t *context;
     fmi2_import_t *fmu;
     char *full_path;
 
-    callbacks = (jm_callbacks *) malloc(sizeof(jm_callbacks));
-    callbacks->malloc = malloc;
-    callbacks->calloc = calloc;
-    callbacks->realloc = realloc;
-    callbacks->free = free;
-    callbacks->logger = importlogger;
-    callbacks->log_level = jm_log_level_all;
-    callbacks->context = 0;
-    context = fmi_import_allocate_context(callbacks);
+    cb.malloc    = malloc;
+    cb.calloc    = calloc;
+    cb.realloc   = realloc;
+    cb.free      = free;
+    cb.logger    = importlogger;
+    cb.log_level = jm_log_level_all;
+    cb.context   = NULL;
+    context = fmi_import_allocate_context(&cb);
     if (configuration != 0) {
         fmi_import_set_configuration(context, configuration);
     }
@@ -62,8 +73,7 @@ void test_parser(char *xml_dir, int should_not_log_expected_msg, int configurati
     if (fmu == NULL) {
         exit(CTEST_RETURN_FAIL);
     }
-    if (!should_not_log_expected_msg && did_not_log_expected_msg ||
-            did_not_log_expected_msg && !should_not_log_expected_msg) {
+    if (should_not_log_expected_msg != did_not_log_expected_msg) { /* XOR */
         exit(CTEST_RETURN_FAIL);
     }
 }
@@ -160,16 +170,146 @@ void test_variable_naming_conventions(void)
     pass_name_check("naming_conventions_xmls/flat/q-char-nonescaped");
 }
 
+static fmi2_import_t* parse_xml(jm_callbacks* cb, const char* xmldir) {
+    fmi_import_context_t* context;
+    fmi2_import_t* xml;
+
+    context = fmi_import_allocate_context(cb);
+
+    xml = fmi2_import_parse_xml(context, xmldir, NULL);
+    fmi_import_free_context(context);
+
+    return xml;
+}
+
+/**
+ * Tests that parsing works as expected, and that the previous locale and
+ * thread settings are reset.
+ *
+ * Test is by default disabled, because it requires the target machine to have
+ * specific language packs.
+ */
+static void test_locale_lc_numeric() {
+
+    jm_callbacks* cb = jm_get_default_callbacks();
+    char* loc_old = NULL;
+    char* tmp = NULL;
+
+    /* Any locale that uses decimal coma instead of decimal point. */
+#ifdef WIN32
+    char* locale_bad = "Swedish_Sweden.1252"; 
+#else
+    char* locale_bad = "sv_SE.utf8";
+#endif
+
+    /* Set/get thread-specific settings (and later check that they are
+     * restored). */
+#ifdef WIN32
+    int thread_setting = _DISABLE_PER_THREAD_LOCALE;
+    _configthreadlocale(thread_setting);
+#else
+    /* Do nothing for Linux since I don't think it's possible to check equality
+     * of locale_t. */
+#endif
+
+    /* NOT MT-SAFE: But it's the only way to test it for Linux. There are
+     * currently no other tests that modify the locale globally, so should be
+     * OK.
+     * Worst case we can run with the '--force-new-ctest-process' ctest flag.
+     */
+    tmp = setlocale(LC_NUMERIC, locale_bad);
+    if (!tmp) {
+        /* If this errors, it's possible that your machine doesn't have
+         * the locale installed.
+         *
+         * Windows: It seemed like I had at least Danish, French, Swedish
+         *   installed by default.
+         *
+         * Linux (Ubuntu 18): I had to install a language pack to get this.
+         */
+        fail("failed to set locale");
+    }
+
+    /*
+     * Value of 'tmp' returned from 'setlocale' may be changed by further calls
+     * to setlocale, and it's also possible that the returned value is not
+     * "string equal" to the argument (i.e. alias values for the same locale).
+     * To be able to later compare the restored value, we therefore must copy
+     * 'tmp'.
+     */
+    loc_old = (char*)malloc(strlen(tmp) + 1);
+    if (!loc_old) {
+        fail("failed to alloc memory");
+    }
+    strcpy(loc_old, tmp);
+    tmp = NULL;
+
+    {
+        /* Perform parsing and verify that the bad global locale did not affect
+         * the result. */
+
+        int failed = 0;
+        char* xmldir = concat(name_check_test_directory, "env/locale");
+        fmi2_import_t* xml = parse_xml(cb, xmldir);
+        free(xmldir);
+
+        if (xml == NULL) {
+            fail("failed to parse FMU");
+        }
+
+        if (fmi2_import_get_default_experiment_start(xml)     != 2.3  ||
+            fmi2_import_get_default_experiment_stop(xml)      != 3.55 ||
+            fmi2_import_get_default_experiment_tolerance(xml) != 1e-6 ||
+            fmi2_import_get_default_experiment_step(xml)      != 2e-3)
+        {
+            /* If the decimal delimiter is comma, sscanf will only parse
+             * until the dot. */
+            printf("Test failure: incorrect default experiment value\n");
+            failed = 1;
+        }
+
+        fmi2_import_free(xml);
+
+        if (failed) {
+            fail("... see above printed messages");
+        }
+     }
+
+    /* Cleanup and verify that locale is properly restored.
+     *
+     * Getting locale should be MT-safe if all setting of locale is done in
+     * per_thread context.
+     */
+    tmp = setlocale(LC_NUMERIC, loc_old);
+    free(loc_old);
+    if (!tmp) {
+        fail("failed to restore locale");
+    } else if (strcmp(tmp, locale_bad)) {
+        fail("unexpected locale");
+    }
+
+#ifdef WIN32
+    if (_configthreadlocale(0) != thread_setting) {
+        /* This was set at the beginning of the test, and should now have been restored. */
+        fail("unexpected Windows thread setting");
+    }
+#endif
+}
+
 int main(int argc, char *argv[])
 {
     if (argc == 2) {
         name_check_test_directory = argv[1];
     } else {
-        printf("Usage: %s <path to folder naming_conventions_xmls>\n", argv[0]);
+        printf("Usage: %s <path to folder 'parser_test_xmls'>\n", argv[0]);
         exit(CTEST_RETURN_FAIL);
     }
 
     test_variable_naming_conventions();
 
+    #ifdef FMILIB_TEST_LOCALE
+    test_locale_lc_numeric();
+    #endif
+
     return 0;
 }

Test/FMI2/parser_test_xmls/env/locale/modelDescription.xml

@@ -0,0 +1,20 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<fmiModelDescription
+  fmiVersion="2.0"
+  modelName="x"
+  guid="x">
+
+<ModelExchange
+  modelIdentifier="x" />
+
+<DefaultExperiment
+  startTime="2.3"
+  stopTime="3.55"
+  tolerance="1e-6"
+  stepSize="2e-3"
+  />
+
+<ModelVariables/>
+<ModelStructure/>
+
+</fmiModelDescription>