From aa124721393da4397b4a27861c6c87a5dd438600 Mon Sep 17 00:00:00 2001 From: Alex Kristiansen Date: Mon, 4 Mar 2024 08:01:42 -0800 Subject: [PATCH 1/3] fix panic --- metric/system/cgroup/reader.go | 7 ++++++- metric/system/cgroup/reader_test.go | 9 +++++++++ metric/system/process/process_common.go | 4 ++-- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/metric/system/cgroup/reader.go b/metric/system/cgroup/reader.go index f9aae99fef..050f39870b 100644 --- a/metric/system/cgroup/reader.go +++ b/metric/system/cgroup/reader.go @@ -68,7 +68,7 @@ const ( memoryStat = "memory" ) -//nolint: deadcode,structcheck,unused // needed by other platforms +// nolint: deadcode,structcheck,unused // needed by other platforms type mount struct { subsystem string // Subsystem name (e.g. cpuacct). mountpoint string // Mountpoint of the subsystem (e.g. /cgroup/cpuacct). @@ -117,6 +117,11 @@ func NewReader(rootfsMountpoint resolve.Resolver, ignoreRootCgroups bool) (*Read // NewReaderOptions creates and returns a new Reader with the given options. func NewReaderOptions(opts ReaderOptions) (*Reader, error) { + // we don't want a nil pointer deref if someone forgets to set this + if opts.RootfsMountpoint == nil { + opts.RootfsMountpoint = resolve.NewTestResolver("/") + } + // Determine what subsystems are supported by the kernel. subsystems, err := SupportedSubsystems(opts.RootfsMountpoint) // We can return a not-quite-an-error ErrCgroupsMissing here, so return the bare error. diff --git a/metric/system/cgroup/reader_test.go b/metric/system/cgroup/reader_test.go index 78ff24e2cc..ac6d388fac 100644 --- a/metric/system/cgroup/reader_test.go +++ b/metric/system/cgroup/reader_test.go @@ -36,6 +36,15 @@ const ( idv2 = "docker-1c8fa019edd4b9d4b2856f4932c55929c5c118c808ed5faee9a135ca6e84b039.scope" ) +func TestReaderOptsWithoutResolve(t *testing.T) { + reader, err := NewReaderOptions(ReaderOptions{}) + require.NoError(t, err) + + // actual value doesn't matter, the point is that we don't set RootfsMountpoint and we __don't__ get a nil pointer deref panic. + _, err = reader.CgroupsVersion(345) + require.Error(t, err) +} + func TestV1EventDifferentPaths(t *testing.T) { pid := 3757 reader, err := NewReader(resolve.NewTestResolver("testdata/ubuntu1804"), true) diff --git a/metric/system/process/process_common.go b/metric/system/process/process_common.go index 859864f164..d2ca94ee69 100644 --- a/metric/system/process/process_common.go +++ b/metric/system/process/process_common.go @@ -39,7 +39,7 @@ import ( // ProcNotExist indicates that a process was not found. var ProcNotExist = errors.New("process does not exist") -//ProcsMap is a convinence wrapper for the oft-used ideom of map[int]ProcState +// ProcsMap is a convinence wrapper for the oft-used ideom of map[int]ProcState type ProcsMap map[int]ProcState // ProcsTrack is a thread-safe wrapper for a process Stat object's internal map of processes. @@ -107,7 +107,7 @@ type Stats struct { host types.Host } -//PidState are the constants for various PID states +// PidState are the constants for various PID states type PidState string var ( From fdffb9eecb84ef9f08ea60756b26c6f3fff3c216 Mon Sep 17 00:00:00 2001 From: Alex Kristiansen Date: Mon, 4 Mar 2024 08:20:06 -0800 Subject: [PATCH 2/3] linter --- metric/system/cgroup/reader.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metric/system/cgroup/reader.go b/metric/system/cgroup/reader.go index c494c2cfe6..c4702fe86d 100644 --- a/metric/system/cgroup/reader.go +++ b/metric/system/cgroup/reader.go @@ -70,7 +70,7 @@ const ( memoryStat = "memory" ) -// nolint: deadcode,structcheck,unused // needed by other platforms +//nolint: deadcode,structcheck,unused // needed by other platforms type mount struct { subsystem string // Subsystem name (e.g. cpuacct). mountpoint string // Mountpoint of the subsystem (e.g. /cgroup/cpuacct). From 63521fcf764c1f020a38b51f51150d5f3cc733be Mon Sep 17 00:00:00 2001 From: fearful-symmetry Date: Mon, 4 Mar 2024 08:53:43 -0800 Subject: [PATCH 3/3] remove unused struct? --- metric/system/cgroup/reader.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/metric/system/cgroup/reader.go b/metric/system/cgroup/reader.go index c4702fe86d..af83d849dc 100644 --- a/metric/system/cgroup/reader.go +++ b/metric/system/cgroup/reader.go @@ -70,15 +70,6 @@ const ( memoryStat = "memory" ) -//nolint: deadcode,structcheck,unused // needed by other platforms -type mount struct { - subsystem string // Subsystem name (e.g. cpuacct). - mountpoint string // Mountpoint of the subsystem (e.g. /cgroup/cpuacct). - path string // Relative path to the cgroup (e.g. /docker/). - id string // ID of the cgroup. - fullPath string // Absolute path to the cgroup. It's the mountpoint joined with the path. -} - // pathListWithTime combines PathList with a timestamp. type pathListWithTime struct { added time.Time