From 643f8a2b408d5f8efeb8c84e0ffe80489b1531fb Mon Sep 17 00:00:00 2001
From: Kir Kolyshkin <kolyshkin@gmail.com>
Date: Wed, 17 Nov 2021 17:30:03 -0800
Subject: [PATCH] libct/specconv: nits

1. Decapitalize errors.
2. Rename isValidName to checkPropertyName.
3. Make it return a specific error.

Suggested-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
 libcontainer/specconv/spec_linux.go      | 22 +++++++++++-----------
 libcontainer/specconv/spec_linux_test.go |  8 ++++----
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/libcontainer/specconv/spec_linux.go b/libcontainer/specconv/spec_linux.go
index 3676654ac1c..9e39a70bede 100644
--- a/libcontainer/specconv/spec_linux.go
+++ b/libcontainer/specconv/spec_linux.go
@@ -273,7 +273,7 @@ func CreateLibcontainerConfig(opts *CreateOpts) (*configs.Config, error) {
 	}
 	spec := opts.Spec
 	if spec.Root == nil {
-		return nil, errors.New("Root must be specified")
+		return nil, errors.New("root must be specified")
 	}
 	rootfsPath := spec.Root.Path
 	if !filepath.IsAbs(rootfsPath) {
@@ -410,20 +410,20 @@ func createLibcontainerMount(cwd string, m specs.Mount) (*configs.Mount, error)
 	return mnt, nil
 }
 
-// isValidName checks if systemd property name is valid. It should consists
-// of latin letters only, and have least 3 of them.
-func isValidName(s string) bool {
+// checkPropertyName checks if systemd property name is valid. A valid name
+// should consist of latin letters only, and have least 3 of them.
+func checkPropertyName(s string) error {
 	if len(s) < 3 {
-		return false
+		return errors.New("too short")
 	}
 	// Check ASCII characters rather than Unicode runes.
 	for _, ch := range s {
 		if (ch >= 'A' && ch <= 'Z') || (ch >= 'a' && ch <= 'z') {
 			continue
 		}
-		return false
+		return errors.New("contains non-alphabetic character")
 	}
-	return true
+	return nil
 }
 
 // Some systemd properties are documented as having "Sec" suffix
@@ -465,12 +465,12 @@ func initSystemdProps(spec *specs.Spec) ([]systemdDbus.Property, error) {
 		if len(name) == len(k) { // prefix not there
 			continue
 		}
-		if !isValidName(name) {
-			return nil, fmt.Errorf("Annotation %s name incorrect: %s", k, name)
+		if err := checkPropertyName(name); err != nil {
+			return nil, fmt.Errorf("annotation %s name incorrect: %w", k, err)
 		}
 		value, err := dbus.ParseVariant(v, dbus.Signature{})
 		if err != nil {
-			return nil, fmt.Errorf("Annotation %s=%s value parse error: %w", k, v, err)
+			return nil, fmt.Errorf("annotation %s=%s value parse error: %w", k, v, err)
 		}
 		// Check for Sec suffix.
 		if trimName := strings.TrimSuffix(name, "Sec"); len(trimName) < len(name) {
@@ -480,7 +480,7 @@ func initSystemdProps(spec *specs.Spec) ([]systemdDbus.Property, error) {
 				name = trimName + "USec"
 				value, err = convertSecToUSec(value)
 				if err != nil {
-					return nil, fmt.Errorf("Annotation %s=%s value parse error: %w", k, v, err)
+					return nil, fmt.Errorf("annotation %s=%s value parse error: %w", k, v, err)
 				}
 			}
 		}
diff --git a/libcontainer/specconv/spec_linux_test.go b/libcontainer/specconv/spec_linux_test.go
index ffcac128eef..56d808699c6 100644
--- a/libcontainer/specconv/spec_linux_test.go
+++ b/libcontainer/specconv/spec_linux_test.go
@@ -780,9 +780,9 @@ func TestIsValidName(t *testing.T) {
 	}
 
 	for _, tc := range testCases {
-		valid := isValidName(tc.in)
-		if valid != tc.valid {
-			t.Errorf("case %q: expected %v, got %v", tc.in, tc.valid, valid)
+		err := checkPropertyName(tc.in)
+		if (err == nil) != tc.valid {
+			t.Errorf("case %q: expected valid: %v, got error: %v", tc.in, tc.valid, err)
 		}
 	}
 }
@@ -790,7 +790,7 @@ func TestIsValidName(t *testing.T) {
 func BenchmarkIsValidName(b *testing.B) {
 	for i := 0; i < b.N; i++ {
 		for _, s := range []string{"", "xx", "xxx", "someValidName", "A name", "Кир", "მადლობა", "合い言葉"} {
-			_ = isValidName(s)
+			_ = checkPropertyName(s)
 		}
 	}
 }