Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added random_device to intarray.cpp #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

added random_device to intarray.cpp #5

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion qapi/protocols/cryptography/key_distribution/cpp/IntArray.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,14 @@ class IntArray {
// Allocates new memory so must be freed
int* random_array = new int[size];
for (int i = 0; i < size; i++) {
random_array[i] = rand() % 2;
try
{
std::random_device rd;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move std::random device outside of the loop, calling it multiple times is redundant.

Copy link
Owner

@seunomonije seunomonije Feb 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See here for an example of it being used. Otp we discussed and I read up some more on it.
std::random_device can fail

  • On construction, ie std::random_device [name], throwing an exception and exiting the whole program if left uncaught.
  • Or when requesting a random number, ie rd() % 2. Currently your code would catch this and use srand() in the same spot.

I think you have 2 options here. One is the easy way out and the other is more proper/more involved, but whichever you want to do is fine with me:

  • For our purposes it's fine if the whole program exits with an exception if std::random_device fails.
  • Possibly, a better way to construct this is to have 2 separate classes, one IntArraySrand and IntArrayRandDevice or something like that, and in a parent class IntArray attempt to build the array using IntArrayRandDevice and if that fails, build it using IntArraySrand. This way you could have std::random_device contained within the IntArrayRandDevice class and so if it fails when constructing the class you know you can't use it anymore.

Copy link
Collaborator

@nsnave nsnave Feb 26, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The second option using the classes would also allow for easier reuse later; notably, being able to combine the generators in different ways for different purposes that appeal better to the different rngs' strengths.

random_array[i] = rd() % 2;
} catch(...)
Copy link
Owner

@seunomonije seunomonije Feb 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try catch rather than actually checking for errors? EDIT: I actually like the try catch. See above.

{
random_array[i] = rand() % 2;
}
}
return random_array;
}
Expand Down