EMULATE_SECURE_CRT sprintf_s doesn't match spec

[Philip Taylor]

secure_crt.cpp emulates sprintf_s, vsprintf_s, swprintf_s, vswprintf_s using the snprintf functions. These have the wrong behaviour when the buffer is too short (based on MSDN's documentation), and ought to be made consistent.

[Philip Taylor]

Some test code for what I think is the expected behaviour (except I'm ignoring that it should report parameter errors when the buffer is too short):

===================================================================
--- source/lib/tests/test_secure_crt.h  (revision 7077)
+++ source/lib/tests/test_secure_crt.h  (working copy)
@@ -242,4 +242,24 @@
                TEST_NCAT(d5,5, s5,2, "12",0,"12ab");
                TEST_NCAT(d6,6, s5,10, "",0,"abcde");
        }
+
+       static void TEST_PRINTF(char* dst, size_t max_dst_chars, const char* dst_val,
+               int expected_ret, const char* expected_dst, const char* fmt, ...)
+       {
+               strcpy(dst, dst_val);
+               va_list ap;
+               va_start(ap, fmt);
+               int ret = vsprintf_s(dst, max_dst_chars, fmt, ap);
+               va_end(ap);
+               TS_ASSERT_EQUALS(ret, expected_ret);
+               TS_ASSERT_STR_EQUALS(dst, expected_dst);
+       }
+
+       void test_printf()
+       {
+               TEST_PRINTF(d10,10, s10, 4, "1234", "%d", 1234);
+               TEST_PRINTF(d10,5, s10, 4, "1234", "%d", 1234);
+               TEST_PRINTF(d10,4, s10, 0, "", "%d", 1234);
+               TEST_PRINTF(d10,3, s10, 0, "", "%d", 1234);
+       }
 };

[Jan Wassenberg]

I've added code to wipe out the destination if it's too short and have added the above test (which must not run with VC's implementation because it raises an error dialog). Could you please mark the ticket as fixed if that is now the case? :)

[Philip Taylor]

Why can't we just disable the error dialog in VC? r7097 does that, and seems to work alright, unless I'm missing some problem with that approach. Incidentally, a few of the string tests fail now when they're running on VC, so we should probably fix things to be consistent.

[Jan Wassenberg]

Ooh, good idea! The tests were expecting ERANGE in some cases instead of EINVAL, which is now fixed.

[(none)]

Milestone Open Source Release deleted