[PATCH] libcxx random_device for Win32 review

[Yaron Keren]

Hi,

1. Yes, already implemented per Nick suggestion.
2. Added.

I would have blended the implementations if there were any code shared
between the implementations.

Since the the only code duplication is the function declarations I find the
"Win32 code" and the "Unix code" being separate easier to read rather than
seeing  #if-#else#-#endif in every function.

Moreover the constructor declaration will not be the same due to the unused
input argument in Win32.

Yaron




2013/10/9 G M <gmisocpp at gmail.com>

> Hi Yaron
>
> Your random_device patch looks good. I'd consider making a few more
> changes though:
>
> 1. I'd #if ! defined(_WIN32) the file handle f in the <random> header
> as the Win32 version's wont use it AFAIKT.
>
> 2. Accordingly, I'd change random.cpp to remove the includes
> relating to files for win32 to emphasis that further. i.e.:
>
> #if ! defined(_WIN32)
>
> #include <fcntl.h>
>
> #include <unistd.h>
>
> #endif
>
> I did wonder if it was worth blending the implementations a bit more to
> avoid replicating the function specifications etc. but on balance I'm not
> suggesting that but others might have an opinion.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131009/8f27d4f8/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: libcxx-rands.diff
Type: application/octet-stream
Size: 1603 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131009/8f27d4f8/attachment.obj>