Use unix.SignalNum in ParseSignal on unix platform

zhsj · zhsj · commit f0d5dd3c823b · 2019-03-03T22:10:58.000+08:00
This removes the signalMap on unix platform, since the signalMap on different architectures is not same, especially it's wrong on mipsx. golang.org/x/sys/unix now has a SignalNum func to convert signal name to a number, thus there's no need to keep this redundant map. Windows platform still needs to have a signalMap, since golang.org/x/sys/windows doesn't have corresponding functions. Address: #3061 Signed-off-by: Shengjing Zhu <zhsj@debian.org>
diff --git a/signal_map_linux.go b/signal_map_linux.go
diff --git a/signal_map_unix.go b/signal_map_unix.go
diff --git a/signals.go b/signals.go
@@ -20,13 +20,11 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"strconv"
-	"strings"
 	"syscall"
 
 	"github.com/containerd/containerd/content"
 	"github.com/containerd/containerd/images"
-	"github.com/opencontainers/image-spec/specs-go/v1"
+	v1 "github.com/opencontainers/image-spec/specs-go/v1"
 )
 
 // StopSignalLabel is a well-known containerd label for storing the stop
@@ -83,23 +81,3 @@ func GetOCIStopSignal(ctx context.Context, image Image, defaultSignal string) (s
 
 	return config.StopSignal, nil
 }
-
-// ParseSignal parses a given string into a syscall.Signal
-// it checks that the signal exists in the platform-appropriate signalMap
-func ParseSignal(rawSignal string) (syscall.Signal, error) {
-	s, err := strconv.Atoi(rawSignal)
-	if err == nil {
-		sig := syscall.Signal(s)
-		for _, msig := range signalMap {
-			if sig == msig {
-				return sig, nil
-			}
-		}
-		return -1, fmt.Errorf("unknown signal %q", rawSignal)
-	}
-	signal, ok := signalMap[strings.TrimPrefix(strings.ToUpper(rawSignal), "SIG")]
-	if !ok {
-		return -1, fmt.Errorf("unknown signal %q", rawSignal)
-	}
-	return signal, nil
-}
diff --git a/signals_test.go b/signals_test.go
@@ -0,0 +1,47 @@
+/*
+   Copyright The containerd Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package containerd
+
+import (
+	"fmt"
+	"syscall"
+	"testing"
+)
+
+func TestParseSignal(t *testing.T) {
+	testSignals := []struct {
+		raw  string
+		want syscall.Signal
+		err  bool
+	}{
+		{"1", syscall.Signal(1), false},
+		{"SIGKILL", syscall.SIGKILL, false},
+		{"NONEXIST", 0, true},
+		{"65536", 0, true},
+	}
+	for _, ts := range testSignals {
+		t.Run(fmt.Sprintf("%s/%d/%t", ts.raw, ts.want, ts.err), func(t *testing.T) {
+			got, err := ParseSignal(ts.raw)
+			if ts.err && err == nil {
+				t.Errorf("ParseSignal(%s) should return error", ts.raw)
+			}
+			if !ts.err && got != ts.want {
+				t.Errorf("ParseSignal(%s) return %d, want %d", ts.raw, got, ts.want)
+			}
+		})
+	}
+}
diff --git a/signals_unix.go b/signals_unix.go
@@ -0,0 +1,47 @@
+// +build aix darwin dragonfly freebsd linux netbsd openbsd solaris
+
+/*
+   Copyright The containerd Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package containerd
+
+import (
+	"fmt"
+	"strconv"
+	"strings"
+	"syscall"
+
+	"golang.org/x/sys/unix"
+)
+
+// ParseSignal parses a given string into a syscall.Signal
+// the rawSignal can be a string with "SIG" prefix,
+// or a signal number in string format.
+func ParseSignal(rawSignal string) (syscall.Signal, error) {
+	s, err := strconv.Atoi(rawSignal)
+	if err == nil {
+		signal := syscall.Signal(s)
+		if unix.SignalName(signal) != "" {
+			return signal, nil
+		}
+		return -1, fmt.Errorf("unknown signal %q", rawSignal)
+	}
+	signal := unix.SignalNum(strings.ToUpper(rawSignal))
+	if signal == 0 {
+		return -1, fmt.Errorf("unknown signal %q", rawSignal)
+	}
+	return signal, nil
+}
diff --git a/signals_windows.go b/signals_windows.go
@@ -17,6 +17,9 @@
 package containerd
 
 import (
+	"fmt"
+	"strconv"
+	"strings"
 	"syscall"
 
 	"golang.org/x/sys/windows"
@@ -37,3 +40,24 @@ var signalMap = map[string]syscall.Signal{
 	"ALRM":   syscall.Signal(windows.SIGALRM),
 	"TERM":   syscall.Signal(windows.SIGTERM),
 }
+
+// ParseSignal parses a given string into a syscall.Signal
+// the rawSignal can be a string with "SIG" prefix,
+// or a signal number in string format.
+func ParseSignal(rawSignal string) (syscall.Signal, error) {
+	s, err := strconv.Atoi(rawSignal)
+	if err == nil {
+		sig := syscall.Signal(s)
+		for _, msig := range signalMap {
+			if sig == msig {
+				return sig, nil
+			}
+		}
+		return -1, fmt.Errorf("unknown signal %q", rawSignal)
+	}
+	signal, ok := signalMap[strings.TrimPrefix(strings.ToUpper(rawSignal), "SIG")]
+	if !ok {
+		return -1, fmt.Errorf("unknown signal %q", rawSignal)
+	}
+	return signal, nil
+}
