libs/cmn: Remove Tempfile, Tempdir, switch to ioutil variants (#2114)

Our Tempfile was just a wrapper on ioutil that panicked instead of error.

Our Tempdir was a less safe variant of ioutil's Tempdir.

CHANGELOG_PENDING.md

@@ -22,6 +22,6 @@ IMPROVEMENTS:
 - [common] bit array functions which take in another parameter are now thread safe
 
 BUG FIXES:
-- [common] \#2109 Safely handle cases where atomic write files already exist
+- [common] Safely handle cases where atomic write files already exist [#2109](https://github.com/tendermint/tendermint/issues/2109)
 - [privval] fix a deadline for accepting new connections in socket private
   validator.

consensus/common_test.go

@@ -378,8 +378,11 @@ func randConsensusNetWithPeers(nValidators, nPeers int, testName string, tickerF
 		if i < nValidators {
 			privVal = privVals[i]
 		} else {
-			_, tempFilePath := cmn.Tempfile("priv_validator_")
+			tempFile, err := ioutil.TempFile("", "priv_validator_")
-			privVal = privval.GenFilePV(tempFilePath)
+			if err != nil {
+				panic(err)
+			}
+			privVal = privval.GenFilePV(tempFile.Name())
 		}
 
 		app := appFunc()

libs/autofile/autofile_test.go

@@ -1,6 +1,7 @@
 package autofile
 
 import (
+	"io/ioutil"
 	"os"
 	"sync/atomic"
 	"syscall"
@@ -13,10 +14,14 @@ import (
 func TestSIGHUP(t *testing.T) {
 
 	// First, create an AutoFile writing to a tempfile dir
-	file, name := cmn.Tempfile("sighup_test")
+	file, err := ioutil.TempFile("", "sighup_test")
-	if err := file.Close(); err != nil {
+	if err != nil {
 		t.Fatalf("Error creating tempfile: %v", err)
 	}
+	if err := file.Close(); err != nil {
+		t.Fatalf("Error closing tempfile: %v", err)
+	}
+	name := file.Name()
 	// Here is the actual AutoFile
 	af, err := OpenAutoFile(name)
 	if err != nil {

libs/common/tempfile.go

@@ -3,7 +3,6 @@ package common
 import (
 	fmt "fmt"
 	"io"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"strconv"
@@ -127,26 +126,3 @@ func WriteFileAtomic(filename string, data []byte, perm os.FileMode) (err error)
 
 	return os.Rename(f.Name(), filename)
 }
-
-//--------------------------------------------------------------------------------
-
-func Tempfile(prefix string) (*os.File, string) {
-	file, err := ioutil.TempFile("", prefix)
-	if err != nil {
-		PanicCrisis(err)
-	}
-	return file, file.Name()
-}
-
-func Tempdir(prefix string) (*os.File, string) {
-	tempDir := os.TempDir() + "/" + prefix + RandStr(12)
-	err := EnsureDir(tempDir, 0700)
-	if err != nil {
-		panic(Fmt("Error creating temp dir: %v", err))
-	}
-	dir, err := os.Open(tempDir)
-	if err != nil {
-		panic(Fmt("Error opening temp dir: %v", err))
-	}
-	return dir, tempDir
-}

libs/db/backend_test.go

@@ -2,6 +2,7 @@ package db
 
 import (
 	"fmt"
+	"io/ioutil"
 	"os"
 	"path/filepath"
 	"testing"
@@ -17,8 +18,8 @@ func cleanupDBDir(dir, name string) {
 
 func testBackendGetSetDelete(t *testing.T, backend DBBackendType) {
 	// Default
-	dir, dirname := cmn.Tempdir(fmt.Sprintf("test_backend_%s_", backend))
+	dirname, err := ioutil.TempDir("", fmt.Sprintf("test_backend_%s_", backend))
-	defer dir.Close()
+	require.Nil(t, err)
 	db := NewDB("testdb", backend, dirname)
 
 	// A nonexistent key should return nil, even if the key is empty

libs/db/common_test.go

@@ -2,12 +2,12 @@ package db
 
 import (
 	"fmt"
+	"io/ioutil"
 	"sync"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	cmn "github.com/tendermint/tendermint/libs/common"
 )
 
 //----------------------------------------
@@ -61,9 +61,9 @@ func checkValuePanics(t *testing.T, itr Iterator) {
 }
 
 func newTempDB(t *testing.T, backend DBBackendType) (db DB) {
-	dir, dirname := cmn.Tempdir("db_common_test")
+	dirname, err := ioutil.TempDir("", "db_common_test")
+	require.Nil(t, err)
 	db = NewDB("testdb", backend, dirname)
-	dir.Close()
 	return db
 }
 

privval/priv_validator_test.go

@@ -3,6 +3,7 @@ package privval
 import (
 	"encoding/base64"
 	"fmt"
+	"io/ioutil"
 	"os"
 	"testing"
 	"time"
@@ -11,22 +12,22 @@ import (
 	"github.com/stretchr/testify/require"
 	"github.com/tendermint/tendermint/crypto"
 	"github.com/tendermint/tendermint/crypto/ed25519"
-	cmn "github.com/tendermint/tendermint/libs/common"
 	"github.com/tendermint/tendermint/types"
 )
 
 func TestGenLoadValidator(t *testing.T) {
 	assert := assert.New(t)
 
-	_, tempFilePath := cmn.Tempfile("priv_validator_")
+	tempFile, err := ioutil.TempFile("", "priv_validator_")
-	privVal := GenFilePV(tempFilePath)
+	require.Nil(t, err)
+	privVal := GenFilePV(tempFile.Name())
 
 	height := int64(100)
 	privVal.LastHeight = height
 	privVal.Save()
 	addr := privVal.GetAddress()
 
-	privVal = LoadFilePV(tempFilePath)
+	privVal = LoadFilePV(tempFile.Name())
 	assert.Equal(addr, privVal.GetAddress(), "expected privval addr to be the same")
 	assert.Equal(height, privVal.LastHeight, "expected privval.LastHeight to have been saved")
 }
@@ -34,7 +35,9 @@ func TestGenLoadValidator(t *testing.T) {
 func TestLoadOrGenValidator(t *testing.T) {
 	assert := assert.New(t)
 
-	_, tempFilePath := cmn.Tempfile("priv_validator_")
+	tempFile, err := ioutil.TempFile("", "priv_validator_")
+	require.Nil(t, err)
+	tempFilePath := tempFile.Name()
 	if err := os.Remove(tempFilePath); err != nil {
 		t.Error(err)
 	}
@@ -91,8 +94,9 @@ func TestUnmarshalValidator(t *testing.T) {
 func TestSignVote(t *testing.T) {
 	assert := assert.New(t)
 
-	_, tempFilePath := cmn.Tempfile("priv_validator_")
+	tempFile, err := ioutil.TempFile("", "priv_validator_")
-	privVal := GenFilePV(tempFilePath)
+	require.Nil(t, err)
+	privVal := GenFilePV(tempFile.Name())
 
 	block1 := types.BlockID{[]byte{1, 2, 3}, types.PartSetHeader{}}
 	block2 := types.BlockID{[]byte{3, 2, 1}, types.PartSetHeader{}}
@@ -101,7 +105,7 @@ func TestSignVote(t *testing.T) {
 
 	// sign a vote for first time
 	vote := newVote(privVal.Address, 0, height, round, voteType, block1)
-	err := privVal.SignVote("mychainid", vote)
+	err = privVal.SignVote("mychainid", vote)
 	assert.NoError(err, "expected no error signing vote")
 
 	// try to sign the same vote again; should be fine
@@ -132,8 +136,9 @@ func TestSignVote(t *testing.T) {
 func TestSignProposal(t *testing.T) {
 	assert := assert.New(t)
 
-	_, tempFilePath := cmn.Tempfile("priv_validator_")
+	tempFile, err := ioutil.TempFile("", "priv_validator_")
-	privVal := GenFilePV(tempFilePath)
+	require.Nil(t, err)
+	privVal := GenFilePV(tempFile.Name())
 
 	block1 := types.PartSetHeader{5, []byte{1, 2, 3}}
 	block2 := types.PartSetHeader{10, []byte{3, 2, 1}}
@@ -141,7 +146,7 @@ func TestSignProposal(t *testing.T) {
 
 	// sign a proposal for first time
 	proposal := newProposal(height, round, block1)
-	err := privVal.SignProposal("mychainid", proposal)
+	err = privVal.SignProposal("mychainid", proposal)
 	assert.NoError(err, "expected no error signing proposal")
 
 	// try to sign the same proposal again; should be fine
@@ -170,8 +175,9 @@ func TestSignProposal(t *testing.T) {
 }
 
 func TestDifferByTimestamp(t *testing.T) {
-	_, tempFilePath := cmn.Tempfile("priv_validator_")
+	tempFile, err := ioutil.TempFile("", "priv_validator_")
-	privVal := GenFilePV(tempFilePath)
+	require.Nil(t, err)
+	privVal := GenFilePV(tempFile.Name())
 
 	block1 := types.PartSetHeader{5, []byte{1, 2, 3}}
 	height, round := int64(10), 1