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.
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
proposal: math/rand: add Reader variable #16776
Comments
I thought this was a dup, but I was thinking of #8330 which went most of the way, but not all the way. I still think there was an open bug or thread about this, but I don't remember the outcome. |
When #8330 was first implemented, I noticed that it didn't include an implementation for a Reader object. Back then, I didn't speak up because it's not a big deal. I think it'd be nice, but everytime I really wanted to use random Reader, I just do |
It would be nice, in particular to provide a concurrent safe version of |
Bumping over to proposal process. I think we should not do this. There are very few reasons to use a pseudo-random byte stream, and I worry about 'rand.Reader' completing to math/rand.Reader instead of crypto/rand.Reader. In general I wish we'd done a better job of keeping math/rand and crypto/rand's APIs completely disjoint, and in particular I regret adding math/rand.Read in the first place. I don't think we should exacerbate the problem by adding math/rand.Reader as well. |
This proposal has been added to the active column of the proposals project |
Based on the discussion above, this proposal seems like a likely decline. |
unfortunate but for whoever gets to this issue while looking for a concrete solution: here is the workaround // SyncReader is a thread-safe wrapper for a reader.
type SyncReader struct {
lk sync.Mutex
reader io.Reader
}
// NewSyncReader returns a new thread-safe reader.
func NewSyncReader(reader io.Reader) *SyncReader {
return &SyncReader{
reader: reader,
}
}
func (r *SyncReader) Read(p []byte) (n int, err error) {
r.lk.Lock()
defer r.lk.Unlock()
return r.reader.Read(p)
}
// Then you can use:
rander = NewSyncReader(rand.New(rand.NewSource(time.Now().UnixNano()))) //nolint:gosec // we want fast not crypto
uuid.Must(uuid.NewRandomFromReader(rander)).String() https://github.com/fortio/fortio/blob/v1.38.0/fhttp/http_utils.go#L603-L621 https://github.com/fortio/fortio/blob/v1.38.0/fhttp/http_client.go#L66 https://github.com/fortio/fortio/blob/v1.38.0/fhttp/http_client.go#L1075 |
No change in consensus, so declined. |
The
crypto/rand
has both a Read function and aReader
object for passing in as an io.Reader. Themath/rand
package only has a Read function and no Reader object.Should we add a Reader to
math/rand
to make it consistent withcrypto/rand
?The text was updated successfully, but these errors were encountered: