[PATCH] libcxx random_device for Win32

Yaron Keren yaron.keren at gmail.com
Tue Oct 8 10:16:28 PDT 2013


Hi, no problem, updated.

Yaron



2013/10/8 Marshall Clow <mclow.lists at gmail.com>

>
> On Oct 8, 2013, at 8:15 AM, Yaron Keren <yaron.keren at gmail.com> wrote:
>
> Sure, updated with #if defined.
>
>
> Sadly (for you), I thought of one other stylistic nit while I was out
> walking.
>
> If you're getting rand_s from <stdlib.h>, wouldn't it be better to include
> it explicitly,
> rather than depending on <random> or some other header to include it for
> you?
>
>
> #if defined(_WIN32)
> // Must be defined before stdlib.h is included to enable rand_s().
> #define _CRT_RAND_S
> #include <stdio.h>
> #endif
>
> [ This is a suggestion, not something that I'm insisting on ]
>
> Other than that, LGTM!
>
> -- Marshall
>
>
>
> 2013/10/8 Marshall Clow <mclow.lists at gmail.com>
>
>> On Oct 8, 2013, at 4:31 AM, Yaron Keren <yaron.keren at gmail.com> wrote:
>>
>> Hi,
>>
>> I fixed the issues.
>> __f_ data member will be used if !_WIN32 as a file descriptor so it's
>> #ifdef-d.
>>
>>
>> Shouldn't the "#  define _CRT_RAND_S" line be wrapped in "#if
>> defined(_WIN32)"
>>
>> like this:
>>
>> #if defined(_WIN32)
>> // Must be defined before stdlib.h is included to enable rand_s().
>> #define _CRT_RAND_S
>> #endif
>>
>> -- Marshall
>>
>>
>>
>>
>> Yaron
>>
>>
>>
>> 2013/10/8 Nico Rieck <nico.rieck at gmail.com>
>>
>>>  #  endif
>>> +#  // Must be defined before stdlib.h is included to enable rand_s().
>>> +#  define _CRT_RAND_S
>>>  #endif  // _WIN32
>>>
>>> Since rand_s is never used in a libc++ header, I don't think it should
>>> be defined here.
>>>
>>> There are also trailing whitespace in the patch. The __f_ data member is
>>> unused and should be removed. (It also can't hold any HANDLE if that's the
>>> reason you kept it around.) And I see no need to duplicate entropy().
>>>
>>> -Nico
>>>
>>
>> <libcxx-rands.diff>_______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>>
>> -- Marshall
>>
>> Marshall Clow     Idio Software   <mailto:mclow.lists at gmail.com<mclow.lists at gmail.com>
>> >
>>
>> A.D. 1517: Martin Luther nails his 95 Theses to the church door and is
>> promptly moderated down to (-1, Flamebait).
>>         -- Yu Suzuki
>>
>>
> <libcxx-rands.diff>
>
>
> -- Marshall
>
> Marshall Clow     Idio Software   <mailto:mclow.lists at gmail.com<mclow.lists at gmail.com>
> >
>
> A.D. 1517: Martin Luther nails his 95 Theses to the church door and is
> promptly moderated down to (-1, Flamebait).
>         -- Yu Suzuki
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131008/3cadad81/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: libcxx-rands.diff
Type: application/octet-stream
Size: 1418 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131008/3cadad81/attachment.obj>


More information about the cfe-commits mailing list