[m-rev.] for post-commit review: Do not use _snprintf functions directly in place of snprintf functions.

Peter Wang novalazy at gmail.com
Mon Jul 23 11:05:24 AEST 2018


The Windows _snprintf family of functions do not guarantee null
termination when the output is truncated so cannot be used as direct
replacements for the snprintf functions. Also, the _snprintf functions
have different return values from the C99 snprintf functions when output
is truncated (like some older snprintf implementations).

Furthermore, on Windows snprintf/vsnprintf may be synonyms for
_snprintf/_vsnprintf so cannot be relied upon to terminate their outputs
either, even if the functions exist.

runtime/mercury_string.c:
runtime/mercury_string.h:
    Define MR_snprintf and MR_vsnprintf as macro synonyms for
    snprintf/vsnprintf ONLY if _snprintf/_vsnprintf do not exist.

    Otherwise, implement MR_snprintf and MR_vsnprintf functions
    that behave like the C99 functions, in terms of _vsnprintf.

    Require that either snprintf/vsnprintf or _snprintf/_vsnprintf
    are available. This should be true on all systems still in use.

runtime/mercury_debug.c:
runtime/mercury_ml_expand_body.h:
runtime/mercury_runtime_util.c:
runtime/mercury_stack_layout.c:
runtime/mercury_stack_trace.c:
runtime/mercury_stacks.c:
runtime/mercury_tabling.c:
runtime/mercury_threadscope.c:
runtime/mercury_trace_base.c:
runtime/mercury_wrapper.c:
trace/mercury_trace_completion.c:
trace/mercury_trace_internal.c:
trace/mercury_trace_spy.c:
trace/mercury_trace_vars.c:
bytecode/mb_disasm.c:
    Use MR_snprintf instead of snprintf/_snprintf
    and MR_vsnprintf instead of vsnprintf/_vsnprintf.

    Drop code paths using sprintf as a fallback.
---
 bytecode/mb_disasm.c             | 19 ++++-----
 runtime/mercury_debug.c          |  9 +---
 runtime/mercury_ml_expand_body.h | 18 +-------
 runtime/mercury_runtime_util.c   | 12 +-----
 runtime/mercury_stack_layout.c   | 54 +++++-------------------
 runtime/mercury_stack_trace.c    |  8 +---
 runtime/mercury_stacks.c         |  2 +-
 runtime/mercury_string.c         | 71 +++++++++++++++++++++++---------
 runtime/mercury_string.h         | 26 ++++++++++++
 runtime/mercury_tabling.c        |  3 +-
 runtime/mercury_threadscope.c    |  2 +-
 runtime/mercury_trace_base.c     | 13 +++---
 runtime/mercury_wrapper.c        | 10 ++---
 trace/mercury_trace_completion.c |  3 +-
 trace/mercury_trace_internal.c   |  8 +---
 trace/mercury_trace_spy.c        | 27 +-----------
 trace/mercury_trace_vars.c       | 46 ++++-----------------
 17 files changed, 126 insertions(+), 205 deletions(-)

diff --git a/bytecode/mb_disasm.c b/bytecode/mb_disasm.c
index dcd318d03..6de58a6a4 100644
--- a/bytecode/mb_disasm.c
+++ b/bytecode/mb_disasm.c
@@ -54,28 +54,28 @@ static void str_var_dir(MB_Var_dir var_dir, char *buffer, int buffer_len);
 
 /* Fills a c string buffer with an operation argument */
 static void str_op_arg(MB_Op_arg op_arg, char *buffer, int buffer_len);
 
 /* Implementation */
 
 
 /*
 ** Macros for printing:
 ** Expects buffer & buffer_len to be defined.
-** Wraps calls to snprintf, checks if the buffer is full and
+** Wraps calls to MR_snprintf, checks if the buffer is full and
 ** if it is, returns from the function
 */
 
 #define PRINT()		if (buffer_len > 1) {			\
 				int last_len;			\
 				assert(buffer_len > 0);		\
-				last_len = snprintf(buffer, buffer_len,
+				last_len = MR_snprintf(buffer, buffer_len,
 
 /* printf arguments get sandwiched between these macros */
 #define ENDPRINT()				);		\
 				if (last_len >= buffer_len) {	\
 					last_len = buffer_len-1;\
 				} \
 				buffer += last_len;		\
 				buffer_len -= last_len;		\
 				assert(buffer_len > 0);		\
 			} else {				\
@@ -742,68 +742,63 @@ str_cons_id(MB_Cons_id cons_id, char *buffer, int buffer_len)
 				ENDPRINT()
 			}
 			break;
 		default:
 			MB_fatal("Attempt to disassemble unknown cons");
 			break;
 	} /* end switch */
 	PRINT()
 		"]"
 	ENDPRINT()
-
-	buffer[buffer_len-1] = 0; /* snprintf may not do it if a long string */
 } /* end str_cons_id() */
 
 static void
 str_tag(MB_Tag tag, char *buffer, int buffer_len)
 {
 	
 	switch (tag.id) {
 		case MB_TAG_SIMPLE:
-			snprintf(buffer, buffer_len,
+			MR_snprintf(buffer, buffer_len,
 				"%s %d",
 				"simple_tag",
 				(int) tag.opt.primary);
 			break;
 		case MB_TAG_COMPLICATED:
-			snprintf(buffer, buffer_len,
+			MR_snprintf(buffer, buffer_len,
 				"%s %d %ld",
 				"complicated_tag", 
 				(int) tag.opt.pair.primary, 
 				(long int) tag.opt.pair.secondary);
 			break;
 		case MB_TAG_COMPLICATED_CONSTANT:
-			snprintf(buffer, buffer_len,
+			MR_snprintf(buffer, buffer_len,
 				"%s %d %ld",
 				"complicated_constant_tag", 
 				(int) tag.opt.pair.primary, 
 				(long int) tag.opt.pair.secondary);
 			break;
 		case MB_TAG_ENUM:
-			snprintf(buffer, buffer_len,
+			MR_snprintf(buffer, buffer_len,
 				"%s %d",
 				"enum_tag",
 				(int) tag.opt.enum_tag);
 			break;
 		case MB_TAG_NONE:
-			snprintf(buffer, buffer_len,
+			MR_snprintf(buffer, buffer_len,
 				"%s",
 				"no_tag");
 			break;
 		default:
 			MB_util_error("Invalid tag: %d\n", tag.id);
 			assert(FALSE); /* XXX */
 			break;
 	} /* end switch */
-	
-	/* snprintf may not append a null character */
-	buffer[buffer_len-1] = 0;
 } /* end str_tag() */
 
 /*
 ** XXX ORDER: Currently we depend on the order of elements in the table.
 */
 static const char *
 bytecode_table[] = {
 	"enter_pred",
 	"endof_pred",
 	"enter_proc",
diff --git a/runtime/mercury_debug.c b/runtime/mercury_debug.c
index 900ba578d..4fecaefdf 100644
--- a/runtime/mercury_debug.c
+++ b/runtime/mercury_debug.c
@@ -1307,28 +1307,21 @@ MR_debug_log_message(const char *format, ...)
     // This should be a reasonable estimate of the size of the buffer that we
     // need. At least twice the size of the format string or 128 bytes.
 
     len = strlen(format);
     len = len * 2;
     len = len < 128 ? 128 : len;
 
     buffer = MR_GC_malloc(len);
     while (1) {
         va_start(args, format);
-#ifdef MR_HAVE_VSNPRINTF
-        result = vsnprintf(buffer, len, format, args);
-#elif MR_HAVE__VSNPRINTF
-        result = _vsnprintf(buffer, len, formst, args);
-#else
-        MR_fatal_error(
-            "MR_debug_log_message: Don't have vsnprintf or _vsnprintf\n");
-#endif
+        result = MR_vsnprintf(buffer, len, format, args);
         va_end(args);
         if (result < len) {
             break;
         }
 
         // Make the buffer bigger.
         len = len * 2;
         buffer = MR_GC_realloc(buffer, len);
     }
 
diff --git a/runtime/mercury_ml_expand_body.h b/runtime/mercury_ml_expand_body.h
index fabc9d909..c745ad3a5 100644
--- a/runtime/mercury_ml_expand_body.h
+++ b/runtime/mercury_ml_expand_body.h
@@ -1616,51 +1616,37 @@ EXPAND_FUNCTION_NAME(MR_TypeInfo type_info, MR_Word *data_word_ptr,
             return;
 
         case MR_TYPECTOR_REP_TICKET:
             handle_functor_name("<<ticket>>");
             handle_zero_arity_args();
             return;
 
         case MR_TYPECTOR_REP_FOREIGN:
             {
                 char buf[MR_FOREIGN_NAME_BUF_SIZE];
-#ifdef  MR_HAVE_SNPRINTF
-                snprintf(buf, MR_FOREIGN_NAME_BUF_SIZE,
+                MR_snprintf(buf, MR_FOREIGN_NAME_BUF_SIZE,
                     "<<foreign(%s, %p)>>",
                     type_ctor_info->MR_type_ctor_name,
                     (void *) *data_word_ptr);
-#else
-                sprintf(buf,
-                    "<<foreign(%s, %p)>>",
-                    type_ctor_info->MR_type_ctor_name,
-                    (void *) *data_word_ptr);
-#endif
                 // The contents of the memory occupied by buf may change.
                 copy_and_handle_functor_name(buf);
                 handle_zero_arity_args();
             }
             return;
 
         case MR_TYPECTOR_REP_STABLE_FOREIGN:
             {
                 char buf[MR_FOREIGN_NAME_BUF_SIZE];
-#ifdef  MR_HAVE_SNPRINTF
-                snprintf(buf, MR_FOREIGN_NAME_BUF_SIZE,
+                MR_snprintf(buf, MR_FOREIGN_NAME_BUF_SIZE,
                     "<<stable_foreign(%s, %p)>>",
                     type_ctor_info->MR_type_ctor_name,
                     (void *) *data_word_ptr);
-#else
-                sprintf(buf,
-                    "<<stable_foreign(%s, %p)>>",
-                    type_ctor_info->MR_type_ctor_name,
-                    (void *) *data_word_ptr);
-#endif
                 // The contents of the memory occupied by buf may change.
                 copy_and_handle_functor_name(buf);
                 handle_zero_arity_args();
             }
             return;
 
         case MR_TYPECTOR_REP_REFERENCE:
             if (noncanon == MR_NONCANON_ABORT) {
                 // XXX Should throw an exception.
                 MR_fatal_error(MR_STRINGIFY(EXPAND_FUNCTION_NAME)
diff --git a/runtime/mercury_runtime_util.c b/runtime/mercury_runtime_util.c
index 632619678..2b532cf9b 100644
--- a/runtime/mercury_runtime_util.c
+++ b/runtime/mercury_runtime_util.c
@@ -16,31 +16,21 @@
 
 #ifdef MR_HAVE_UNISTD_H
   #include  <unistd.h>
 #endif
 
 #include    <errno.h>
 
 static void
 generic_strerror(char *buf, size_t buflen, int errnum)
 {
-#if defined(MR_HAVE_SNPRINTF)
-    snprintf(buf, buflen, "Error %d", errnum);
-#elif defined(MR_HAVE__SNPRINTF)
-    // _snprintf does not guarantee null termination.
-    _snprintf(buf, buflen, "Error %d", errnum);
-    if (buflen > 0) {
-        buf[buflen - 1] = '\0';
-    }
-#else
-    #error "generic_error: unable to define"
-#endif
+    MR_snprintf(buf, buflen, "Error %d", errnum);
 }
 
 const char *
 MR_strerror(int errnum, char *buf, size_t buflen)
 {
 #if defined(MR_HAVE_STRERROR_S) && !defined(MR_MINGW)
     // MSVC has strerror_s. It also exists in C11 Annex K and is enabled by
     // defining a preprocessor macro __STDC_WANT_LIB_EXT1__
     //
     // On MinGW-w64, strerror_s results in an undefined reference to strerror_s
diff --git a/runtime/mercury_stack_layout.c b/runtime/mercury_stack_layout.c
index db84b1010..d7cf7004b 100644
--- a/runtime/mercury_stack_layout.c
+++ b/runtime/mercury_stack_layout.c
@@ -57,80 +57,48 @@ MR_name_in_string_table(const char *string_table, MR_Integer string_table_size,
 
         name_code >>= 1;
         kind = name_code & 0x1f;
         name_code >>= 5;
         n = name_code & 0x3ff;
         offset = name_code >> 10;
 
         switch (kind) {
             case 0:
                 if (n == 0) {
-#ifdef  MR_HAVE_SNPRINTF
-                    snprintf(buf, MR_MAX_VARNAME_SIZE, "STATE_VARIABLE_%s",
-                        string_table + offset);
-#else
-                    sprintf(buf, "STATE_VARIABLE_%s",
-                        string_table + offset);
-#endif
+                    MR_snprintf(buf, MR_MAX_VARNAME_SIZE,
+                        "STATE_VARIABLE_%s", string_table + offset);
                 } else {
-#ifdef  MR_HAVE_SNPRINTF
-                    snprintf(buf, MR_MAX_VARNAME_SIZE, "STATE_VARIABLE_%s_%d",
-                        string_table + offset, n - 1);
-#else
-                    sprintf(buf, "STATE_VARIABLE_%s_%d",
-                        string_table + offset, n - 1);
-#endif
+                    MR_snprintf(buf, MR_MAX_VARNAME_SIZE,
+                        "STATE_VARIABLE_%s_%d", string_table + offset, n - 1);
                 }
                 break;
 
             case 1:
-#ifdef  MR_HAVE_SNPRINTF
-                snprintf(buf, MR_MAX_VARNAME_SIZE, "TypeCtorInfo_%d", n);
-#else
-                sprintf(buf, "TypeCtorInfo_%d", n);
-#endif
+                MR_snprintf(buf, MR_MAX_VARNAME_SIZE, "TypeCtorInfo_%d", n);
                 break;
 
             case 2:
-#ifdef  MR_HAVE_SNPRINTF
-                snprintf(buf, MR_MAX_VARNAME_SIZE, "TypeInfo_%d", n);
-#else
-                sprintf(buf, "TypeInfo_%d", n);
-#endif
+                MR_snprintf(buf, MR_MAX_VARNAME_SIZE, "TypeInfo_%d", n);
                 break;
 
             case 3:
-#ifdef  MR_HAVE_SNPRINTF
-                snprintf(buf, MR_MAX_VARNAME_SIZE, "BaseTypeClassInfo_for_%s",
-                    string_table + offset);
-#else
-                sprintf(buf, "BaseTypeClassInfo_for_%s",
-                    string_table + offset);
-#endif
+                MR_snprintf(buf, MR_MAX_VARNAME_SIZE,
+                    "BaseTypeClassInfo_for_%s", string_table + offset);
                 break;
 
             case 4:
-#ifdef  MR_HAVE_SNPRINTF
-                snprintf(buf, MR_MAX_VARNAME_SIZE, "TypeClassInfo_for_%s",
-                    string_table + offset);
-#else
-                sprintf(buf, "TypeClassInfo_for_%s",
-                    string_table + offset);
-#endif
+                MR_snprintf(buf, MR_MAX_VARNAME_SIZE,
+                    "TypeClassInfo_for_%s", string_table + offset);
                 break;
 
             case 5:
-#ifdef  MR_HAVE_SNPRINTF
-                snprintf(buf, MR_MAX_VARNAME_SIZE, "PolyConst%d", n);
-#else
-                sprintf(buf, "PolyConst%d", n);
-#endif
+                MR_snprintf(buf, MR_MAX_VARNAME_SIZE, "PolyConst%d", n);
                 break;
 
             default:
                 MR_fatal_error("MR_hlds_var_name: unknown kind");
                 break;
         }
 
         if (should_copy != NULL) {
             *should_copy = MR_TRUE;
         }
diff --git a/runtime/mercury_stack_trace.c b/runtime/mercury_stack_trace.c
index b77ee6d10..4ff26c25b 100644
--- a/runtime/mercury_stack_trace.c
+++ b/runtime/mercury_stack_trace.c
@@ -10,24 +10,20 @@
 
 #include "mercury_imp.h"
 #include "mercury_stack_trace.h"
 #include "mercury_stack_layout.h"
 #include "mercury_debug.h"
 #include "mercury_array_macros.h"
 #include "mercury_trace_base.h"
 #include "mercury_tabling.h"
 #include <stdio.h>
 
-#if defined(MR_HAVE__SNPRINTF) && ! defined(MR_HAVE_SNPRINTF)
-  #define snprintf  _snprintf
-#endif
-
 static int          MR_compare_proc_layout_ptrs(
                         const void *pl1, const void *pl2);
 
 static  MR_StackWalkStepResult
                     MR_stack_walk_succip_layout(MR_Code *success,
                         const MR_LabelLayout **return_label_layout_ptr,
                         MR_Word **base_sp_ptr, MR_Word **base_curfr_ptr,
                         const char **problem_ptr);
 
 #ifndef MR_HIGHLEVEL_CODE
@@ -1958,23 +1954,23 @@ MR_print_call_trace_info(FILE *fp, const MR_ProcLayout *proc_layout,
         // of the function MR_trace_event_print_internal_report in
         // trace/mercury_trace_internal.c. Any changes here will probably
         // require similar changes there.
 
         if (MR_standardize_event_details) {
             char    buf[64];    // Plenty big enough.
 
             // Do not print the context id, since it is not standardized.
             event_num = MR_standardize_event_num(event_num);
             call_num = MR_standardize_call_num(call_num);
-            snprintf(buf, 64, "E%lu", event_num);
+            MR_snprintf(buf, 64, "E%lu", event_num);
             fprintf(fp, "%7s ", buf);
-            snprintf(buf, 64, "C%lu", call_num);
+            MR_snprintf(buf, 64, "C%lu", call_num);
             fprintf(fp, "%7s ", buf);
             fprintf(fp, "%4lu ", depth);
         } else {
             // Do not print the context id, since it is the same for
             // all the calls in the stack.
 
             fprintf(fp, "%7lu %7lu %4lu ", event_num, call_num, depth);
         }
     } else {
         // Ensure that the remaining columns line up.
diff --git a/runtime/mercury_stacks.c b/runtime/mercury_stacks.c
index 57a0d2543..70eafbde9 100644
--- a/runtime/mercury_stacks.c
+++ b/runtime/mercury_stacks.c
@@ -1088,21 +1088,21 @@ MR_pneg_enter_else(const char *context)
             char        *buf;
 
             msg = "failing out of negated context with incomplete consumer";
             if (context != NULL) {
                 // The 10 accounts for the ": ", the final '\0',
                 // and leaves some space to spare.
 
                 len = strlen(context) + strlen(msg) + 10;
                 buf = malloc(len);
                 if (buf != NULL) {
-                    snprintf(buf, len, "%s: %s", context, msg);
+                    MR_snprintf(buf, len, "%s: %s", context, msg);
                     MR_fatal_error(buf);
                 } else {
                     MR_fatal_error(msg);
                 }
             } else {
                 MR_fatal_error(msg);
             }
         }
 
         MR_table_free(l);
diff --git a/runtime/mercury_string.c b/runtime/mercury_string.c
index 8161ffb7e..8a54a9c28 100644
--- a/runtime/mercury_string.c
+++ b/runtime/mercury_string.c
@@ -2,54 +2,99 @@
 
 // Copyright (C) 2000-2002, 2006, 2011-2012 The University of Melbourne.
 // Copyright (C) 2015-2016, 2018 The Mercury team.
 // This file is distributed under the terms specified in COPYING.LIB.
 
 // mercury_string.c - string handling
 
 #include "mercury_imp.h"
 #include "mercury_string.h"
 
-#if defined(MR_HAVE__VSNPRINTF) && ! defined(MR_HAVE_VSNPRINTF)
-  #define vsnprintf _vsnprintf
+#ifdef _MSC_VER
+    // Disable warnings about using _vsnprintf being deprecated.
+    #pragma warning(disable:4996)
+
+    // va_copy is available from VC 2013 onwards.
+    #if _MSC_VER < 1800
+        #define va_copy(a, b)   ((a) = (b))
+    #endif
 #endif
 
-#if defined(MR_HAVE_VSNPRINTF) || defined(MR_HAVE__VSNPRINTF)
-  #define MR_HAVE_A_VSNPRINTF
+#if defined(MR_HAVE__VSNPRINTF)
+int
+MR_vsnprintf(char *str, size_t size, const char *format, va_list ap)
+{
+    va_list     ap_copy;
+    int         n;
+
+    if (size == 0) {
+        return _vsnprintf(NULL, 0, format, ap);
+    }
+
+    // _vsnprintf does not append a null terminator if the output is truncated.
+    // Follow the MS advice of initialising the buffer to null before calling
+    // _vsnprintf with a count strictly less than the buffer length.
+    memset(str, 0, size);
+    va_copy(ap_copy, ap);
+    n = _vsnprintf(str, size - 1, format, ap_copy);
+    va_end(ap_copy);
+
+    if (n == -1) {
+        // Return the number of characters that would have been written
+        // without truncation, to match the behaviour of C99 vsnprintf.
+        n = _vsnprintf(NULL, 0, format, ap);
+    }
+
+    return n;
+}
+#endif
+
+#if defined(MR_HAVE__SNPRINTF)
+int
+MR_snprintf(char *str, size_t size, const char *format, ...)
+{
+    va_list     ap;
+    int         n;
+
+    va_start(ap, format);
+    n = MR_vsnprintf(str, size, format, ap);
+    va_end(ap);
+
+    return n;
+}
 #endif
 
 #define BUFFER_SIZE 4096
 
 MR_String
 MR_make_string(MR_AllocSiteInfoPtr alloc_id, const char *fmt, ...)
 {
     va_list     ap;
     MR_String   result;
     int         n;
     char        *p;
 
-#ifdef MR_HAVE_A_VSNPRINTF
     int         size = BUFFER_SIZE;
     char        fixed[BUFFER_SIZE];
     MR_bool     dynamically_allocated = MR_FALSE;
 
     // On the first iteration we try with a fixed-size buffer.
     // If that didn't work, use a dynamically allocated array twice
     // the size of the fixed array and keep growing the array until
     // the string fits.
 
     p = fixed;
 
     while (1) {
         // Try to print in the allocated space.
         va_start(ap, fmt);
-        n = vsnprintf(p, size, fmt, ap);
+        n = MR_vsnprintf(p, size, fmt, ap);
         va_end(ap);
 
         // If that worked, return the string.
         if (n > -1 && n < size) {
             break;
         }
 
         // Else try again with more space.
         if (n > -1) {   // glibc 2.1
             size = n + 1; // precisely what is needed
@@ -58,42 +103,28 @@ MR_make_string(MR_AllocSiteInfoPtr alloc_id, const char *fmt, ...)
         }
 
         if (!dynamically_allocated) {
             p = MR_NEW_ARRAY(char, size);
             dynamically_allocated = MR_TRUE;
         } else {
             p = MR_RESIZE_ARRAY(p, char, size);
         }
     }
 
-#else
-    // It is possible for this buffer to overflow,
-    // and then bad things may happen.
-
-    char fixed[40960];
-
-    va_start(ap, fmt);
-    n = vsprintf(fixed, fmt, ap);
-    va_end(ap);
-
-    p = fixed;
-#endif
     MR_restore_transient_hp();
     MR_allocate_aligned_string_msg(result, strlen(p), alloc_id);
     MR_save_transient_hp();
     strcpy(result, p);
 
-#ifdef MR_HAVE_A_VSNPRINTF
     if (dynamically_allocated) {
         MR_free(p);
     }
-#endif
 
     return result;
 }
 
 // The code for this function should be kept in sync with that of the
 // quote_string predicates in library/term_io.m.
 MR_bool
 MR_escape_string_quote(MR_String *ptr, const char * string)
 {
     MR_Integer pos = 0;
diff --git a/runtime/mercury_string.h b/runtime/mercury_string.h
index e60b6846e..4b173333a 100644
--- a/runtime/mercury_string.h
+++ b/runtime/mercury_string.h
@@ -6,20 +6,46 @@
 
 // mercury_string.h - string handling
 
 #ifndef MERCURY_STRING_H
 #define MERCURY_STRING_H
 
 #include "mercury_heap.h"       // for MR_offset_incr_hp_atomic
 
 #include <string.h>             // for strcmp() etc.
 #include <stdarg.h>
+#include <stdio.h>
+
+// On Windows, snprintf/vsnprintf may be synonyms for _snprintf/_vsnprintf and
+// thus not conform to the C99 specification. Since it is next to impossible to
+// tell at compile time which implementation we are getting, just define a
+// wrapper over _vsnprintf if it exists.
+// Beginning with the UCRT in Visual Studio 2015 and Windows 10, snprintf and
+// vsnprintf are C99 standard compliant so we may be able to drop this code
+// eventually.
+
+#if defined(MR_HAVE__VSNPRINTF)
+    extern int
+    MR_vsnprintf(char *str, size_t size, const char *format, va_list ap);
+#elif defined(MR_HAVE_VSNPRINTF)
+    #define MR_vsnprintf vsnprintf
+#else
+    #error "Missing both vsnprintf and _vsnprintf"
+#endif
+
+#if defined(MR_HAVE__SNPRINTF)
+    extern int MR_snprintf(char *str, size_t size, const char *format, ...);
+#elif defined(MR_HAVE_SNPRINTF)
+    #define MR_snprintf snprintf
+#else
+    #error "Missing both snprintf and _snprintf"
+#endif
 
 // Mercury characters (Unicode code points) are given type `MR_Char',
 // which is a typedef for `MR_int_least32_t'.
 // Mercury strings are stored as pointers to '\0'-terminated arrays of `char'.
 // Strings are UTF-8 encoded.
 // Mercury strings must not contain null characters. Unexpected null characters
 // are a source of security vulnerabilities.
 //
 // The actual typedefs are in mercury_types.h to avoid problems with
 // circular #includes.
diff --git a/runtime/mercury_tabling.c b/runtime/mercury_tabling.c
index f5ee699bb..97b147ca0 100644
--- a/runtime/mercury_tabling.c
+++ b/runtime/mercury_tabling.c
@@ -1662,16 +1662,17 @@ void mercury_sys_init_table_modules_write_out_proc_statics(FILE *fp)
 #endif
 
 ////////////////////////////////////////////////////////////////////////////
 
 #ifdef MR_TABLE_DEBUG
 static void
 MR_table_assert_failed(const char *file, unsigned line)
 {
     char    buf[256];
 
-    snprintf(buf, 256, "assertion failed: file %s, line %d", file, line);
+    MR_snprintf(buf, sizeof(buf), "assertion failed: file %s, line %d",
+        file, line);
     MR_fatal_error(buf);
 }
 #endif
 
 ////////////////////////////////////////////////////////////////////////////
diff --git a/runtime/mercury_threadscope.c b/runtime/mercury_threadscope.c
index aac97c246..021b2a242 100644
--- a/runtime/mercury_threadscope.c
+++ b/runtime/mercury_threadscope.c
@@ -1868,21 +1868,21 @@ MR_open_output_file_and_write_prelude(void)
     MR_Unsigned     i;
 
     progname_copy = strdup(MR_progname);
     progname_base = basename(progname_copy);
 
     // This is an over-approximation on the amount of space needed
     // for this filename.
 
     filename_len = strlen(progname_base) + strlen(MR_TS_FILENAME_FORMAT) + 1;
     MR_threadscope_output_filename = MR_GC_NEW_ARRAY(char, filename_len);
-    snprintf(MR_threadscope_output_filename, filename_len,
+    MR_snprintf(MR_threadscope_output_filename, filename_len,
         MR_TS_FILENAME_FORMAT, progname_base);
     free(progname_copy);
     progname_copy = NULL;
     progname_base = NULL;
 
     MR_threadscope_output_file = fopen(MR_threadscope_output_filename, "w");
     if (!MR_threadscope_output_file) {
         MR_perror(MR_threadscope_output_filename);
         return;
     }
diff --git a/runtime/mercury_trace_base.c b/runtime/mercury_trace_base.c
index cbedf30e5..ef8b110dd 100644
--- a/runtime/mercury_trace_base.c
+++ b/runtime/mercury_trace_base.c
@@ -32,24 +32,20 @@ ENDINIT
 #include <errno.h>
 
 #ifdef MR_HAVE_UNISTD_H
   #include <unistd.h>       // for the write system call
 #endif
 
 #ifdef MR_HAVE_SYS_WAIT_H
   #include <sys/wait.h>     // for the wait system call
 #endif
 
-#if defined(MR_HAVE__SNPRINTF) && ! defined(MR_HAVE_SNPRINTF)
-  #define snprintf  _snprintf
-#endif
-
 #define MR_TRACE_COUNT_SUMMARY_MAX_DEFAULT  20
 
 void                (*MR_trace_shutdown)(void) = NULL;
 
 MR_bool             MR_trace_count_enabled = MR_FALSE;
 MR_bool             MR_coverage_test_enabled = MR_FALSE;
 const char          *MR_trace_count_summary_file = NULL;
 const char          *MR_trace_count_summary_cmd = "mtc_union";
 unsigned int        MR_trace_count_summary_max =
                         MR_TRACE_COUNT_SUMMARY_MAX_DEFAULT;
@@ -290,21 +286,21 @@ MR_trace_record_label_exec_counts(void *dummy)
         if (MR_FILE_EXISTS(MR_trace_count_summary_file)) {
             int     i;
 
             // 30 bytes must be enough for the dot, the value of i, and '\0'.
             name_len = strlen(MR_trace_count_summary_file) + 30;
             name = MR_malloc(name_len);
 
             fp = NULL;
             // Search for a suffix that doesn't exist yet.
             for (i = 1; i <= MR_trace_count_summary_max; i++) {
-                snprintf(name, name_len, "%s.%d",
+                MR_snprintf(name, name_len, "%s.%d",
                     MR_trace_count_summary_file, i);
                 if (! MR_FILE_EXISTS(name)) {
                     // File doesn't exist, commit to this one.
                     if (i == MR_trace_count_summary_max) {
                         summarize = MR_TRUE;
                     }
 
                     break;
                 }
             }
@@ -318,21 +314,21 @@ MR_trace_record_label_exec_counts(void *dummy)
     } else {
         char    *s;
 
         // If no trace counts file name is provided, then we generate
         // a file name.
 
         // 100 bytes must be enough for the process id, dots and '\0'.
         name_len = strlen(MERCURY_TRACE_COUNTS_PREFIX) + strlen(program_name)
             + 100;
         name = MR_malloc(name_len);
-        snprintf(name, name_len, ".%s.%s.%d", MERCURY_TRACE_COUNTS_PREFIX,
+        MR_snprintf(name, name_len, ".%s.%s.%d", MERCURY_TRACE_COUNTS_PREFIX,
             program_name, getpid());
 
         // Make sure name is an acceptable filename.
         for (s = name; *s != '\0'; s++) {
             if (*s == '/') {
                 *s = '_';
             }
         }
     }
 
@@ -382,21 +378,22 @@ MR_trace_record_label_exec_counts(void *dummy)
         cmd = MR_malloc(cmd_len);
 
         strcpy(cmd, MR_trace_count_summary_cmd);
         strcat(cmd, " -o ");
         strcat(cmd, MR_trace_count_summary_file);
         strcat(cmd, TEMP_SUFFIX);
         strcat(cmd, " ");
         strcat(cmd, MR_trace_count_summary_file);
 
         for (i = 1; i <= MR_trace_count_summary_max; i++) {
-            snprintf(name, name_len, "%s.%d", MR_trace_count_summary_file, i);
+            MR_snprintf(name, name_len, "%s.%d",
+                MR_trace_count_summary_file, i);
             strcat(cmd, " ");
             strcat(cmd, name);
         }
 
         strcat(cmd, " > /dev/null 2>&1");
 
         old_options = getenv("MERCURY_OPTIONS");
         if (old_options != NULL) {
             (void) MR_setenv("MERCURY_OPTIONS", "", MR_TRUE);
             summary_status = system(cmd);
@@ -409,21 +406,21 @@ MR_trace_record_label_exec_counts(void *dummy)
             strcpy(cmd, "mv ");
             strcat(cmd, MR_trace_count_summary_file);
             strcat(cmd, TEMP_SUFFIX);
             strcat(cmd, " ");
             strcat(cmd, MR_trace_count_summary_file);
             mv_status = system(cmd);
 
             if (mv_status == 0) {
                 // Delete all files whose data is now in the summary file.
                 for (i = 1; i <= MR_trace_count_summary_max; i++) {
-                    snprintf(name, name_len, "%s.%d",
+                    MR_snprintf(name, name_len, "%s.%d",
                         MR_trace_count_summary_file, i);
                     unlink_status = unlink(name);
                     if (unlink_status != 0) {
                         MR_fatal_error(
                             "couldn't create summary of trace data");
                     }
                 }
             } else {
                 MR_fatal_error("couldn't create summary of trace data");
             }
diff --git a/runtime/mercury_wrapper.c b/runtime/mercury_wrapper.c
index cac03f1e9..23505d0da 100644
--- a/runtime/mercury_wrapper.c
+++ b/runtime/mercury_wrapper.c
@@ -51,24 +51,20 @@ ENDINIT
 #include    "mercury_init.h"
 #include    "mercury_dummy.h"
 #include    "mercury_stack_layout.h"
 #include    "mercury_trace_base.h"
 #include    "mercury_deep_profiling.h"
 #include    "mercury_memory.h"          // for MR_copy_string()
 #include    "mercury_memory_handlers.h" // for MR_default_handler
 #include    "mercury_thread.h"          // for MR_debug_threads
 #include    "mercury_threadscope.h"
 
-#if defined(MR_HAVE__SNPRINTF) && ! defined(MR_HAVE_SNPRINTF)
-  #define snprintf  _snprintf
-#endif
-
 // global variables concerned with testing (i.e. not with the engine)
 
 // command-line options
 
 // Sizes of data areas (including redzones), in kilobytes
 // (but we later multiply by 1024 to convert to bytes, then make sure they are
 // at least as big as the primary cache size then round up to the page size).
 //
 // XXX We should associate a *single* unit with each of these sizes, and
 // use that unit *consistently*; anything else is just asking for trouble.
@@ -1153,36 +1149,36 @@ MR_process_environment_options(void)
             &option_argv, &option_argc);
         if (error_msg != NULL) {
             char    *where_buf;
             int     where_buf_cur;
 
             where_buf = MR_GC_NEW_ARRAY(char, WHERE_BUF_SIZE);
             where_buf[0] = '\0';
             where_buf_cur = 0;
 
             if (gen_env_options[0] != '\0') {
-                snprintf(where_buf, WHERE_BUF_SIZE,
+                MR_snprintf(where_buf, WHERE_BUF_SIZE,
                     "the %s environment variable", MERCURY_OPTIONS);
             }
 
             if (prog_env_options[0] != '\0') {
                 where_buf_cur = strlen(where_buf);
-                snprintf(where_buf + where_buf_cur,
+                MR_snprintf(where_buf + where_buf_cur,
                     WHERE_BUF_SIZE - where_buf_cur,
                     "%sthe %s environment variable",
                     where_buf_cur == 0 ? "" : " and/or ",
                     prog_env_option_name);
             }
 
             if (MR_runtime_flags[0] != '\0') {
                 where_buf_cur = strlen(where_buf);
-                snprintf(where_buf + where_buf_cur,
+                MR_snprintf(where_buf + where_buf_cur,
                     WHERE_BUF_SIZE - where_buf_cur,
                     "%sthe runtime options built into the executable",
                     where_buf_cur == 0 ? "" : " and/or ");
             }
 
             MR_fatal_error("error parsing %s:\n%s\n", where_buf, error_msg);
         }
 
         MR_GC_free(dummy_command_line);
         MR_process_options(option_argc, option_argv);
diff --git a/trace/mercury_trace_completion.c b/trace/mercury_trace_completion.c
index 61a30056b..6c48b9073 100644
--- a/trace/mercury_trace_completion.c
+++ b/trace/mercury_trace_completion.c
@@ -5,20 +5,21 @@
 // This file is distributed under the terms specified in COPYING.LIB.
 
 // mercury_trace_completion.c
 //
 // Main author: stayl
 //
 // Command line completion for mdb.
 
 #include "mercury_memory.h"
 #include "mercury_std.h"
+#include "mercury_string.h"
 #include "mercury_array_macros.h"
 #include "mercury_trace_completion.h"
 #include "mercury_trace_internal.h"
 #include "mercury_trace_alias.h"
 #include "mercury_trace_tables.h"
 
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 
@@ -489,21 +490,21 @@ MR_insert_module_into_source_file_line_table(const MR_ModuleLayout *module)
             // to the array if we find it.
 
             if (cur_line > 0 &&
                 line_num == file->MR_mfl_label_lineno[cur_line - 1])
             {
                 // The string we would add would be the same as the string
                 // for the previous line number.
                 continue;
             }
 
-            snprintf(&MR_source_file_line_chars[file_name_len + 1],
+            MR_snprintf(&MR_source_file_line_chars[file_name_len + 1],
                 LINE_NUM_MAX_CHARS, "%d", line_num);
             MR_source_file_lines[MR_source_file_line_next] =
                 strdup(MR_source_file_line_chars);
             MR_source_file_line_next++;
         }
     }
 }
 
 static int
 MR_compare_source_file_lines(const void *ptr1, const void *ptr2)
diff --git a/trace/mercury_trace_internal.c b/trace/mercury_trace_internal.c
index a96e7f3cd..84f420fd9 100644
--- a/trace/mercury_trace_internal.c
+++ b/trace/mercury_trace_internal.c
@@ -108,24 +108,20 @@
 // The initial size of arrays of words.
 #define MR_INIT_WORD_COUNT  20
 
 // An upper bound on the maximum number of characters in a number.
 // If a number has more than this many chars, the user is in trouble.
 #define MR_NUMBER_LEN       80
 
 #define MDBRC_FILENAME          ".mdbrc"
 #define DEFAULT_MDBRC_FILENAME  "mdbrc"
 
-#if defined(MR_HAVE__SNPRINTF) && ! defined(MR_HAVE_SNPRINTF)
-  #define snprintf  _snprintf
-#endif
-
 // XXX We should consider whether all the static variables in this module
 // should be thread local.
 
 // Debugger I/O streams.
 // Replacements for stdin/stdout/stderr respectively.
 //
 // The distinction between MR_mdb_out and MR_mdb_err is analogous to
 // the distinction between stdout and stderr: ordinary output, including
 // information messages about conditions which are not errors, should
 // go to MR_mdb_out, but error messages should go to MR_mdb_err.
@@ -1374,23 +1370,23 @@ MR_trace_event_print_internal_report(MR_EventInfo *event_info)
         char        buf[64];
         MR_Unsigned event_num;
         MR_Unsigned call_num;
 
         // Do not print the context id. It contains the values of the arguments
         // cast to integers. Since those arguments may originally have been
         // addresses, their values may differ from run to run.
 
         event_num = MR_standardize_event_num(event_info->MR_event_number);
         call_num = MR_standardize_call_num(event_info->MR_call_seqno);
-        snprintf(buf, 64, "E%ld", (long) event_num);
+        MR_snprintf(buf, 64, "E%ld", (long) event_num);
         fprintf(MR_mdb_out, "%8s: ", buf);
-        snprintf(buf, 64, "C%ld", (long) call_num);
+        MR_snprintf(buf, 64, "C%ld", (long) call_num);
         fprintf(MR_mdb_out, "%6s ", buf);
         fprintf(MR_mdb_out, "%s",
             MR_simplified_port_names[event_info->MR_trace_port]);
     } else {
 #ifdef  MR_USE_MINIMAL_MODEL_OWN_STACKS
         MR_Generator    *generator;
         int             i;
 
         generator = MR_ENGINE(MR_eng_this_context)->MR_ctxt_owner_generator;
         if (generator != NULL) {
diff --git a/trace/mercury_trace_spy.c b/trace/mercury_trace_spy.c
index 8dd552402..3918c6ef0 100644
--- a/trace/mercury_trace_spy.c
+++ b/trace/mercury_trace_spy.c
@@ -17,28 +17,20 @@
 #include "mercury_trace.h"
 #include "mercury_trace_spy.h"
 #include "mercury_trace_tables.h"
 #include "mercury_trace_util.h"
 #include "mercury_trace_vars.h"
 
 #include "mdb.cterm.mh"
 
 #include <stdlib.h>
 
-#if defined(MR_HAVE__SNPRINTF) && ! defined(MR_HAVE_SNPRINTF)
-  #define snprintf  _snprintf
-#endif
-
-#if defined(MR_HAVE_SNPRINTF) || defined(MR_HAVE__SNPRINTF)
-  #define MR_HAVE_A_SNPRINTF
-#endif
-
 const char      *MR_spy_when_names[] =
 {
     "all",
     "interface",
     "entry",
     "specific",
     "linenumber",
     "user_event",
     "user_event_set",
 };
@@ -728,43 +720,28 @@ MR_add_line_spy_point(MR_SpyAction action, MR_SpyIgnore_When ignore_when,
         &num_file_matches, &num_line_matches);
     new_size = MR_spied_label_next;
 
     if (new_size == old_size) {
         if (num_line_matches != 0) {
             // Every line match should add a new spy point.
             MR_fatal_error("MR_add_line_spy_point: num_line_matches != 0");
         }
 
         // There were no matching labels.
-#ifdef  MR_HAVE_A_SNPRINTF
         if (num_file_matches == 0) {
-            snprintf(MR_error_msg_buf, MR_ERROR_MSG_BUF_SIZE,
+            MR_snprintf(MR_error_msg_buf, MR_ERROR_MSG_BUF_SIZE,
                 "there is no debuggable source file named %s", filename);
         } else {
-            snprintf(MR_error_msg_buf, MR_ERROR_MSG_BUF_SIZE,
+            MR_snprintf(MR_error_msg_buf, MR_ERROR_MSG_BUF_SIZE,
                 "there is no event at line %d in %s",
                 linenumber, filename);
         }
-#else
-        // Not absolutely safe, but the risk of overflow is minimal.
-        if (num_file_matches == 0) {
-            sprintf(MR_error_msg_buf,
-                "there is no debuggable source file named %s", filename);
-        } else {
-            sprintf(MR_error_msg_buf,
-                "there is no event at line %d in file %s",
-                linenumber, filename);
-        }
-        if (strlen(MR_error_msg_buf) >= MR_ERROR_MSG_BUF_SIZE) {
-            MR_fatal_error("MR_add_line_spy_point: buf overflow");
-        }
-#endif
         *problem = MR_error_msg_buf;
         return -1;
     }
 
     if (num_line_matches == 0) {
         MR_fatal_error("MR_add_line_spy_point: num_line_matches == 0");
     }
 
     // The matching labels were added at the end of the spied label table.
     // We must make the table sorted again.
diff --git a/trace/mercury_trace_vars.c b/trace/mercury_trace_vars.c
index d8af37209..44e5fad93 100644
--- a/trace/mercury_trace_vars.c
+++ b/trace/mercury_trace_vars.c
@@ -2153,89 +2153,57 @@ MR_trace_printed_var_name(const MR_ProcLayout *proc,
 {
     const MR_ProgVarDetails     *var;
     const MR_AttributeDetails   *attr;
     MR_ConstString              attr_var_name;
 
     switch (value->MR_value_kind) {
         case MR_VALUE_ATTRIBUTE:
             attr = &value->MR_value_attr;
             attr_var_name = MR_hlds_var_name(proc,
                 attr->MR_attr_var_hlds_number, NULL);
-#ifdef  MR_HAVE_SNPRINTF
             if (attr_var_name != NULL && strcmp(attr_var_name, "") != 0) {
-                snprintf(MR_var_name_buf, MR_TRACE_VAR_NAME_BUF_SIZE,
+                MR_snprintf(MR_var_name_buf, MR_TRACE_VAR_NAME_BUF_SIZE,
                     "%s (attr %d, %s)", attr->MR_attr_name,
                     attr->MR_attr_num, attr_var_name);
             } else {
-                snprintf(MR_var_name_buf, MR_TRACE_VAR_NAME_BUF_SIZE,
+                MR_snprintf(MR_var_name_buf, MR_TRACE_VAR_NAME_BUF_SIZE,
                     "%s (attr %d)", attr->MR_attr_name,
                     attr->MR_attr_num);
             }
-#else
-            if (attr_var_name != NULL && strcmp(attr_var_name, "") != 0) {
-                sprintf(MR_var_name_buf,
-                    "%s (attr %d, %s)", attr->MR_attr_name,
-                    attr->MR_attr_num, attr_var_name);
-            } else {
-                sprintf(MR_var_name_buf,
-                    "%s (attr %d)", attr->MR_attr_name,
-                    attr->MR_attr_num);
-            }
-#endif
             break;
 
         case MR_VALUE_PROG_VAR:
             var = &value->MR_value_var;
 
             // If the variable name starts with "HeadVar__", then the
             // argument number is part of the name.
 
             if (var->MR_var_is_headvar &&
                 ! MR_streq(var->MR_var_basename, "HeadVar__"))
             {
                 if (var->MR_var_is_ambiguous) {
-#ifdef  MR_HAVE_SNPRINTF
-                    snprintf(MR_var_name_buf, MR_TRACE_VAR_NAME_BUF_SIZE,
+                    MR_snprintf(MR_var_name_buf, MR_TRACE_VAR_NAME_BUF_SIZE,
                         "%s(%d) (arg %d)", var->MR_var_fullname,
                         var->MR_var_hlds_number, var->MR_var_is_headvar);
-#else
-                    sprintf(MR_var_name_buf, "%s(%d) (arg %d)",
-                        var->MR_var_fullname,
-                        var->MR_var_hlds_number, var->MR_var_is_headvar);
-#endif
                 } else {
-#ifdef  MR_HAVE_SNPRINTF
-                    snprintf(MR_var_name_buf, MR_TRACE_VAR_NAME_BUF_SIZE,
+                    MR_snprintf(MR_var_name_buf, MR_TRACE_VAR_NAME_BUF_SIZE,
                         "%s (arg %d)", var->MR_var_fullname,
                         var->MR_var_is_headvar);
-#else
-                    sprintf(MR_var_name_buf, "%s (arg %d)",
-                        var->MR_var_fullname, var->MR_var_is_headvar);
-#endif
                 }
             } else {
                 if (var->MR_var_is_ambiguous) {
-#ifdef  MR_HAVE_SNPRINTF
-                    snprintf(MR_var_name_buf, MR_TRACE_VAR_NAME_BUF_SIZE,
+                    MR_snprintf(MR_var_name_buf, MR_TRACE_VAR_NAME_BUF_SIZE,
                         "%s(%d)", var->MR_var_fullname,
                         var->MR_var_hlds_number);
-#else
-                    sprintf(MR_var_name_buf, "%s(%d)",
-                        var->MR_var_fullname, var->MR_var_hlds_number);
-#endif
                 } else {
-#ifdef  MR_HAVE_SNPRINTF
-                    snprintf(MR_var_name_buf, MR_TRACE_VAR_NAME_BUF_SIZE, "%s",
-                        var->MR_var_fullname);
-#else
-                    sprintf(MR_var_name_buf, "%s", var->MR_var_fullname);
-#endif
+                    MR_snprintf(MR_var_name_buf, MR_TRACE_VAR_NAME_BUF_SIZE,
+                        "%s", var->MR_var_fullname);
                 }
             }
 
             break;
     }
 
     return MR_var_name_buf;
 }
 
 static  const char *
-- 
2.18.0



More information about the reviews mailing list