Saurabh, can you post a final patch?
On Fri, Dec 13, 2013 at 11:18:17AM -0800, Saurabh Shah wrote: > I don't have a strong preference about the structuring. Anything that is > intuitive is fine with me. With regards to keeping all pragma comments > separate, I actually like keeping them close to the files that use them. > However, since these are DLLs, I don't see any major downside in declaring > them in a separate file (except that the pragma declare file might get > out-of-sync with the actual libraries used by the code). > > Thanks! > Saurabh > > From: Alin Serdean > <aserd...@cloudbasesolutions.com<mailto:aserd...@cloudbasesolutions.com>> > Date: Friday, December 13, 2013 10:28 AM > To: Saurabh Shah <ssaur...@vmware.com<mailto:ssaur...@vmware.com>>, > "b...@nicira.com<mailto:b...@nicira.com>" > <b...@nicira.com<mailto:b...@nicira.com>>, > "shet...@nicira.com<mailto:shet...@nicira.com>" > <shet...@nicira.com<mailto:shet...@nicira.com>> > Cc: "dev@openvswitch.org<mailto:dev@openvswitch.org>" > <dev@openvswitch.org<mailto:dev@openvswitch.org>> > Subject: RE: [ovs-dev] [Windows thread 3] > > I was thinking to include all pragmas and other needed defines in a common > header used through all of the source > files(http://openvswitch.org/pipermail/dev/2013-December/034983.html), but > this is just my thought. > > The way you want to structure it is up too you :-). > > Alin. > ________________________________ > From: Saurabh Shah [ssaur...@vmware.com<mailto:ssaur...@vmware.com>] > Sent: Friday, December 13, 2013 2:41 AM > To: Alin Serdean; b...@nicira.com<mailto:b...@nicira.com>; > shet...@nicira.com<mailto:shet...@nicira.com> > Cc: dev@openvswitch.org<mailto:dev@openvswitch.org> > Subject: Re: [ovs-dev] [Windows thread 3] > > Looks good to me. Just one minor comment. You probably also want to include > the following? > > #include <windows.h> > #pragma comment(lib, "advapi32.lib") > > Btw, printing the error message will require a small change which maps > strerror_r to strerror_s. Not sure, if we should just add it to this commit. > > Saurabh > > From: Alin Serdean > <aserd...@cloudbasesolutions.com<mailto:aserd...@cloudbasesolutions.com>> > Date: Thursday, December 12, 2013 1:59 PM > To: Saurabh Shah <ssaur...@vmware.com<mailto:ssaur...@vmware.com>>, > "b...@nicira.com<mailto:b...@nicira.com>" > <b...@nicira.com<mailto:b...@nicira.com>>, > "shet...@nicira.com<mailto:shet...@nicira.com>" > <shet...@nicira.com<mailto:shet...@nicira.com>> > Cc: "dev@openvswitch.org<mailto:dev@openvswitch.org>" > <dev@openvswitch.org<mailto:dev@openvswitch.org>> > Subject: RE: [ovs-dev] [Windows thread 3] > > Updated. Thanks for the feedback :-). > > Final version would look something like: > > lib/entropy.c | 13 +++++++++++++ > 1 files changed, 13 insertions(+), 0 deletions(-) > --- > diff --git a/lib/entropy.c b/lib/entropy.c > index 02f56e0..45e83ec 100644 > --- a/lib/entropy.c > +++ b/lib/entropy.c > @@ -33,6 +33,7 @@ static const char urandom[] = "/dev/urandom"; > int > get_entropy(void *buffer, size_t n) > { > +#ifndef _WIN32 > size_t bytes_read; > int error; > int fd; > @@ -49,6 +50,18 @@ get_entropy(void *buffer, size_t n) > if (error) { > VLOG_ERR("%s: read error (%s)", urandom, > ovs_retval_to_string(error)); > } > +#else > + int error = 0; > + HCRYPTPROV crypt_prov = 0; > + CryptAcquireContext(&crypt_prov, NULL, NULL, PROV_RSA_FULL, > CRYPT_VERIFYCONTEXT); > + > + if (!CryptGenRandom(crypt_prov, n, buffer)) { > + error = GetLastError(); > + VLOG_ERR("CryptGenRandom: read error (%s)", > ovs_retval_to_string(error)); > + } > + > + CryptReleaseContext(crypt_prov, 0); > +#endif > return error; > } > --- > > If there are no further remarks I would post in a patch. > > Alin. > > ________________________________ > From: Saurabh Shah [ssaur...@vmware.com<mailto:ssaur...@vmware.com>] > Sent: Wednesday, December 11, 2013 9:53 PM > To: Alin Serdean; b...@nicira.com<mailto:b...@nicira.com>; > shet...@nicira.com<mailto:shet...@nicira.com> > Cc: dev@openvswitch.org<mailto:dev@openvswitch.org> > Subject: Re: [ovs-dev] [Windows thread 3] > > > Hey, > > > The following is a quick patch for secure pseudorandom number generator on > windows. I split the functionality with a brutal ifdef macro. Feedback on the > code and suggestions for a nicer implementation is appreciated :). > > diff --git a/lib/entropy.c b/lib/entropy.c > index 02f56e0..ec9d95c 100644 > --- a/lib/entropy.c > +++ b/lib/entropy.c > @@ -20,6 +20,9 @@ > #include <errno.h> > #include <fcntl.h> > #include <unistd.h> > +#ifdef _WIN32 > +#include <Wincrypt.h> > +#endif > > #include "socket-util.h" > #include "vlog.h" > @@ -33,6 +36,7 @@ static const char urandom[] = "/dev/urandom"; > int > get_entropy(void *buffer, size_t n) > { > +#ifndef _WIN32 > size_t bytes_read; > int error; > int fd; > @@ -49,6 +53,20 @@ get_entropy(void *buffer, size_t n) > if (error) { > VLOG_ERR("%s: read error (%s)", urandom, > ovs_retval_to_string(error)); > } > +#else > + int error = 1; > + HCRYPTPROV crypt_prov = 0; > + CryptAcquireContext(&crypt_prov, NULL, NULL, PROV_RSA_FULL, 0); > + > > Microsoft documentation suggests using CRYPT_VERIFYCONTEXT. Although, I > haven't tested to see what sort of an impact this will have. > > http://msdn.microsoft.com/en-us/library/windows/desktop/aa379886(v=vs.85).aspx<https://urldefense.proofpoint.com/v1/url?u=http://msdn.microsoft.com/en-us/library/windows/desktop/aa379886%28v%3Dvs.85%29.aspx&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=pEkjsHfytvHEWufeZPpgqSOJMdMjuZPbesVsNhCUc0E%3D%0A&m=zoQn%2BOKvRPDqVVu5wfXEQ7LzlyDiCzTd%2BoRL3B70vuQ%3D%0A&s=bf3a9912332ddbdb2c2109e0613776296f4ecc3c50b51a489f75b67da27a015f> > For performance reasons, we recommend that you set the pszContainer parameter > to NULL and the dwFlags parameter to CRYPT_VERIFYCONTEXT in all situations > where you do not require a persisted key. In particular, consider setting the > pszContainer parameter to NULL and the dwFlags parameter to > CRYPT_VERIFYCONTEXT for the following scenarios: > > + if (CryptGenRandom(crypt_prov, n, buffer)) { > + error = 0; > + } > + > + if (error) { > + VLOG_ERR("CryptGenRandom: read error (%s)", urandom, > ovs_retval_to_string(error)); > + } > > How about doing instead - > > int error = 0; > If (! CryptGetRandom(crypt_prov, n, buffer)) { > error = GetLastError(); > VLOG_ERR("CryptGenRandom: read error (%s)", urandom, > ovs_retval_to_string(error)); > } > > + CryptReleaseContext(crypt_prov, 0); > +#endif > return error; > } > > > Kind Regards, > > Alin. > > _______________________________________________ > dev mailing list > dev@openvswitch.org<mailto:dev@openvswitch.org> > https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman/listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=pEkjsHfytvHEWufeZPpgqSOJMdMjuZPbesVsNhCUc0E%3D%0A&m=KlUcJXE7spv5Cm%2FmexYFbql6rLI%2BJfpjXWgtb05Lero%3D%0A&s=ccb6c3872370fa5ee60d07509dba3d0a07ebc526274c18553e02ab434de8bcdb > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
