[Patch] Implement sys::Process::GetRandomNumber on Windows

[Aaron Ballman]

LGTM!

~Aaron

On Tue, Feb 4, 2014 at 7:54 AM, Stephan Tolksdorf <st at quanttec.com> wrote:
> Aaron Ballman wrote:
>>
>> Aside from minor nits, LGTM. Nits below:
>
>
> Thank you for the review! I've attached an updated patch that incorporates
> the suggested changes.
>
> - Stephan
>
>
>>
>>>  From b2976da4fc38c9fb41498915af3d6eee8bac7267 Mon Sep 17 00:00:00 2001
>>> From: Stephan Tolksdorf <st at quanttec.com>
>>> Date: Mon, 3 Feb 2014 21:25:01 +0100
>>> Subject: Implement llvm::sys::Process::GetRandomNumber() for Windows
>>>
>>> This function was already implemented for Unix. The implementation uses
>>> the
>>> rand_s function, which according to MSDN is available on Windows XP and
>>> later.
>>> libc++ also uses rand_s for implementing random_device. I've checked
>>> that mingw-w64 supports rand_s.
>>> ---
>>>   lib/Support/Process.cpp           | 4 ++++
>>>   lib/Support/Windows/Process.inc   | 7 +++++++
>>>   unittests/Support/ProcessTest.cpp | 6 ++++++
>>>   3 files changed, 17 insertions(+)
>>>
>>> diff --git a/lib/Support/Process.cpp b/lib/Support/Process.cpp
>>> index d5168f0..009c7c8 100644
>>> --- a/lib/Support/Process.cpp
>>> +++ b/lib/Support/Process.cpp
>>> @@ -9,12 +9,16 @@
>>>   //
>>>   //  This header file implements the operating system Process concept.
>>>   //
>>>
>>> //===----------------------------------------------------------------------===//
>>>
>>>   #include "llvm/Config/config.h"
>>> +#if LLVM_ON_WIN32
>>> +# define _CRT_RAND_S // makes stdlib.h declare the rand_s function
>>
>>
>> LLVM comments are usually complete sentences (capitalization, period,
>> etc).
>>
>>> +# include <stdlib.h>
>>> +#endif
>>>   #include "llvm/Support/ErrorHandling.h"
>>>   #include "llvm/Support/Process.h"
>>>
>>>   using namespace llvm;
>>>   using namespace sys;
>>>
>>> diff --git a/lib/Support/Windows/Process.inc
>>> b/lib/Support/Windows/Process.inc
>>> index 750097e..73ce97a 100644
>>> --- a/lib/Support/Windows/Process.inc
>>> +++ b/lib/Support/Windows/Process.inc
>>> @@ -357,6 +357,13 @@ const char *Process::OutputReverse() {
>>>
>>>   const char *Process::ResetColor() {
>>>     if (UseANSI) return "\033[0m";
>>>     SetConsoleTextAttribute(GetStdHandle(STD_OUTPUT_HANDLE),
>>> defaultColors());
>>>     return 0;
>>>   }
>>> +
>>> +unsigned Process::GetRandomNumber() {
>>> +  unsigned result;
>>> +  const errno_t ec = rand_s(&result); // supported in Win XP and later
>>
>>
>> I would remove the comment; also, result should be declared to match
>> rand_s's signature (so unsigned int instead of vanilla unsigned).
>>
>>> +  assert(ec == 0 && "rand_s failed");
>>> +  return result;
>>> +}
>>> diff --git a/unittests/Support/ProcessTest.cpp
>>> b/unittests/Support/ProcessTest.cpp
>>> index af6a6f9..387d5d3 100644
>>> --- a/unittests/Support/ProcessTest.cpp
>>> +++ b/unittests/Support/ProcessTest.cpp
>>> @@ -36,12 +36,18 @@ TEST(ProcessTest, SelfProcess) {
>>>     EXPECT_LT(TimeValue::MinTime,
>>> process::get_self()->get_system_time());
>>>     EXPECT_GT(TimeValue::MaxTime,
>>> process::get_self()->get_system_time());
>>>     EXPECT_LT(TimeValue::MinTime, process::get_self()->get_wall_time());
>>>     EXPECT_GT(TimeValue::MaxTime, process::get_self()->get_wall_time());
>>>   }
>>>
>>> +TEST(ProcessTest, GetRandomNumberTest) {
>>> +  const unsigned r1 = Process::GetRandomNumber();
>>> +  const unsigned r2 = Process::GetRandomNumber();
>>> +  EXPECT_NE((r1 | r2), 0); // 0 should be extremely unlikely
>>> +}
>>> +
>>>   #ifdef _MSC_VER
>>>   #define setenv(name, var, ignore) _putenv_s(name, var)
>>>   #endif
>>>
>>>   #if HAVE_SETENV || _MSC_VER
>>>   TEST(ProcessTest, Basic) {
>>> --
>>> 1.7.11.msysgit.1
>>
>>
>> ~Aaron
>>
>> On Mon, Feb 3, 2014 at 7:03 PM, Stephan Tolksdorf <st at quanttec.com> wrote:
>>>
>>> Hi,
>>>
>>> This is a little patch that implements sys::Process::GetRandomNumber on
>>> Windows. Currently this function only has a Unix implementation.
>>>
>>> The implementation uses the s_rand function, which is available on
>>> Windows
>>> XP and later. It's the same function that libc++ uses for implementing
>>> random_device on Windows.
>>>
>>> - Stephan
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>