From d6e0932b9668488dcc01da288da88a52c25e7b98 Mon Sep 17 00:00:00 2001
From: Ivan Andreev <ivandeex@gmail.com>
Date: Fri, 11 Jun 2021 03:35:10 +0300
Subject: [PATCH] Remove CGO bits from process_openbsd.go (also v3)

Prior to this commit CGO was used in OpenBSD implementation of
Process.CmdlineSliceWithContext() for parsing the "kern.proc.args"
sysctl output. It requires some pointer arithmetics and raw pointer
dereferencing.

Having CGO in the "process" module prevents it from being go vet'ted
on any platform other than OpenBSD. In order to overcome this
limitation, the sysctl output parsing was reimplemented without raw
pointer deferencing. The resulting code might be slightly slower
than the original one, but it is cleaner and safer.

Since this fix allows go vet with GOOS=openbsd to run without any
issues on all platforms, openbsd entries were also added to the "vet"
Makefile target.

Co-authored-by: Sergey Vinogradov <cbrpnk@gmail.com>
---
 Makefile                      |  4 ++++
 process/process_openbsd.go    | 53 +++++++++++++++++++++++++++++++++++--------
 v3/process/process_openbsd.go | 53 +++++++++++++++++++++++++++++++++++--------
 3 files changed, 90 insertions(+), 20 deletions(-)

diff --git a/Makefile b/Makefile
index 7a540a9..39f199e 100644
--- a/Makefile
+++ b/Makefile
@@ -58,6 +58,10 @@ vet:
 	GOOS=linux GOARCH=s390x go vet ./...
 
 	GOOS=netbsd GOARCH=amd64 go vet ./...
+
+	GOOS=openbsd GOARCH=386 go vet ./...
+	GOOS=openbsd GOARCH=amd64 go vet ./...
+
 	GOOS=solaris GOARCH=amd64 go vet ./...
 
 	GOOS=windows GOARCH=amd64 go vet ./...
diff --git a/process/process_openbsd.go b/process/process_openbsd.go
index 0404b4b..902664b 100644
--- a/process/process_openbsd.go
+++ b/process/process_openbsd.go
@@ -3,8 +3,11 @@
 package process
 
 import (
-	"C"
+	"bytes"
 	"context"
+	"encoding/binary"
+	"fmt"
+	"io"
 	"os/exec"
 	"path/filepath"
 	"strconv"
@@ -78,19 +81,49 @@ func (p *Process) CmdlineSliceWithContext(ctx context.Context) ([]string, error)
 		return nil, err
 	}
 
-	argc := 0
-	argvp := unsafe.Pointer(&buf[0])
-	argv := *(**C.char)(unsafe.Pointer(argvp))
-	size := unsafe.Sizeof(argv)
+	/* From man sysctl(2):
+	The buffer pointed to by oldp is filled with an array of char
+	pointers followed by the strings themselves. The last char
+	pointer is a NULL pointer. */
 	var strParts []string
+	r := bytes.NewReader(buf)
+	baseAddr := uintptr(unsafe.Pointer(&buf[0]))
+	for {
+		argvp, err := readPtr(r)
+		if err != nil {
+			return nil, err
+		}
+		if argvp == 0 { // check for a NULL pointer
+			break
+		}
+		offset := argvp - baseAddr
+		length := uintptr(bytes.IndexByte(buf[offset:], 0))
+		str := string(buf[offset : offset+length])
+		strParts = append(strParts, str)
+	}
 
-	for argv != nil {
-		strParts = append(strParts, C.GoString(argv))
+	return strParts, nil
+}
 
-		argc++
-		argv = *(**C.char)(unsafe.Pointer(uintptr(argvp) + uintptr(argc)*size))
+// readPtr reads a pointer data from a given reader. WARNING: only little
+// endian architectures are supported.
+func readPtr(r io.Reader) (uintptr, error) {
+	switch sizeofPtr {
+	case 4:
+		var p uint32
+		if err := binary.Read(r, binary.LittleEndian, &p); err != nil {
+			return 0, err
+		}
+		return uintptr(p), nil
+	case 8:
+		var p uint64
+		if err := binary.Read(r, binary.LittleEndian, &p); err != nil {
+			return 0, err
+		}
+		return uintptr(p), nil
+	default:
+		return 0, fmt.Errorf("unsupported pointer size")
 	}
-	return strParts, nil
 }
 
 func (p *Process) CmdlineWithContext(ctx context.Context) (string, error) {
diff --git a/v3/process/process_openbsd.go b/v3/process/process_openbsd.go
index 7325f09..0977a11 100644
--- a/v3/process/process_openbsd.go
+++ b/v3/process/process_openbsd.go
@@ -3,8 +3,11 @@
 package process
 
 import (
-	"C"
+	"bytes"
 	"context"
+	"encoding/binary"
+	"fmt"
+	"io"
 	"os/exec"
 	"path/filepath"
 	"strconv"
@@ -78,19 +81,49 @@ func (p *Process) CmdlineSliceWithContext(ctx context.Context) ([]string, error)
 		return nil, err
 	}
 
-	argc := 0
-	argvp := unsafe.Pointer(&buf[0])
-	argv := *(**C.char)(unsafe.Pointer(argvp))
-	size := unsafe.Sizeof(argv)
+	/* From man sysctl(2):
+	The buffer pointed to by oldp is filled with an array of char
+	pointers followed by the strings themselves. The last char
+	pointer is a NULL pointer. */
 	var strParts []string
+	r := bytes.NewReader(buf)
+	baseAddr := uintptr(unsafe.Pointer(&buf[0]))
+	for {
+		argvp, err := readPtr(r)
+		if err != nil {
+			return nil, err
+		}
+		if argvp == 0 { // check for a NULL pointer
+			break
+		}
+		offset := argvp - baseAddr
+		length := uintptr(bytes.IndexByte(buf[offset:], 0))
+		str := string(buf[offset : offset+length])
+		strParts = append(strParts, str)
+	}
 
-	for argv != nil {
-		strParts = append(strParts, C.GoString(argv))
+	return strParts, nil
+}
 
-		argc++
-		argv = *(**C.char)(unsafe.Pointer(uintptr(argvp) + uintptr(argc)*size))
+// readPtr reads a pointer data from a given reader. WARNING: only little
+// endian architectures are supported.
+func readPtr(r io.Reader) (uintptr, error) {
+	switch sizeofPtr {
+	case 4:
+		var p uint32
+		if err := binary.Read(r, binary.LittleEndian, &p); err != nil {
+			return 0, err
+		}
+		return uintptr(p), nil
+	case 8:
+		var p uint64
+		if err := binary.Read(r, binary.LittleEndian, &p); err != nil {
+			return 0, err
+		}
+		return uintptr(p), nil
+	default:
+		return 0, fmt.Errorf("unsupported pointer size")
 	}
-	return strParts, nil
 }
 
 func (p *Process) CmdlineWithContext(ctx context.Context) (string, error) {