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

[Stephan Tolksdorf]

On 14-02-04 14:56, Aaron Ballman wrote:
> LGTM!

I forgot to write: I don't have commit access, please commit for me. Thanks!

- Stephan

>
> ~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
>>>>
>>