Remove extraneous allocations. #3
Conversation
|
Thanks for the informative PR! I do request that you sign the CLA if you'd like this merged. If not, would it be ok if I piggy-backed on this code and applied it to the other package as well? |
|
I signed the individual CLA, though I didn't (and won't) provide a cell phone number or address. Let me know if you have any questions though I would be glad to help. |
|
Are you merging this or should I close? |
|
@cstockton unfortunately our open source office rules require phone number and address based on what @jaredsmith told me :( Unless something's changed on that end I will have to decline this PR |
|
@jaredsmith What is your privacy policy around this personal information? It's a google doc which makes it very unclear who has access to these details. While sometimes I come across a CLA that may ask for my address, It's unusual to require a personal phone number in a CLA, under what scenario would CapitalOne even need my address- let alone call me? |
|
That's a great question -- we'll only contact you related to your open source contributions -- we don't use the data for any other reasons. Our legal team just likes having an address and phone number if there's a legal question about a particular contribution. The only people who have access to the data are the project leaders (so that they can see if someone has signed a CLA or not) and our Open Source Office. Nobody else has the data. If there's any other questions I can answer regarding the CLAs or our open source office, please don't hesitate to ask. |
|
@jaredsmith I'll sign it after work as long as it will not be shared with third parties, used for marketing, advertising or recruitment purposes. That is: Only used to contact me in the event a legal dispute or question on code I've contributed. Though I would appreciate being tagged here first. Maybe a little clarity on this on the document would be helpful. Thanks! |
|
@anitgandhi This was signed. |
|
Thanks @cstockton . Have one change to request, see the review comment |
ff3/ff3.go
Outdated
| @@ -215,6 +215,8 @@ func (f Cipher) Encrypt(X string) (string, error) { | |||
| return ret, nil | |||
| } | |||
|
|
|||
| var zeros = make([]byte, 16) | |||
There was a problem hiding this comment.
Can you re-use the ivZero for this? Seems like it would accomplish the same thing since you're taking a slice of it, but right now zeros feels redundant
There was a problem hiding this comment.
I updated this it was redundant, thanks.
|
@anitgandhi Updated pull request thanks for the patience while I figured out the CLA stuff. |
|
No worries, glad the CLA stuff was sorted out @cstockton 👍 |
What's in this PR?
I removed extraneous allocations to illustrate the techniques for use in other areas of the library if the author would like.
TODOs
I saw the CLA after I made the changes, my mistake. I'll look into it tomorrow evening maybe, depending how much information I need to disclose.