On Oct 8, 2013, at 8:15 AM, Yaron Keren <[email protected]> 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 <[email protected]> > On Oct 8, 2013, at 4:31 AM, Yaron Keren <[email protected]> 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 <[email protected]> >> # 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 >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > > -- Marshall > > Marshall Clow Idio Software <mailto:[email protected]> > > 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:[email protected]> A.D. 1517: Martin Luther nails his 95 Theses to the church door and is promptly moderated down to (-1, Flamebait). -- Yu Suzuki
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
