From 4878dff3e2c89382699c29c10dc5036367275575 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 5 Oct 2020 12:33:53 -0700 Subject: Remove excessive error wrapping In case os.Open[File], os.Mkdir[All], ioutil.ReadFile and the like fails, the error message already contains the file name and the operation that fails, so there is no need to wrap the error with something like "open %s failed". While at it - replace a few places with os.Open, ioutil.ReadAll with ioutil.ReadFile. - replace errors.Wrapf with errors.Wrap for cases where there are no %-style arguments. Signed-off-by: Kir Kolyshkin --- pkg/cgroups/cgroups.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'pkg/cgroups/cgroups.go') diff --git a/pkg/cgroups/cgroups.go b/pkg/cgroups/cgroups.go index f23787bd4..3cf736bdc 100644 --- a/pkg/cgroups/cgroups.go +++ b/pkg/cgroups/cgroups.go @@ -131,7 +131,7 @@ func getAvailableControllers(exclude map[string]controllerHandler, cgroup2 bool) infos, err := ioutil.ReadDir(cgroupRoot) if err != nil { - return nil, errors.Wrapf(err, "read directory %s", cgroupRoot) + return nil, err } controllers := []controller{} for _, i := range infos { @@ -157,7 +157,7 @@ func (c *CgroupControl) getCgroupv1Path(name string) string { func createCgroupv2Path(path string) (deferredError error) { content, err := ioutil.ReadFile("/sys/fs/cgroup/cgroup.controllers") if err != nil { - return errors.Wrapf(err, "read /sys/fs/cgroup/cgroup.controllers") + return err } if !strings.HasPrefix(path, "/sys/fs/cgroup/") { return fmt.Errorf("invalid cgroup path %s", path) @@ -180,7 +180,7 @@ func createCgroupv2Path(path string) (deferredError error) { if i > 0 { if err := os.Mkdir(current, 0755); err != nil { if !os.IsExist(err) { - return errors.Wrapf(err, "mkdir %s", path) + return err } } else { // If the directory was created, be sure it is not left around on errors. @@ -237,7 +237,7 @@ func (c *CgroupControl) initialize() (err error) { } path := c.getCgroupv1Path(ctr.name) if err := os.MkdirAll(path, 0755); err != nil { - return errors.Wrapf(err, "error creating cgroup path %s for %s", path, ctr.name) + return errors.Wrapf(err, "error creating cgroup path for %s", ctr.name) } } } @@ -265,7 +265,7 @@ func (c *CgroupControl) createCgroupDirectory(controller string) (bool, error) { func readFileAsUint64(path string) (uint64, error) { data, err := ioutil.ReadFile(path) if err != nil { - return 0, errors.Wrapf(err, "open %s", path) + return 0, err } v := cleanString(string(data)) if v == "max" { @@ -425,7 +425,7 @@ func rmDirRecursively(path string) error { } entries, err := ioutil.ReadDir(path) if err != nil { - return errors.Wrapf(err, "read %s", path) + return err } for _, i := range entries { if i.IsDir() { -- cgit v1.2.3-54-g00ecf From d4aa89bb40b3a2c1730c9bff31a681007a3feb97 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 5 Oct 2020 12:41:28 -0700 Subject: pkg/cgroups/createCgroupv2Path: nits 1. Check the path validity before trying to read the cgroup.controllers. 2. Do not hardcode "/sys/fs/cgroup". 3. Simplify creating the "+this +that" string. 4. Do not wrap ioutil.WriteFile error. Signed-off-by: Kir Kolyshkin --- pkg/cgroups/cgroups.go | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) (limited to 'pkg/cgroups/cgroups.go') diff --git a/pkg/cgroups/cgroups.go b/pkg/cgroups/cgroups.go index 3cf736bdc..0d7ed05b2 100644 --- a/pkg/cgroups/cgroups.go +++ b/pkg/cgroups/cgroups.go @@ -2,6 +2,7 @@ package cgroups import ( "bufio" + "bytes" "fmt" "io/ioutil" "math" @@ -155,23 +156,15 @@ func (c *CgroupControl) getCgroupv1Path(name string) string { // createCgroupv2Path creates the cgroupv2 path and enables all the available controllers func createCgroupv2Path(path string) (deferredError error) { - content, err := ioutil.ReadFile("/sys/fs/cgroup/cgroup.controllers") - if err != nil { - return err - } - if !strings.HasPrefix(path, "/sys/fs/cgroup/") { + if !strings.HasPrefix(path, cgroupRoot+"/") { return fmt.Errorf("invalid cgroup path %s", path) } - - res := "" - for i, c := range strings.Split(strings.TrimSpace(string(content)), " ") { - if i == 0 { - res = fmt.Sprintf("+%s", c) - } else { - res += fmt.Sprintf(" +%s", c) - } + content, err := ioutil.ReadFile(cgroupRoot + "/cgroup.controllers") + if err != nil { + return err } - resByte := []byte(res) + ctrs := bytes.Fields(content) + res := append([]byte("+"), bytes.Join(ctrs, []byte(" +"))...) current := "/sys/fs" elements := strings.Split(path, "/") @@ -194,8 +187,8 @@ func createCgroupv2Path(path string) (deferredError error) { // We enable the controllers for all the path components except the last one. It is not allowed to add // PIDs if there are already enabled controllers. if i < len(elements[3:])-1 { - if err := ioutil.WriteFile(filepath.Join(current, "cgroup.subtree_control"), resByte, 0755); err != nil { - return errors.Wrapf(err, "write %s", filepath.Join(current, "cgroup.subtree_control")) + if err := ioutil.WriteFile(filepath.Join(current, "cgroup.subtree_control"), res, 0755); err != nil { + return err } } } -- cgit v1.2.3-54-g00ecf