From 2b05b035fcf2727f6b073c037cc84267a49d76ed Mon Sep 17 00:00:00 2001
From: Nadav Strahilevitz <nadav.strahilevitz@aquasec.com>
Date: Thu, 10 Oct 2024 08:45:56 +0000
Subject: [PATCH] chore(bufferdecoder): readability refactors

Move some functions around, rename and improve documentation.
---
 pkg/bufferdecoder/decoder.go                 | 28 ++++++++--
 pkg/bufferdecoder/decoder_test.go            |  2 +-
 pkg/bufferdecoder/eventsreader.go            | 59 +++++++++-----------
 pkg/bufferdecoder/eventsreader_bench_test.go |  8 +--
 pkg/bufferdecoder/eventsreader_test.go       |  6 +-
 pkg/ebpf/capture.go                          |  2 +-
 6 files changed, 58 insertions(+), 47 deletions(-)

diff --git a/pkg/bufferdecoder/decoder.go b/pkg/bufferdecoder/decoder.go
index 4acb3f5407a5..c76c03df2443 100644
--- a/pkg/bufferdecoder/decoder.go
+++ b/pkg/bufferdecoder/decoder.go
@@ -39,8 +39,14 @@ func (decoder *EbpfDecoder) BuffLen() int {
 	return len(decoder.buffer)
 }
 
-// ReadAmountBytes returns the total amount of bytes that decoder has read from its buffer up until now.
-func (decoder *EbpfDecoder) ReadAmountBytes() int {
+// BytesRead returns the total amount of bytes that decoder has read from its buffer up until now.
+func (decoder *EbpfDecoder) BytesRead() int {
+	return decoder.cursor
+}
+
+// MoveCursor moves the buffer cursor over n bytes. Returns the new cursor position.
+func (decoder *EbpfDecoder) MoveCursor(n int) int {
+	decoder.cursor += n
 	return decoder.cursor
 }
 
@@ -107,7 +113,7 @@ func (decoder *EbpfDecoder) DecodeArguments(args []trace.Argument, argnum int, e
 		args[idx] = arg
 	}
 
-	// Fill missing arguments metadata
+	// Fill missing arguments
 	for i := 0; i < len(evtFields); i++ {
 		if args[i].Value == nil {
 			args[i].ArgMeta = evtFields[i]
@@ -260,8 +266,20 @@ func (decoder *EbpfDecoder) DecodeBytes(msg []byte, size int) error {
 	return nil
 }
 
-// DecodeIntArray translate from the decoder buffer, starting from the decoder cursor, to msg, size * 4 bytes (in order to get int32).
-func (decoder *EbpfDecoder) DecodeIntArray(msg []int32, size int) error {
+// ReadBytesLen is a helper which allocates a known size bytes buffer and decodes
+// the bytes from the buffer into it.
+func (decoder *EbpfDecoder) ReadBytesLen(len int) ([]byte, error) {
+	var err error
+	res := make([]byte, len)
+	err = decoder.DecodeBytes(res[:], len)
+	if err != nil {
+		return nil, errfmt.Errorf("error reading byte array: %v", err)
+	}
+	return res, nil
+}
+
+// DecodeInt32Array translate from the decoder buffer, starting from the decoder cursor, to msg, size * 4 bytes (in order to get int32).
+func (decoder *EbpfDecoder) DecodeInt32Array(msg []int32, size int) error {
 	offset := decoder.cursor
 	if len(decoder.buffer[offset:]) < size*4 {
 		return ErrBufferTooShort
diff --git a/pkg/bufferdecoder/decoder_test.go b/pkg/bufferdecoder/decoder_test.go
index 6d7900b6ff3c..78beb51dbaf3 100644
--- a/pkg/bufferdecoder/decoder_test.go
+++ b/pkg/bufferdecoder/decoder_test.go
@@ -354,7 +354,7 @@ func TestDecodeIntArray(t *testing.T) {
 	raw = append(raw, 1, 2, 3, 4, 5, 6, 7, 8)
 	decoder := New(raw)
 	var obtained [2]int32
-	err := decoder.DecodeIntArray(obtained[:], 2)
+	err := decoder.DecodeInt32Array(obtained[:], 2)
 	assert.Equal(t, nil, err)
 	rawcp := append(raw, 1, 2, 3, 4, 5, 6, 7, 8)
 	dataBuff := bytes.NewBuffer(rawcp)
diff --git a/pkg/bufferdecoder/eventsreader.go b/pkg/bufferdecoder/eventsreader.go
index 83469dddec2e..b7e891e2adcf 100644
--- a/pkg/bufferdecoder/eventsreader.go
+++ b/pkg/bufferdecoder/eventsreader.go
@@ -79,23 +79,22 @@ func readArgFromBuff(id events.ID, ebpfMsgDecoder *EbpfDecoder, fields []trace.A
 	case trace.STR_T:
 		res, err = readStringFromBuff(ebpfMsgDecoder)
 	case trace.STR_ARR_T:
-		// TODO optimization: create slice after getting arrLen
-		var ss []string
 		var arrLen uint8
 		err = ebpfMsgDecoder.DecodeUint8(&arrLen)
 		if err != nil {
 			return uint(argIdx), arg, errfmt.Errorf("error reading string array number of elements: %v", err)
 		}
+		strSlice := make([]string, 0, arrLen)
 		for i := 0; i < int(arrLen); i++ {
 			s, err := readStringFromBuff(ebpfMsgDecoder)
 			if err != nil {
 				return uint(argIdx), arg, errfmt.Errorf("error reading string element: %v", err)
 			}
-			ss = append(ss, s)
+			strSlice = append(strSlice, s)
 		}
-		res = ss
+		res = strSlice
 	case trace.ARGS_ARR_T:
-		var ss []string
+		var strSlice []string
 		var arrLen uint32
 		var argNum uint32
 
@@ -107,18 +106,18 @@ func readArgFromBuff(id events.ID, ebpfMsgDecoder *EbpfDecoder, fields []trace.A
 		if err != nil {
 			return uint(argIdx), arg, errfmt.Errorf("error reading args number: %v", err)
 		}
-		resBytes, err := ReadByteSliceFromBuff(ebpfMsgDecoder, int(arrLen))
+		resBytes, err := ebpfMsgDecoder.ReadBytesLen(int(arrLen))
 		if err != nil {
 			return uint(argIdx), arg, errfmt.Errorf("error reading args array: %v", err)
 		}
-		ss = strings.Split(string(resBytes), "\x00")
-		if ss[len(ss)-1] == "" {
-			ss = ss[:len(ss)-1]
+		strSlice = strings.Split(string(resBytes), "\x00")
+		if strSlice[len(strSlice)-1] == "" {
+			strSlice = strSlice[:len(strSlice)-1]
 		}
-		for int(argNum) > len(ss) {
-			ss = append(ss, "?")
+		for int(argNum) > len(strSlice) {
+			strSlice = append(strSlice, "?")
 		}
-		res = ss
+		res = strSlice
 	case trace.BYTES_T:
 		var size uint32
 		err = ebpfMsgDecoder.DecodeUint32(&size)
@@ -129,10 +128,10 @@ func readArgFromBuff(id events.ID, ebpfMsgDecoder *EbpfDecoder, fields []trace.A
 		if size > 4096 && (id < events.NetPacketBase || id > events.MaxNetID) {
 			return uint(argIdx), arg, errfmt.Errorf("byte array size too big: %d", size)
 		}
-		res, err = ReadByteSliceFromBuff(ebpfMsgDecoder, int(size))
+		res, err = ebpfMsgDecoder.ReadBytesLen(int(size))
 	case trace.INT_ARR_2_T:
 		var intArray [2]int32
-		err = ebpfMsgDecoder.DecodeIntArray(intArray[:], 2)
+		err = ebpfMsgDecoder.DecodeInt32Array(intArray[:], 2)
 		if err != nil {
 			return uint(argIdx), arg, errfmt.Errorf("error reading int elements: %v", err)
 		}
@@ -165,7 +164,7 @@ func readArgFromBuff(id events.ID, ebpfMsgDecoder *EbpfDecoder, fields []trace.A
 	return uint(argIdx), arg, nil
 }
 
-func GetFieldType(fieldType string) trace.DecodeAs {
+func GetDecodeType(fieldType string) trace.DecodeAs {
 	switch fieldType {
 	case "int":
 		return trace.INT_T
@@ -228,7 +227,7 @@ func readSockaddrFromBuff(ebpfMsgDecoder *EbpfDecoder) (map[string]string, error
 					char        sun_path[108];  // Pathname
 			};
 		*/
-		sunPath, err := readStringVarFromBuff(ebpfMsgDecoder, 108)
+		sunPath, err := readVarStringFromBuffer(ebpfMsgDecoder, 108)
 		if err != nil {
 			return nil, errfmt.Errorf("error parsing sockaddr_un: %v", err)
 		}
@@ -258,7 +257,7 @@ func readSockaddrFromBuff(ebpfMsgDecoder *EbpfDecoder) (map[string]string, error
 			return nil, errfmt.Errorf("error parsing sockaddr_in: %v", err)
 		}
 		res["sin_addr"] = PrintUint32IP(addr)
-		_, err := ReadByteSliceFromBuff(ebpfMsgDecoder, 8)
+		_, err := ebpfMsgDecoder.ReadBytesLen(8)
 		if err != nil {
 			return nil, errfmt.Errorf("error parsing sockaddr_in: %v", err)
 		}
@@ -289,7 +288,7 @@ func readSockaddrFromBuff(ebpfMsgDecoder *EbpfDecoder) (map[string]string, error
 			return nil, errfmt.Errorf("error parsing sockaddr_in6: %v", err)
 		}
 		res["sin6_flowinfo"] = strconv.Itoa(int(flowinfo))
-		addr, err := ReadByteSliceFromBuff(ebpfMsgDecoder, 16)
+		addr, err := ebpfMsgDecoder.ReadBytesLen(16)
 		if err != nil {
 			return nil, errfmt.Errorf("error parsing sockaddr_in6: %v", err)
 		}
@@ -304,6 +303,9 @@ func readSockaddrFromBuff(ebpfMsgDecoder *EbpfDecoder) (map[string]string, error
 	return res, nil
 }
 
+// readStringFromBuff reads strings from the event buffer using the following format:
+//
+// [32bit:string_size][string_size-1:byte_buffer][8bit:null_terminator]
 func readStringFromBuff(ebpfMsgDecoder *EbpfDecoder) (string, error) {
 	var err error
 	var size uint32
@@ -314,7 +316,7 @@ func readStringFromBuff(ebpfMsgDecoder *EbpfDecoder) (string, error) {
 	if size > 4096 {
 		return "", errfmt.Errorf("string size too big: %d", size)
 	}
-	res, err := ReadByteSliceFromBuff(ebpfMsgDecoder, int(size-1)) // last byte is string terminating null
+	res, err := ebpfMsgDecoder.ReadBytesLen(int(size - 1)) // last byte is string terminating null
 	defer func() {
 		var dummy int8
 		err := ebpfMsgDecoder.DecodeInt8(&dummy) // discard last byte which is string terminating null
@@ -328,9 +330,10 @@ func readStringFromBuff(ebpfMsgDecoder *EbpfDecoder) (string, error) {
 	return string(res), nil
 }
 
-// readStringVarFromBuff reads a null-terminated string from `buff`
-// max length can be passed as `max` to optimize memory allocation, otherwise pass 0
-func readStringVarFromBuff(decoder *EbpfDecoder, max int) (string, error) {
+// readVarStringFromBuffer reads a null-terminated string from the ebpf buffer where the size is not
+// known. The helper will read from the buffer char-by-char until it hits the null terminator
+// or the given max length. The cursor will then skip to the point in the buffer after the max length.
+func readVarStringFromBuffer(decoder *EbpfDecoder, max int) (string, error) {
 	var err error
 	var char int8
 	res := make([]byte, 0, max)
@@ -355,20 +358,10 @@ func readStringVarFromBuff(decoder *EbpfDecoder, max int) (string, error) {
 	// The exact reason for this Trim is not known, so remove it for now,
 	// since it increases processing time.
 	// res = bytes.TrimLeft(res[:], "\000")
-	decoder.cursor += max - count // move cursor to the end of the buffer
+	decoder.MoveCursor(max - count) // skip the cursor to the desired endpoint
 	return string(res), nil
 }
 
-func ReadByteSliceFromBuff(ebpfMsgDecoder *EbpfDecoder, len int) ([]byte, error) {
-	var err error
-	res := make([]byte, len)
-	err = ebpfMsgDecoder.DecodeBytes(res[:], len)
-	if err != nil {
-		return nil, errfmt.Errorf("error reading byte array: %v", err)
-	}
-	return res, nil
-}
-
 // PrintUint32IP prints the IP address encoded as a uint32
 func PrintUint32IP(in uint32) string {
 	ip := make(net.IP, net.IPv4len)
diff --git a/pkg/bufferdecoder/eventsreader_bench_test.go b/pkg/bufferdecoder/eventsreader_bench_test.go
index 305b0c3f9d50..bd1136a69ceb 100644
--- a/pkg/bufferdecoder/eventsreader_bench_test.go
+++ b/pkg/bufferdecoder/eventsreader_bench_test.go
@@ -12,7 +12,7 @@ func BenchmarkReadStringVarFromBuff_ShortString(b *testing.B) {
 
 	for i := 0; i < b.N; i++ {
 		decoder := New(buffer)
-		str, _ = readStringVarFromBuff(decoder, max)
+		str, _ = readVarStringFromBuffer(decoder, max)
 	}
 	_ = str
 }
@@ -24,7 +24,7 @@ func BenchmarkReadStringVarFromBuff_MediumString(b *testing.B) {
 
 	for i := 0; i < b.N; i++ {
 		decoder := New(buffer)
-		str, _ = readStringVarFromBuff(decoder, max)
+		str, _ = readVarStringFromBuffer(decoder, max)
 	}
 	_ = str
 }
@@ -37,7 +37,7 @@ func BenchmarkReadStringVarFromBuff_LongString(b *testing.B) {
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
 		decoder := New(buffer)
-		str, _ = readStringVarFromBuff(decoder, max)
+		str, _ = readVarStringFromBuffer(decoder, max)
 	}
 	_ = str
 }
@@ -50,7 +50,7 @@ func BenchmarkReadStringVarFromBuff_LongStringLowMax(b *testing.B) {
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
 		decoder := New(buffer)
-		str, _ = readStringVarFromBuff(decoder, max)
+		str, _ = readVarStringFromBuffer(decoder, max)
 	}
 	_ = str
 }
diff --git a/pkg/bufferdecoder/eventsreader_test.go b/pkg/bufferdecoder/eventsreader_test.go
index 9fe6817387b4..fa14e76f8b86 100644
--- a/pkg/bufferdecoder/eventsreader_test.go
+++ b/pkg/bufferdecoder/eventsreader_test.go
@@ -183,7 +183,7 @@ func TestReadArgFromBuff(t *testing.T) {
 			if tc.name == "unknown" {
 				return
 			}
-			assert.Empty(t, decoder.BuffLen()-decoder.ReadAmountBytes(), tc.name) // passed in buffer should be emptied out
+			assert.Empty(t, decoder.BuffLen()-decoder.BytesRead(), tc.name) // passed in buffer should be emptied out
 		})
 	}
 }
@@ -258,13 +258,13 @@ func TestReadStringVarFromBuff(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			decoder := New(tt.buffer)
-			actual, err := readStringVarFromBuff(decoder, tt.max)
+			actual, err := readVarStringFromBuffer(decoder, tt.max)
 			if tt.expectError {
 				assert.Error(t, err)
 			} else {
 				assert.NoError(t, err)
 				assert.Equal(t, tt.expected, actual)
-				assert.Equal(t, tt.expectedCursor, decoder.ReadAmountBytes())
+				assert.Equal(t, tt.expectedCursor, decoder.BytesRead())
 			}
 		})
 	}
diff --git a/pkg/ebpf/capture.go b/pkg/ebpf/capture.go
index 296c8f35bbc3..f17a81bcdc88 100644
--- a/pkg/ebpf/capture.go
+++ b/pkg/ebpf/capture.go
@@ -170,7 +170,7 @@ func (t *Tracee) handleFileCaptures(ctx context.Context) {
 				}
 			}
 
-			dataBytes, err := bufferdecoder.ReadByteSliceFromBuff(ebpfMsgDecoder, int(meta.Size))
+			dataBytes, err := ebpfMsgDecoder.ReadBytesLen(int(meta.Size))
 			if err != nil {
 				if err := f.Close(); err != nil {
 					t.handleError(err)