Code review - Issue 8303043: code review 8303043: crypto/rand: better panic message for invalid Int argument.https://codereview.appspot.com/2013-04-18T06:21:19+00:00rietveld
Message from unknown
2013-04-03T06:15:18+00:00remyoudomphengurn:md5:01542520e9b590de73d4f1d0c09e8be4
Message from unknown
2013-04-03T06:15:22+00:00remyoudomphengurn:md5:b7be2960d8984d33046ab2d673a8dff2
Message from unknown
2013-04-03T06:15:37+00:00remyoudomphengurn:md5:71c21f661ac9fa9ef49eaea3f6b6791a
Message from remyoudompheng@gmail.com
2013-04-03T06:15:41+00:00remyoudomphengurn:md5:14f7c52ca2327bf03c68d27837877e58
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),
I'd like you to review this change to
https://go.googlecode.com/hg/
Message from dave@cheney.net
2013-04-03T06:18:21+00:00dfcurn:md5:92a4bfe03ac12c5b23d0c36f26d04ce5
Thank you. Leaving for the release candidate masters to decide if it makes the cut.
https://codereview.appspot.com/8303043/diff/5001/src/pkg/crypto/rand/util.go
File src/pkg/crypto/rand/util.go (right):
https://codereview.appspot.com/8303043/diff/5001/src/pkg/crypto/rand/util.go#newcode106
src/pkg/crypto/rand/util.go:106: panic("crypto/rand: argument to Int is <= 0")
Could you change math/rand at the same time to use this wording, I think it is an improvement over what exists now.
Message from bradfitz@golang.org
2013-04-03T07:50:14+00:00bradfitzurn:md5:fdd5fbbd7f47863c6d06905c24de1348
LGTM
Don't care about nil too?
On Tue, Apr 2, 2013 at 11:15 PM, <remyoudompheng@gmail.com> wrote:
> Reviewers: golang-dev1,
>
> Message:
> Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change to
> https://go.googlecode.com/hg/
>
>
> Description:
> crypto/rand: better panic message for invalid Int argument.
>
> Also document the panic to be consistent with math/rand.
>
> Fixes issue 5187.
>
> Please review this at https://codereview.appspot.**com/8303043/<https://codereview.appspot.com/8303043/>
>
> Affected files:
> M src/pkg/crypto/rand/util.go
>
>
> Index: src/pkg/crypto/rand/util.go
> ==============================**==============================**=======
> --- a/src/pkg/crypto/rand/util.go
> +++ b/src/pkg/crypto/rand/util.go
> @@ -100,8 +100,11 @@
> }
> }
>
> -// Int returns a uniform random value in [0, max).
> +// Int returns a uniform random value in [0, max). It panics if max <= 0.
> func Int(rand io.Reader, max *big.Int) (n *big.Int, err error) {
> + if max.Sign() <= 0 {
> + panic("crypto/rand: argument to Int is <= 0")
> + }
> k := (max.BitLen() + 7) / 8
>
> // b is the number of bits in the most significant byte of max.
>
>
> --
>
> ---You received this message because you are subscribed to the Google
> Groups "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegroups.com>
> .
> For more options, visit https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/opt_out>
> .
>
>
>
Message from remyoudompheng@gmail.com
2013-04-08T07:05:54+00:00remyoudomphengurn:md5:16a180cd1f9cdf4fced3419fe3997bcc
Is this okay for Go 1.1?
What about:
- the rand package?
- the nil pointer case?
Do you advise something?
Message from bradfitz@golang.org
2013-04-08T07:09:06+00:00bradfitzurn:md5:cf9d0e0f3c7ea697f75e71c6d0bfd474
Seems fine for 1.1, but I'm not concerned either way.
If it's going to crash either way, we can crash with a nice error message.
Or not.
Likewise with nil. Your call.
On Mon, Apr 8, 2013 at 12:05 AM, <remyoudompheng@gmail.com> wrote:
> Is this okay for Go 1.1?
>
> What about:
> - the rand package?
> - the nil pointer case?
>
> Do you advise something?
>
> https://codereview.appspot.**com/8303043/<https://codereview.appspot.com/8303043/>
>
Message from r@golang.org
2013-04-15T19:53:47+00:00rurn:md5:6125e43e5f025faf37ff42cfb252630b
LGTM
Message from bradfitz@golang.org
2013-04-18T06:21:19+00:00bradfitzurn:md5:775295384f1322a852c55e8eac8d4333
*** Submitted as https://code.google.com/p/go/source/detail?r=4543c39256f9 ***
crypto/rand: better panic message for invalid Int argument.
Also document the panic to be consistent with math/rand.
Fixes issue 5187.
R=golang-dev, dave, bradfitz, r
CC=golang-dev
https://codereview.appspot.com/8303043
Committer: Brad Fitzpatrick <bradfitz@golang.org>