2 ms·

It's not just the errors that were made though. I mean, ignoring the off-by-one error: repeat byte250 = randomSalsaByte() until byte250 <= 250 I am

by jlgreco 13y ago

It's not just the errors that were made though. I mean, ignoring the off-by-one error: repeat byte250 = randomSalsaByte() until byte250 <= 250 I am not sure what sort of code-review process they had in place if someone saw that and thought "Yup, brilliant. Ship it." I am sure we have all written code like that at one point in our lives, but code that is shipping to the general public?

mistercow 13y ago
How else do you generate an unbiased random number between 0 and 250? As far as I know, that's the standard algorithm.
- jlgreco 13y ago
  My concern is with why they are doing that in the first place. It is possible that I am missing something here, I haven't looked at their actual code, but it seems to me that this concern would have been entirely avoided if they went with 0-F instead of 0-9.
  - mistercow 13y ago
    Yes, I definitely agree with that, and I don't understand why they did it that way. Generating a float in the [0,1) range by producing decimal digits seems utterly perverse. The sane, naive, and correct way to do it is to generate an integer between 0 and 2^53 and then divide by 2^53.
    - marshray 13y ago
      Or use an array of integers 0 <= n < 2^x. Some of the Javascript libraries involved are even using such a format.
- mischanix 13y ago
  Perhaps grab 5 bytes instead?
billforsternz 13y ago
It's not the most elegant code I've ever seen - but I don't otherwise see a red flag. What's your problem with it ? I don't think the worry that a semi-infinite string of legitimate values >250 is going to come out of randomSalsaByte() is a real, practical problem. You may as well worry that the complete works of Shakespeare asciified might come out of the PRNG. In theory it could happen. In practice, it won't happen even once in 10^100 universe lifetimes. The transistors in the CPU are going to spontaneously fail whilst executing the code zillions of times more often.
- axman6 13y ago
  The issue isn't the potentially infinite loop, it's the fact it creates an uneven distribution of values, giving 0 a 1/250 greater chance of being selected, and thus makes the results of the PRNG more predictable. The article makes this pretty clear...
  - mpyne 13y ago
    The article also makes clear that it's a standard fencepost error. Freakishly poor crypto, but not bush-league programming.
  - billforsternz 13y ago
    What part of "ignoring the off by one error" in the parent don't you get ? Edit: Sorry, unnecessarily snarky, and I could have made my (now uneditable) post clearer by saying ">=250" rather than ">250".

by jlgreco 13y ago

mistercow 13y ago

How else do you generate an unbiased random number between 0 and 250? As far as I know, that's the standard algorithm.

jlgreco 13y ago
My concern is with why they are doing that in the first place. It is possible that I am missing something here, I haven't looked at their actual code, but it seems to me that this concern would have been entirely avoided if they went with 0-F instead of 0-9.
- mistercow 13y ago
  Yes, I definitely agree with that, and I don't understand why they did it that way. Generating a float in the [0,1) range by producing decimal digits seems utterly perverse. The sane, naive, and correct way to do it is to generate an integer between 0 and 2^53 and then divide by 2^53.
  - marshray 13y ago
    Or use an array of integers 0 <= n < 2^x. Some of the Javascript libraries involved are even using such a format.
mischanix 13y ago
Perhaps grab 5 bytes instead?

billforsternz 13y ago

It's not the most elegant code I've ever seen - but I don't otherwise see a red flag. What's your problem with it ? I don't think the worry that a semi-infinite string of legitimate values >250 is going to come out of randomSalsaByte() is a real, practical problem. You may as well worry that the complete works of Shakespeare asciified might come out of the PRNG. In theory it could happen. In practice, it won't happen even once in 10^100 universe lifetimes. The transistors in the CPU are going to spontaneously fail whilst executing the code zillions of times more often.

axman6 13y ago
The issue isn't the potentially infinite loop, it's the fact it creates an uneven distribution of values, giving 0 a 1/250 greater chance of being selected, and thus makes the results of the PRNG more predictable. The article makes this pretty clear...
- mpyne 13y ago
  The article also makes clear that it's a standard fencepost error. Freakishly poor crypto, but not bush-league programming.
- billforsternz 13y ago
  What part of "ignoring the off by one error" in the parent don't you get ? Edit: Sorry, unnecessarily snarky, and I could have made my (now uneditable) post clearer by saying ">=250" rather than ">250".