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.

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: crypto, hash: add WriteString support #14757

Closed
josharian opened this issue Mar 10, 2016 · 10 comments
Closed

proposal: crypto, hash: add WriteString support #14757

josharian opened this issue Mar 10, 2016 · 10 comments

Comments

@josharian
Copy link
Contributor

Add WriteString to crypto/sha1, crypto/sha256, hash/crc64, etc. Similar to #11805. Probably easy pickings.

@josharian
Copy link
Contributor Author

And see CL 20524 for a place this would get used.

@bradfitz bradfitz added Performance Suggested Issues that may be good for new contributors looking for work to do. labels Apr 9, 2016
@bradfitz bradfitz added this to the Unplanned milestone Apr 9, 2016
@sctb
Copy link
Contributor

sctb commented Apr 24, 2016

Initial CL submitted. I expect it's not quite there yet; glad to make any changes necessary.

@gopherbot
Copy link

CL https://golang.org/cl/22422 mentions this issue.

@minux
Copy link
Member

minux commented May 1, 2016 via email

@josharian
Copy link
Contributor Author

I think you're probably right. Before abandoning this, I'd like to do a bit of research in my Go corpus, but that might take a while.

@josharian josharian removed the Suggested Issues that may be good for new contributors looking for work to do. label Jul 21, 2017
@Scratch-net
Copy link

Having WriteString might indirectly suggest using it to hash passwords, which is a very, very bad idea.

@FiloSottile FiloSottile changed the title crypto, hash: add WriteString support proposal: crypto, hash: add WriteString support Dec 2, 2019
@FiloSottile FiloSottile modified the milestones: Unplanned, Proposal Dec 2, 2019
@rsc rsc added this to Incoming in Proposals (old) Dec 4, 2019
@rsc
Copy link
Contributor

rsc commented Dec 17, 2019

I am very skeptical of an unsafe implementation of WriteString here.
Even the one in os.File is not unsafe - it does Write([]byte(s)).
I don't believe crypto should set a precedent that "unsafe is OK if it's for performance",
for others to follow.

@rsc
Copy link
Contributor

rsc commented Jan 15, 2020

Based on the discussion above (and the general lack of interest over 4 years), this seems like a likely decline.

Leaving open for a week for final comments.

@rsc rsc moved this from Incoming to Likely Decline in Proposals (old) Jan 15, 2020
@rsc
Copy link
Contributor

rsc commented Jan 22, 2020

No change in consensus, so declined.

@rsc rsc closed this as completed Jan 22, 2020
@rsc rsc moved this from Likely Decline to Accepted in Proposals (old) Jan 22, 2020
@rsc rsc moved this from Accepted to Declined in Proposals (old) Jan 22, 2020
@rsc
Copy link
Contributor

rsc commented Aug 5, 2020

Over on #38776 we realized that WriteString can be added without requiring use of unsafe, so maybe we should do that. WriteString is now included in #38776.

@golang golang locked and limited conversation to collaborators Aug 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Development

No branches or pull requests

8 participants