Hi, no problem, updated. Yaron
2013/10/8 Marshall Clow <[email protected]> > > 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]<[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]<[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
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
