defuse / passgen

A password generator.
78 stars 25 forks source link

Leaks fixed #26

Closed offa closed 9 years ago

offa commented 9 years ago

I have fixed three (possible) leaks.

defuse commented 9 years ago

Thanks!

The tests fail because we actually need more than 5 digits for the number of samples, to run statistical tests:

https://github.com/defuse/passgen/blob/master/tools/statistical_test.rb#L2

I'll fix that up then merge it. Why is %5d better than %d, is there some risk to using %d?

offa commented 9 years ago

The tests fail because we actually need more than 5 digits for the number of samples, to run statistical tests:

I see. Was realy surprised - my changes aren't that big to fail the tests.

Why is %5d better than %d, is there some risk to using %d?

With %5d scanf() parses only 5 digits; without you can feed as much digits you can supply. This can cause problems up to a crash.

defuse commented 9 years ago

On 10/20/2014 01:45 PM, offa wrote:

The tests fail because we actually need more than 5 digits for the number of samples, to run statistical tests:

I see. Was realy surprised - my changes aren't that big to fail the tests.

Hehehe. I have strict tests. ;)

Why is %5d better than %d, is there some risk to using %d?

With %5d scanf() parses only 5 digits; without you can feed as much digits you can supply. This can cause problems up to a crash.

Makes sense. I'll use %10d since that can represent at least 32-bit numbers.

offa commented 9 years ago

I've pushed a commit with length set to 10 - let's see, what travis says now.

coveralls commented 9 years ago

Coverage Status

Coverage decreased (-0.44%) when pulling b01d0169adfdaa01e63ac279ccfd456b1a0de2a9 on offa:leaks_fixed into 4615c2065ed3f811ffa9645ff183116c6307655c on defuse:master.

offa commented 9 years ago

Ok, better now - but not perfect.

defuse commented 9 years ago

Awesome, thanks very much!