Ignore invalid key files in keystore directory. #4700
Conversation
matrushka
force-pushed
the
ignore_invalid_key_files
branch
4 times, most recently
from
407ec27
to
3d097c5
Compare
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
|
|
keystore/keystore_test.go
Outdated
| @@ -146,6 +147,7 @@ func TestKeystoreBasics(t *testing.T) { | |||
|
|
|||
| func TestInvalidKeyFiles(t *testing.T) { | |||
| tdir, err := ioutil.TempDir("", "keystore-test") | |||
| defer os.RemoveAll(tdir) | |||
There was a problem hiding this comment.
should be after the error check.
matrushka
force-pushed
the
ignore_invalid_key_files
branch
2 times, most recently
from
326f1c3
to
fbe955d
Compare
|
@magik6k by the way the formatting error was in a file I haven't touched at all. Also running |
|
Oh, for some reason un-fmt'ed code got merged to master, pushed a fix in #4701 |
keystore/keystore.go
Outdated
| return nil, err | ||
| } | ||
|
|
||
| var list []string |
There was a problem hiding this comment.
Probably best to preallocate list := make([]string, 0, len(dirs)).
There was a problem hiding this comment.
I'd leave out the capacity param since we don't know if all the dirs are gonna make it to the list.
There was a problem hiding this comment.
Over allocating slightly is better than under allocating, in general (especially when we only over allocate in an error case). Probably not that much of an issue here as this function isn't frequently called but I know it'll continue to bug me.
keystore/keystore.go
Outdated
| } | ||
| } | ||
|
|
||
| return list, err |
There was a problem hiding this comment.
Returning the error here is a bit confusing. It should always be nil (the go idiom is to just return list, nil).
keystore/keystore.go
Outdated
|
|
||
| for _, name := range dirs { | ||
| err := validateName(name) | ||
| if err == nil { |
There was a problem hiding this comment.
Should probably log the error. You'll need to import the logger and construct it at the top of the file:
import (
// ...
logging "gx/ipfs/QmRb5jh8z2E8hMGN2tkvs1yHynUanqnZ3UeKwgN1i9P1F8/go-log"
)
var log = logging.Logger("keystore")
// ...
|
LGTM. Thanks! Mind rebasing to make the tests pass? |
…ectory. * Has calls the validateName function before checking if we have the file * List filters the returned list of file names by validateName. License: MIT Signed-off-by: matrushka <[email protected]>
License: MIT Signed-off-by: matrushka <[email protected]>
…ments. License: MIT Signed-off-by: matrushka <[email protected]>
License: MIT Signed-off-by: matrushka <[email protected]>
matrushka
force-pushed
the
ignore_invalid_key_files
branch
from
d1d1586
to
e338cdf
Compare
Modified keystore to ignore invalid key files inside the keystore directory to address #4681