<div dir="rtl"><div dir="ltr">Hi, no problem, updated.</div><div dir="ltr"><br></div><div dir="ltr">Yaron</div><div dir="ltr"><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote"><div dir="ltr">2013/10/8 Marshall Clow <span dir="ltr"><<a href="mailto:mclow.lists@gmail.com" target="_blank">mclow.lists@gmail.com</a>></span></div>
<blockquote class="gmail_quote" style="margin:0 .8ex;border-left:1px #ccc solid;border-right:1px #ccc solid;padding-left:1ex;padding-right:1ex"><div style="word-wrap:break-word"><br><div><div class="im"><div>On Oct 8, 2013, at 8:15 AM, Yaron Keren <<a href="mailto:yaron.keren@gmail.com" target="_blank">yaron.keren@gmail.com</a>> wrote:</div>
<br><blockquote type="cite"><div dir="rtl"><div dir="ltr">Sure, updated with #if defined.</div></div></blockquote><div><br></div></div>Sadly (for you), I thought of one other stylistic nit while I was out walking.</div><div>
<br></div><div>If you're getting rand_s from <stdlib.h>, wouldn't it be better to include it explicitly, </div><div>rather than depending on <random> or some other header to include it for you?</div><div>
<br></div><div><br></div><div>#if defined(_WIN32)</div><div><div class="im">// Must be defined before stdlib.h is included to enable rand_s().<br>#define _CRT_RAND_S<br></div>#include <stdio.h></div><div>#endif</div>
<div><br></div><div>[ This is a suggestion, not something that I'm insisting on ]</div><div><br></div><div>Other than that, LGTM!</div><div><br></div><div>-- Marshall</div><div><br></div><div><br></div><div><blockquote type="cite">
<div><div class="h5"><div class="gmail_extra"><div class="gmail_quote"><div dir="ltr"><br></div><div dir="ltr">2013/10/8 Marshall Clow <span dir="ltr"><<a href="mailto:mclow.lists@gmail.com" target="_blank">mclow.lists@gmail.com</a>></span></div>
<blockquote class="gmail_quote" style="margin:0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;border-right-width:1px;border-right-color:rgb(204,204,204);border-right-style:solid;padding-left:1ex;padding-right:1ex">
<div style="word-wrap:break-word"><div><div><div>On Oct 8, 2013, at 4:31 AM, Yaron Keren <<a href="mailto:yaron.keren@gmail.com" target="_blank">yaron.keren@gmail.com</a>> wrote:</div><br><blockquote type="cite">
<div dir="rtl"><div dir="ltr">Hi,</div><div dir="ltr"><br></div><div dir="ltr">I fixed the issues.</div><div dir="ltr">__f_ data member will be used if !_WIN32 as a file descriptor so it's #ifdef-d.</div></div></blockquote>
<div><br></div></div>Shouldn't the "# define _CRT_RAND_S" line be wrapped in "#if defined(_WIN32)"</div><div><br></div><div>like this:</div><div><br></div><div><div>#if defined(_WIN32)</div><div>
<div>// Must be defined before stdlib.h is included to enable rand_s().</div></div><div>#define _CRT_RAND_S</div><div>#endif</div><div><br></div></div><div>-- Marshall</div><div><br></div><div><div><br></div><br><blockquote type="cite">
<div><div dir="rtl"><div dir="ltr">
<br></div><div dir="ltr">Yaron</div><div dir="ltr"><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote"><div dir="ltr">2013/10/8 Nico Rieck <span dir="ltr"><<a href="mailto:nico.rieck@gmail.com" target="_blank">nico.rieck@gmail.com</a>></span></div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"> # endif<br>
+# // Must be defined before stdlib.h is included to enable rand_s().<br>
+# define _CRT_RAND_S<br>
#endif // _WIN32<br>
<br>
Since rand_s is never used in a libc++ header, I don't think it should be defined here.<br>
<br>
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().<span><font color="#888888"><br>
<br>
-Nico<br>
</font></span></blockquote></div><br></div>
</div><span><libcxx-rands.diff></span>_______________________________________________<br>cfe-commits mailing list<br><a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br><div>
<span style="border-collapse:separate;border-spacing:0px">-- Marshall<br><br>Marshall Clow Idio Software <<a href="mailto:mclow.lists@gmail.com" target="_blank">mailto:mclow.lists@gmail.com</a>><br><br>A.D. 1517: Martin Luther nails his 95 Theses to the church door and is promptly moderated down to (-1, Flamebait).<br>
-- Yu Suzuki</span>
</div>
<br></div></blockquote></div><br></div>
</div></div><span><libcxx-rands.diff></span></blockquote></div><div class="im"><br><div>
<span style="border-collapse:separate;border-spacing:0px">-- Marshall<br><br>Marshall Clow Idio Software <<a href="mailto:mclow.lists@gmail.com" target="_blank">mailto:mclow.lists@gmail.com</a>><br><br>A.D. 1517: Martin Luther nails his 95 Theses to the church door and is promptly moderated down to (-1, Flamebait).<br>
-- Yu Suzuki</span>
</div>
<br></div></div></blockquote></div><br></div>