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.

Sign up for GitHub

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

Jump to bottom

Replace deprecated io/ioutil functions #1452

Merged
merged 1 commit into from
Dec 20, 2023
Merged

Replace deprecated io/ioutil functions #1452

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions pkg/climain/mlrcli_shebang.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package climain

import (
"fmt"
"io/ioutil"
"os"
"regexp"
"strings"

Expand Down Expand Up @@ -49,7 +49,7 @@ func maybeInterpolateDashS(args []string) ([]string, error) {
remainingArgs := args[3:]

// Read the bytes in the filename given after -s.
byteContents, rerr := ioutil.ReadFile(filename)
byteContents, rerr := os.ReadFile(filename)
if rerr != nil {
return nil, fmt.Errorf("mlr: cannot read %s: %v", filename, rerr)
}
Expand All @@ -68,7 +68,7 @@ func maybeInterpolateDashS(args []string) ([]string, error) {

if stripComments {
re := regexp.MustCompile(`#.*`)
for i, _ := range lines {
for i := range lines {
lines[i] = re.ReplaceAllString(lines[i], "")
}
}
Expand Down
3 changes: 1 addition & 2 deletions pkg/entrypoint/entrypoint.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ package entrypoint

import (
"fmt"
"io/ioutil"
"os"
"path"

Expand Down Expand Up @@ -135,7 +134,7 @@ func processInPlace(
containingDirectory := path.Dir(fileName)
// Names like ./mlr-in-place-2148227797 and ./mlr-in-place-1792078347,
// as revealed by printing handle.Name().
handle, err := ioutil.TempFile(containingDirectory, "mlr-in-place-")
handle, err := os.CreateTemp(containingDirectory, "mlr-in-place-")
if err != nil {
return err
}
Expand Down
17 changes: 10 additions & 7 deletions pkg/lib/readfiles.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
package lib

import (
"io/ioutil"
"os"
"strings"

Expand Down Expand Up @@ -34,10 +33,10 @@ func LoadStringsFromFileOrDir(path string, extension string) ([]string, error) {
}
}

// LoadStringFromFile is just a wrapper around ioutil.ReadFile,
// LoadStringFromFile is just a wrapper around os.ReadFile,
// with a cast from []byte to string.
func LoadStringFromFile(filename string) (string, error) {
data, err := ioutil.ReadFile(filename)
data, err := os.ReadFile(filename)
if err != nil {
return "", err
}
Expand All @@ -51,14 +50,18 @@ func LoadStringFromFile(filename string) (string, error) {
func LoadStringsFromDir(dirname string, extension string) ([]string, error) {
dslStrings := make([]string, 0)

entries, err := ioutil.ReadDir(dirname)
f, err := os.Open(dirname)
if err != nil {
return nil, err
}
defer f.Close()

for i := range entries {
entry := &entries[i]
name := (*entry).Name()
names, err := f.Readdirnames(-1)
if err != nil {
return nil, err
}

for _, name := range names {
Comment on lines -54 to +64
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I replaced ioutil.ReadDir with os.Open 1 + Readdirnames 2 because we are only making use of the file names here.

I wrote a small benchmark, and the results show that it is slightly faster and more efficient to use Readdirnames.

// Before running the benchmark, create 1000 .txt files in /tmp
// for i in {1..1000}; do touch "/tmp/file_$i.txt"; done

func IoutilReadDir(dirname string, extension string) ([]string, error) {
	dslStrings := make([]string, 0)

	entries, err := ioutil.ReadDir(dirname)
	if err != nil {
		return nil, err
	}

	for i := range entries {
		entry := &entries[i]
		name := (*entry).Name()
		if !strings.HasSuffix(name, extension) {
			continue
		}

		path := dirname + "/" + name
		dslString, err := LoadStringFromFile(path)
		if err != nil {
			return nil, err
		}

		dslStrings = append(dslStrings, dslString)
	}

	return dslStrings, nil
}

func OsReadDir(dirname string, extension string) ([]string, error) {
	dslStrings := make([]string, 0)

	entries, err := os.ReadDir(dirname)
	if err != nil {
		return nil, err
	}

	for i := range entries {
		entry := &entries[i]
		name := (*entry).Name()
		if !strings.HasSuffix(name, extension) {
			continue
		}

		path := dirname + "/" + name
		dslString, err := LoadStringFromFile(path)
		if err != nil {
			return nil, err
		}

		dslStrings = append(dslStrings, dslString)
	}

	return dslStrings, nil
}

func OsReaddirnames(dirname string, extension string) ([]string, error) {
	f, err := os.Open(dirname)
	if err != nil {
		return nil, err
	}

	entries, err := f.Readdirnames(-1)
	if err != nil {
		return nil, err
	}

	dslStrings := make([]string, 0, len(entries))

	for _, name := range entries {
		if !strings.HasSuffix(name, extension) {
			continue
		}

		path := dirname + "/" + name
		dslString, err := LoadStringFromFile(path)
		if err != nil {
			return nil, err
		}

		dslStrings = append(dslStrings, dslString)
	}

	return dslStrings, nil
}

func BenchmarkIoutilReadDir(b *testing.B) {
	for i := 0; i < b.N; i++ {
		_, err := IoutilReadDir("/tmp", ".txt")
		if err != nil {
			b.Fatal(err)
		}
	}
}

func BenchmarkOsReadDir(b *testing.B) {
	for i := 0; i < b.N; i++ {
		_, err := OsReadDir("/tmp", ".txt")
		if err != nil {
			b.Fatal(err)
		}
	}
}

func BenchmarkOsReaddirnames(b *testing.B) {
	for i := 0; i < b.N; i++ {
		_, err := OsReaddirnames("/tmp", ".txt")
		if err != nil {
			b.Fatal(err)
		}
	}
}
goos: linux
goarch: amd64
pkg: github.com/johnkerl/miller/pkg/lib
cpu: AMD Ryzen 7 PRO 4750U with Radeon Graphics
BenchmarkIoutilReadDir-16     	     126	   9289861 ns/op	 1237645 B/op	    9122 allocs/op
BenchmarkOsReadDir-16         	     158	   7633255 ns/op	 1056843 B/op	    8091 allocs/op
BenchmarkOsReaddirnames-16    	     218	   5493111 ns/op	  959764 B/op	    7048 allocs/op
PASS
ok  	github.com/johnkerl/miller/pkg/lib	5.852s

Footnotes

  1. https://pkg.go.dev/os#Open ↩

  2. https://pkg.go.dev/os#File.Readdirnames ↩

if !strings.HasSuffix(name, extension) {
continue
}
Expand Down
5 changes: 2 additions & 3 deletions pkg/lib/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package lib

import (
"fmt"
"io/ioutil"
"os"
"sort"
"strconv"
Expand Down Expand Up @@ -186,9 +185,9 @@ func GetArrayKeysSorted(input map[string]string) []string {
// WriteTempFile places the contents string into a temp file, which the caller
// must remove.
func WriteTempFileOrDie(contents string) string {
// Use "" as first argument to ioutil.TempFile to use default directory.
// Use "" as first argument to os.CreateTemp to use default directory.
// Nominally "/tmp" or somesuch on all unix-like systems, but not for Windows.
handle, err := ioutil.TempFile("", "mlr-temp")
handle, err := os.CreateTemp("", "mlr-temp")
if err != nil {
fmt.Printf("mlr: could not create temp file.\n")
os.Exit(1)
Expand Down
69 changes: 32 additions & 37 deletions pkg/terminals/regtest/regtester.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ package regtest
import (
"container/list"
"fmt"
"io/ioutil"
"os"
"path/filepath"
"runtime"
Expand Down Expand Up @@ -153,7 +152,6 @@ func (regtester *RegTester) resetCounts() {
func (regtester *RegTester) Execute(
casePaths []string,
) bool {

// Don't let the current user's settings affect expected results
for _, name := range envVarsToUnset {
os.Unsetenv(name)
Expand Down Expand Up @@ -279,42 +277,34 @@ func (regtester *RegTester) executeSingleDirectory(
) (bool, bool) {
passed := true
// TODO: comment
hasCaseSubdirectories := regtester.hasCaseSubdirectories(dirName)
fileNames, hasCaseSubdirectories := regtester.hasCaseSubdirectories(dirName)

if !regtester.plainMode {
if hasCaseSubdirectories && regtester.verbosityLevel >= 2 {
fmt.Printf("%s BEGIN %s/\n", MajorSeparator, dirName)
}
}

entries, err := ioutil.ReadDir(dirName)
if err != nil {
fmt.Printf("%s: %v\n", dirName, err)
passed = false
} else {

for i := range entries {
entry := &entries[i]
path := dirName + "/" + (*entry).Name()
for _, name := range fileNames {
path := dirName + "/" + name

ok := regtester.executeSinglePath(path)
if !ok {
passed = false
}
ok := regtester.executeSinglePath(path)
if !ok {
passed = false
}
}

// Only print if there are .cmd files directly in this directory.
// Otherwise it's just a directory-of-directories and we don't need to
// multiply announce.
if hasCaseSubdirectories {
if passed {
if !regtester.plainMode {
fmt.Printf("%s %s\n", colorizer.MaybeColorizePass("PASS", true), dirName)
}
} else {
if !regtester.plainMode {
fmt.Printf("%s %s\n", colorizer.MaybeColorizeFail("FAIL", true), dirName)
}
// Only print if there are .cmd files directly in this directory.
// Otherwise it's just a directory-of-directories and we don't need to
// multiply announce.
if hasCaseSubdirectories {
if passed {
if !regtester.plainMode {
fmt.Printf("%s %s\n", colorizer.MaybeColorizePass("PASS", true), dirName)
}
} else {
if !regtester.plainMode {
fmt.Printf("%s %s\n", colorizer.MaybeColorizeFail("FAIL", true), dirName)
}
}
}
Expand All @@ -340,22 +330,27 @@ func (regtester *RegTester) executeSingleDirectory(

func (regtester *RegTester) hasCaseSubdirectories(
dirName string,
) bool {
) ([]string, bool) {
f, err := os.Open(dirName)
if err != nil {
fmt.Printf("%s: %v\n", dirName, err)
os.Exit(1)
}
defer f.Close()

entries, err := ioutil.ReadDir(dirName)
names, err := f.Readdirnames(-1)
if err != nil {
fmt.Printf("%s: %v\n", dirName, err)
os.Exit(1)
}

for i := range entries {
entry := &entries[i]
path := dirName + string(filepath.Separator) + (*entry).Name()
for _, name := range names {
path := dirName + string(filepath.Separator) + name
if regtester.isCaseDirectory(path) {
return true
return names, true
}
}
return false
return names, false
}

func (regtester *RegTester) isCaseDirectory(
Expand Down Expand Up @@ -774,7 +769,7 @@ func (regtester *RegTester) loadFile(
fileName string,
caseDir string,
) (string, error) {
byteContents, err := ioutil.ReadFile(fileName)
byteContents, err := os.ReadFile(fileName)
if err != nil {
return "", err
}
Expand All @@ -789,7 +784,7 @@ func (regtester *RegTester) storeFile(
fileName string,
contents string,
) error {
err := ioutil.WriteFile(fileName, []byte(contents), 0666)
err := os.WriteFile(fileName, []byte(contents), 0o666)
if err != nil {
return err
}
Expand Down
Loading