[m-rev.] for post-commit review; fix system RNG handles for non-unrandom C implementations

Julien Fischer jfischer at opturion.com
Sat Feb 20 16:08:44 AEDT 2021


Hi Peter,

On Sat, 20 Feb 2021, Peter Wang wrote:

> BTW, I would like to see as much of the code in the C foreign_decl moved
> into the foreign_code block, to reduce the amount of #includes that end
> up in the .mh file, which would then be (unnecessarily) included in the
> C files of any modules that import the system_rng module.
> It will require marking foreign_procs with may_not_duplicate to ensure
> they aren't opt-exported, but that's fine.

The diff below does that:

Julien.

------------------------

Respond to review comments.

library/random.system_rng.m:
      Shift most of the #includes into a local foreign_decl pragma
      so that they do not get included in the .mh file.

      Move the C# and Java foreign code pragmas to the end of the file,
      with the C one.

diff --git a/library/random.system_rng.m b/library/random.system_rng.m
index 205a612..383e218 100644
--- a/library/random.system_rng.m
+++ b/library/random.system_rng.m
@@ -97,41 +97,18 @@

  %---------------------------------------------------------------------------%

-
  :- pragma foreign_decl("C", "

  #include \"mercury_conf_param.h\"
-#include \"mercury_memory.h\"
-#include \"mercury_misc.h\"
-#include \"mercury_runtime_util.h\"
-#include \"mercury_std.h\"
-#include \"mercury_string.h\"
-#include \"mercury_types.h\"
-#include \"mercury_windows.h\"

-#if defined(MR_HAVE_UNISTD_H)
-    #include <unistd.h>
-#endif
-#if defined(MR_HAVE_FCNTL_H)
-    #include <fcntl.h>
-#endif
-#if defined(MR_HAVE_SYS_STAT_H)
-    #include <sys/stat.h>
-#endif
  #if defined(MR_HAVE_SYS_PARAM_H)
      #include <sys/param.h>
  #endif
+
  #if defined(MR_MAC_OSX)
      #include <AvailabilityMacros.h>
  #endif

-#include <errno.h>
-#include <stdint.h>
-
-// On Windows we need to ensure that _CRT_RAND_S is defined before stdlib.h
-// is included but this must be done in runtime/mercury_std.h.
-#include <stdlib.h>
-
  // The following macros define if the system random number exists on this
  // system and, if so, how it is accessed.
  //
@@ -195,129 +172,6 @@ struct ML_SystemRandomHandle_Struct {
  };
  typedef struct ML_SystemRandomHandle_Struct *ML_SystemRandomHandle;

-// When succeeded is MR_TRUE, returns a handle through which the system
-// RNG can be accessed; err_msg will point to the empty string in this case.
-// When succeeded is MR_FALSE, the value return is not a valid handle and
-// err_msg will point to a string (on the Mercury heap) describing why a handle
-// for the system RNG could not be acquired.
-//
-extern ML_SystemRandomHandle ML_random_open(MR_Bool *succeeded,
-    MR_String *err_msg);
-
-// Attempt to close the handle to the system RNG.
-// Returns MR_TRUE on success with err_msg pointing to the empty string.
-// Returns MR_FALSE on failure with err_msg pointing to a string (on the
-// Mercury heap) that describes whey the handle could not be closed.
-//
-extern MR_Bool ML_random_close(ML_SystemRandomHandle handle, MR_String *err_msg);
-
-// Fill buffer with len random bytes generated by the system RNG.
-// Returns MR_TRUE if len bytes were generated; err_msg will point to the empty
-// string.
-// Returns MR_FALSE if the system RNG has been unable to generate len bytes.
-// In this case the contents of buffer are indeterminate and err_msg will point
-// to a string (on the Mercury heap) that describes the problem.
-//
-extern MR_Bool ML_random_generate_bytes(ML_SystemRandomHandle handle,
-    unsigned char *buffer, size_t len, MR_String *err_msg);
-").
-
-:- pragma foreign_code("C#", "
-
-    public class ML_SystemRandomHandle {
-
-        private System.Security.Cryptography.RandomNumberGenerator handle;
-
-        public ML_SystemRandomHandle() {
-            handle =
-                System.Security.Cryptography.RandomNumberGenerator.Create();
-        }
-
-        public void close() {
-            handle = null;
-        }
-
-        public bool isClosed() {
-            return (handle == null);
-        }
-
-        public byte getByte() {
-            byte[] bytes = new byte[1];
-            handle.GetBytes(bytes);
-            return bytes[0];
-        }
-
-        public ushort getUShort() {
-            byte[] bytes = new byte[2];
-            handle.GetBytes(bytes);
-            return (ushort) (bytes[1] << 8 | (bytes[0] & 0x00ff));
-        }
-
-        public uint getUInt() {
-            byte[] bytes = new byte[4];
-            handle.GetBytes(bytes);
-            return (uint) (
-                bytes[3] << 24 |
-                bytes[2] << 16 |
-                bytes[1] << 8  |
-                bytes[0]);
-        }
-
-        public ulong getULong() {
-            byte[] bytes = new byte[8];
-            handle.GetBytes(bytes);
-            return (ulong) (
-                (ulong) bytes[7] << 56 |
-                (ulong) bytes[6] << 48 |
-                (ulong) bytes[5] << 40 |
-                (ulong) bytes[4] << 32 |
-                (ulong) bytes[3] << 24 |
-                (ulong) bytes[2] << 16 |
-                (ulong) bytes[1] << 8  |
-                (ulong) bytes[0]);
-        }
-    }
-").
-
-:- pragma foreign_code("Java", "
-
-    public static class ML_SystemRandomHandle {
-
-        private java.security.SecureRandom handle;
-
-        public ML_SystemRandomHandle() {
-            handle = new java.security.SecureRandom();
-        }
-
-        public void close() {
-            handle = null;
-        }
-
-        public boolean isClosed() {
-            return (handle == null);
-        }
-
-        public byte getByte() {
-            byte[] bytes = new byte[1];
-            handle.nextBytes(bytes);
-            return bytes[0];
-        }
-
-        public short getShort() {
-            byte[] bytes = new byte[2];
-            handle.nextBytes(bytes);
-            return (short)
-                (bytes[0] << java.lang.Byte.SIZE | (bytes[1] & 0x00ff));
-        }
-
-        public int getInt() {
-            return handle.nextInt();
-        }
-
-        public long getLong() {
-            return handle.nextLong();
-        }
-    }
  ").

  %---------------------------------------------------------------------------%
@@ -391,7 +245,8 @@ open_system_rng(Result, !IO) :-
  :- pragma foreign_proc("C",
      do_open_system_rng(Handle::out, IsOk::out, ErrorMsg::out,
          _IO0::di, _IO::uo),
-    [will_not_call_mercury, promise_pure, thread_safe, tabled_for_io],
+    [will_not_call_mercury, promise_pure, thread_safe, tabled_for_io,
+        may_not_duplicate],
  "
      Handle = ML_random_open(&IsOk, &ErrorMsg);
  ").
@@ -433,7 +288,8 @@ close_system_rng(Handle, !IO) :-
  :- pragma foreign_proc("C",
      do_close_system_rng(Handle::in, IsOk::out, ErrorMsg::out,
          _IO0::di, _IO::uo),
-    [will_not_call_mercury, promise_pure, thread_safe],
+    [will_not_call_mercury, promise_pure, thread_safe,
+        may_not_duplicate],
  "
      IsOk = ML_random_close(Handle, &ErrorMsg);
  ").
@@ -485,7 +341,8 @@ generate_uint8(Handle, U8, !IO) :-
  :- pragma foreign_proc("C",
      do_generate_uint8(Handle::in, U8::out, IsOk::out, ErrorMsg::out,
          _IO0::di, _IO::uo),
-    [will_not_call_mercury, promise_pure, thread_safe, tabled_for_io],
+    [will_not_call_mercury, promise_pure, thread_safe, tabled_for_io,
+        may_not_duplicate],
  "
      union {
          uint8_t n;
@@ -545,7 +402,8 @@ generate_uint16(Handle, U16, !IO) :-
  :- pragma foreign_proc("C",
      do_generate_uint16(Handle::in, U16::out, IsOk::out, ErrorMsg::out,
          _IO0::di, _IO::uo),
-    [will_not_call_mercury, promise_pure, thread_safe, tabled_for_io],
+    [will_not_call_mercury, promise_pure, thread_safe, tabled_for_io,
+        may_not_duplicate],
  "
      union {
          uint16_t n;
@@ -605,7 +463,8 @@ generate_uint32(Handle, U32, !IO) :-
  :- pragma foreign_proc("C",
      do_generate_uint32(Handle::in, U32::out, IsOk::out, ErrorMsg::out,
          _IO0::di, _IO::uo),
-    [will_not_call_mercury, promise_pure, thread_safe, tabled_for_io],
+    [will_not_call_mercury, promise_pure, thread_safe, tabled_for_io,
+        may_not_duplicate],
  "
      union {
          uint32_t n;
@@ -665,7 +524,8 @@ generate_uint64(Handle, U64, !IO) :-
  :- pragma foreign_proc("C",
      do_generate_uint64(Handle::in, U64::out, IsOk::out, ErrorMsg::out,
          _IO0::di, _IO::uo),
-    [will_not_call_mercury, promise_pure, thread_safe, tabled_for_io],
+    [will_not_call_mercury, promise_pure, thread_safe, tabled_for_io,
+        may_not_duplicate],
  "
      union {
          uint64_t n;
@@ -718,11 +578,69 @@ throw_system_rng_error(Pred, Msg) :-

  %---------------------------------------------------------------------------%

+:- pragma foreign_decl("C", local, "
+
+#include \"mercury_memory.h\"          // For MR_GC_NEW.
+#include \"mercury_misc.h\"            // For MR_fatal_error.
+#include \"mercury_runtime_util.h\"    // For MR_sterror.
+#include \"mercury_regs.h\"            // For MR_{save,restore}_transient_hp
+#include \"mercury_reg_workarounds.h\" // For MR_memcpy.
+#include \"mercury_std.h\"             // For MR_TRUE, MR_FALSE etc.
+#include \"mercury_string.h\"          // For MR_make_aligned_string_copy etc.
+#include \"mercury_types.h\"           // For MR_Bool.
+#include \"mercury_windows.h\"
+
+#if defined(ML_SYSRAND_IMPL_URANDOM)
+  #if defined(MR_HAVE_UNISTD_H)
+    #include <unistd.h>
+  #endif
+  #if defined(MR_HAVE_FCNTL_H)
+    #include <fcntl.h>
+  #endif
+  #if defined(MR_HAVE_SYS_STAT_H)
+    #include <sys/stat.h>
+  #endif
+#endif
+
+#include <errno.h>
+#include <stdint.h>
+
+// On Windows we need to ensure that _CRT_RAND_S is defined before stdlib.h
+// is included but this must be done in runtime/mercury_std.h.
+#include <stdlib.h>
+
+// When succeeded is MR_TRUE, returns a handle through which the system
+// RNG can be accessed; err_msg will point to the empty string in this case.
+// When succeeded is MR_FALSE, the value return is not a valid handle and
+// err_msg will point to a string (on the Mercury heap) describing why a handle
+// for the system RNG could not be acquired.
+//
+static ML_SystemRandomHandle ML_random_open(MR_Bool *succeeded,
+    MR_String *err_msg);
+
+// Attempt to close the handle to the system RNG.
+// Returns MR_TRUE on success with err_msg pointing to the empty string.
+// Returns MR_FALSE on failure with err_msg pointing to a string (on the
+// Mercury heap) that describes why the handle could not be closed.
+//
+static MR_Bool ML_random_close(ML_SystemRandomHandle handle, MR_String *err_msg);
+
+// Fill buffer with len random bytes generated by the system RNG.
+// Returns MR_TRUE if len bytes were generated; err_msg will point to the empty
+// string.
+// Returns MR_FALSE if the system RNG has been unable to generate len bytes.
+// In this case the contents of buffer are indeterminate and err_msg will point
+// to a string (on the Mercury heap) that describes the problem.
+//
+static MR_Bool ML_random_generate_bytes(ML_SystemRandomHandle handle,
+    unsigned char *buffer, size_t len, MR_String *err_msg);
+").
+
  :- pragma foreign_code("C", "

  #if defined(ML_SYSRAND_IMPL_URANDOM)

-ML_SystemRandomHandle
+static ML_SystemRandomHandle
  ML_random_open(MR_Bool *succeeded, MR_String *err_msg)
  {
      int fd;
@@ -751,7 +669,7 @@ ML_random_open(MR_Bool *succeeded, MR_String *err_msg)
      return handle;
  }

-MR_Bool
+static MR_Bool
  ML_random_close(ML_SystemRandomHandle handle, MR_String *err_msg)
  {
      char errbuf[MR_STRERROR_BUF_SIZE];
@@ -770,7 +688,7 @@ ML_random_close(ML_SystemRandomHandle handle, MR_String *err_msg)
      }
  }

-MR_Bool
+static MR_Bool
  ML_random_generate_bytes(ML_SystemRandomHandle handle,
      unsigned char *buffer, size_t len, MR_String *err_msg)
  {
@@ -801,7 +719,7 @@ ML_random_generate_bytes(ML_SystemRandomHandle handle,

  #elif defined(ML_SYSRAND_IMPL_ARC4RANDOM)

-ML_SystemRandomHandle
+static ML_SystemRandomHandle
  ML_random_open(MR_Bool *succeeded, MR_String *err_msg)
  {
      ML_SystemRandomHandle handle =
@@ -812,7 +730,7 @@ ML_random_open(MR_Bool *succeeded, MR_String *err_msg)
      return handle;
  }

-MR_Bool
+static MR_Bool
  ML_random_close(ML_SystemRandomHandle handle, MR_String *err_msg)
  {
      if (handle->ML_srh_is_open) {
@@ -826,7 +744,7 @@ ML_random_close(ML_SystemRandomHandle handle, MR_String *err_msg)
      }
  }

-MR_Bool
+static MR_Bool
  ML_random_generate_bytes(ML_SystemRandomHandle handle,
      unsigned char *buffer, size_t len, MR_String *err_msg)
  {
@@ -842,7 +760,7 @@ ML_random_generate_bytes(ML_SystemRandomHandle handle,

  #elif defined(ML_SYSRAND_IMPL_RAND_S)

-ML_SystemRandomHandle
+static ML_SystemRandomHandle
  ML_random_open(MR_Bool *succeeded, MR_String *err_msg)
  {
      ML_SystemRandomHandle handle =
@@ -853,7 +771,7 @@ ML_random_open(MR_Bool *succeeded, MR_String *err_msg)
      return handle;
  }

-MR_Bool
+static MR_Bool
  ML_random_close(ML_SystemRandomHandle handle, MR_String *err_msg)
  {
      if (handle->MR_srh_is_open) {
@@ -867,7 +785,7 @@ ML_random_close(ML_SystemRandomHandle handle, MR_String *err_msg)
      }
  }

-MR_Bool
+static MR_Bool
  ML_random_generate_bytes(ML_SystemRandomHandle handle,
      unsigned char *buffer, size_t len, MR_String *err_msg)
  {
@@ -909,7 +827,7 @@ rand_s_failure_handler:

  #else // ML_SYSRAND_IMPL_NONE

-ML_SystemRandomHandle
+static ML_SystemRandomHandle
  ML_random_open(MR_Bool *succeeded, MR_String *err_msg)
  {
      succeeded = MR_FALSE;
@@ -917,13 +835,13 @@ ML_random_open(MR_Bool *succeeded, MR_String *err_msg)
      return 0; // Dummy value.
  }

-MR_Bool
+static MR_Bool
  ML_random_close(ML_SystemRandomHandle handle, MR_String *err_msg)
  {
      MR_fatal_error(\"ML_random_close - no system RNG available.\");
  }

-MR_Bool
+static MR_Bool
  ML_random_generate_bytes(ML_SystemRandomHandle handle,
      unsigned char *buffer, size_t len, MR_String *err_msg)
  {
@@ -935,5 +853,108 @@ ML_random_generate_bytes(ML_SystemRandomHandle handle,
  ").

  %---------------------------------------------------------------------------%
+
+:- pragma foreign_code("C#", "
+
+    public class ML_SystemRandomHandle {
+
+        private System.Security.Cryptography.RandomNumberGenerator handle;
+
+        public ML_SystemRandomHandle() {
+            handle =
+                System.Security.Cryptography.RandomNumberGenerator.Create();
+        }
+
+        public void close() {
+            handle = null;
+        }
+
+        public bool isClosed() {
+            return (handle == null);
+        }
+
+        public byte getByte() {
+            byte[] bytes = new byte[1];
+            handle.GetBytes(bytes);
+            return bytes[0];
+        }
+
+        public ushort getUShort() {
+            byte[] bytes = new byte[2];
+            handle.GetBytes(bytes);
+            return (ushort) (bytes[1] << 8 | (bytes[0] & 0x00ff));
+        }
+
+        public uint getUInt() {
+            byte[] bytes = new byte[4];
+            handle.GetBytes(bytes);
+            return (uint) (
+                bytes[3] << 24 |
+                bytes[2] << 16 |
+                bytes[1] << 8  |
+                bytes[0]);
+        }
+
+        public ulong getULong() {
+            byte[] bytes = new byte[8];
+            handle.GetBytes(bytes);
+            return (ulong) (
+                (ulong) bytes[7] << 56 |
+                (ulong) bytes[6] << 48 |
+                (ulong) bytes[5] << 40 |
+                (ulong) bytes[4] << 32 |
+                (ulong) bytes[3] << 24 |
+                (ulong) bytes[2] << 16 |
+                (ulong) bytes[1] << 8  |
+                (ulong) bytes[0]);
+        }
+    }
+").
+
+%---------------------------------------------------------------------------%
+
+:- pragma foreign_code("Java", "
+
+    public static class ML_SystemRandomHandle {
+
+        private java.security.SecureRandom handle;
+
+        public ML_SystemRandomHandle() {
+            handle = new java.security.SecureRandom();
+        }
+
+        public void close() {
+            handle = null;
+        }
+
+        public boolean isClosed() {
+            return (handle == null);
+        }
+
+        public byte getByte() {
+            byte[] bytes = new byte[1];
+            handle.nextBytes(bytes);
+            return bytes[0];
+        }
+
+        public short getShort() {
+            byte[] bytes = new byte[2];
+            handle.nextBytes(bytes);
+            return (short)
+                (bytes[0] << java.lang.Byte.SIZE | (bytes[1] & 0x00ff));
+        }
+
+        public int getInt() {
+            return handle.nextInt();
+        }
+
+        public long getLong() {
+            return handle.nextLong();
+        }
+    }
+").
+
+
+%---------------------------------------------------------------------------%
  :- end_module random.system_rng.
  %---------------------------------------------------------------------------%


More information about the reviews mailing list