Skip to content

Conversation

Vindaar
Copy link
Contributor

@Vindaar Vindaar commented Feb 26, 2018

Moves the readPasswordFromStdin proc from the rdstdin to the terminal module. This way the proc is not dependent on linenoise anymore, which it does not use anyways.

I had to convert the password variable (TaintedString argument of the proc) manually to a string, before setLen is called. Otherwise the call fails in taint mode.

Moves the `readPasswordFrom` from the rdstdin to the terminal
module. This gets rid of the linenoise dependency, which is not
required for the proc.
Sets "password: " as a sensible default for the prompt in the
`readPasswordFromStdin` proc
@Vindaar Vindaar force-pushed the pr/moveReadPassword branch from 5f9ae03 to 55b6049 Compare February 26, 2018 22:08
@Araq
Copy link
Member

Araq commented Feb 26, 2018

The commit "replace password TaintedString with temp pwordstr string" is not good. Use s.string to treat the tainted string as a real string. No need for temporary variables.

@Vindaar
Copy link
Contributor Author

Vindaar commented Feb 26, 2018

Ok, it's what I did at first, but I felt that replacing all occurrences became a little to verbose. I'll undo it. :)

@Araq
Copy link
Member

Araq commented Feb 26, 2018

Well you can introduce a helper template here.

In order to comply with taint mode, we need to convert the password
variable in `readPasswordFromStdin` to a string, before we can set its length.
@Vindaar Vindaar force-pushed the pr/moveReadPassword branch from 06ac793 to 913db49 Compare February 26, 2018 23:16
@Vindaar
Copy link
Contributor Author

Vindaar commented Feb 26, 2018

You mean like an inline template such as:

template pword: untyped = password.string

?

@Araq
Copy link
Member

Araq commented Feb 27, 2018

yes

@Araq
Copy link
Member

Araq commented Feb 27, 2018

Unrelated test failures, merging anyway.

@Araq Araq merged commit a897371 into nim-lang:devel Feb 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants