Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: 修复sys_exec_test.go函数中TestReadv测试函数使用错误 #297

Merged
merged 2 commits into from
Jan 23, 2024

Conversation

BinaryOracle
Copy link
Contributor

问题描述

项目路径下提供了一个测试用例文件: sys_exec_test.go , 其中提供了一个测试函数TestReadv,用于测试Readv系统调用:

func TestReadv(t *testing.T) {
	// 1. 向fd中写入数据
	r, w := GetSysFdPairs()
	vs := [][]byte{
		[]byte("first line"),  // len=10
		[]byte("second line"), // len=11
		[]byte("third line"),  // len=10
	}
	w1, _ := syscall.Write(w, vs[0])
	w2, _ := syscall.Write(w, vs[1])
	w3, _ := syscall.Write(w, vs[2])
	Equal(t, w1+w2+w3, 31)

	// 2. 准备bs和ivs缓冲区
	var barrier = barrier{}
	barrier.bs = [][]byte{
		make([]byte, 0),
		make([]byte, 10),
		make([]byte, 11),
		make([]byte, 10),
	}
	barrier.ivs = make([]syscall.Iovec, len(barrier.bs))
	// 3. 调用readv系统调用,读取数据到ivs , ivs缓冲区指针指向了bs
	rn, err := readv(r, barrier.bs, barrier.ivs)
	MustNil(t, err)
	Equal(t, rn, 31)
	// 4. 打印bs缓冲区中的数据
	for i, v := range barrier.bs {
		t.Logf("READ [%d] %s", i, v)
	}
}

在没有修改前,readv的源码如下:

// readv wraps the readv system call.
// return 0, nil means EOF.
func readv(fd int, bs [][]byte, ivs []syscall.Iovec) (n int, err error) {
        // 依次为每个ivs[i].base 与 bs[i] 建立映射关系
	iovLen := iovecs(bs, ivs)
	if iovLen == 0 {
		return 0, nil
	}
	// syscall
	r, _, e := syscall.RawSyscall(syscall.SYS_READV, uintptr(fd), uintptr(unsafe.Pointer(&ivs[0])), uintptr(iovLen))
        // 此处会清空bs和ivs缓冲区
	resetIovecs(bs, ivs[:iovLen])
	if e != 0 {
		return int(r), syscall.Errno(e)
	}
	return int(r), nil
}

最终输出的结果为 , 缓冲区中的数据为空,都被清空了,不符合预期:

=== RUN   TestReadv
    sys_exec_test.go:120: READ [0] 
    sys_exec_test.go:120: READ [1] 
    sys_exec_test.go:120: READ [2] 
    sys_exec_test.go:120: READ [3] 
--- PASS: TestReadv (0.00s)
PASS

解决办法

将readv中清空缓冲区的方法调用移除:

// readv wraps the readv system call.
// return 0, nil means EOF.
func readv(fd int, bs [][]byte, ivs []syscall.Iovec) (n int, err error) {
	iovLen := iovecs(bs, ivs)
	if iovLen == 0 {
		return 0, nil
	}
	// syscall
	r, _, e := syscall.RawSyscall(syscall.SYS_READV, uintptr(fd), uintptr(unsafe.Pointer(&ivs[0])), uintptr(iovLen))
	if e != 0 {
		return int(r), syscall.Errno(e)
	}
	return int(r), nil
}

再次运行测试用例,输出如下:

=== RUN   TestReadv
    sys_exec_test.go:124: READ [0] 
    sys_exec_test.go:124: READ [1] first line
    sys_exec_test.go:124: READ [2] second line
    sys_exec_test.go:124: READ [3] third line
--- PASS: TestReadv (0.00s)
PASS

@BinaryOracle BinaryOracle requested review from a team as code owners December 6, 2023 13:23
@CLAassistant
Copy link

CLAassistant commented Dec 6, 2023

CLA assistant check
All committers have signed the CLA.

@joway
Copy link
Member

joway commented Dec 7, 2023

@BinaryOracle 这个其实是这段测试有点问题。原本的代码没问题。之所以要reset,是因为 barrier 不能持有原始 []byte 的引用,否则这段 []byte 永远不能被 GC

Copy link
Member

@joway joway left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

可以修改 TestReadv ,独立创建每一个 []byte 的变量,然后直接读取变量中的内容。而不是读取 [][]byte 里的内容

@BinaryOracle BinaryOracle changed the title fix: 修复执行syscall.SYS_READV系统调用包装函数readv时,读完数据后,又清空缓冲区的bug fix: 修复sys_exec_test.go函数中TestReadv测试函数使用错误 Dec 7, 2023
@BinaryOracle
Copy link
Contributor Author

BinaryOracle commented Dec 7, 2023

@BinaryOracle 这个其实是这段测试有点问题。原本的代码没问题。之所以要reset,是因为 barrier 不能持有原始 []byte 的引用,否则这段 []byte 永远不能被 GC

我改了一下pr,把TestReadv测试函数修正了一下:

func TestReadv(t *testing.T) {
	r, w := GetSysFdPairs()
	vs := [][]byte{
		[]byte("first line"),  // len=10
		[]byte("second line"), // len=11
		[]byte("third line"),  // len=10
	}
	w1, _ := syscall.Write(w, vs[0])
	w2, _ := syscall.Write(w, vs[1])
	w3, _ := syscall.Write(w, vs[2])
	Equal(t, w1+w2+w3, 31)

	var barrier = barrier{
		bs: make([][]byte, 4),
	}
	res := [][]byte{
		make([]byte, 0),
		make([]byte, 10),
		make([]byte, 11),
		make([]byte, 10),
	}
	for i := range res {
		barrier.bs[i] = res[i]
	}
	barrier.ivs = make([]syscall.Iovec, len(barrier.bs))
	rn, err := readv(r, barrier.bs, barrier.ivs)
	MustNil(t, err)
	Equal(t, rn, 31)
	for i, v := range res {
		t.Logf("READ [%d] %s", i, v)
	}
}

@joway
Copy link
Member

joway commented Dec 8, 2023

@BinaryOracle Hi,能再辛苦你能签署下这个吗?
image

@BinaryOracle
Copy link
Contributor Author

@BinaryOracle Hi,能再辛苦你能签署下这个吗? image

@BinaryOracle
Copy link
Contributor Author

@BinaryOracle Hi,能再辛苦你能签署下这个吗? image

我看签署了CLA之后,但是显示处于pending状态:

image

然后我这边协议签署文件发送到对应的邮箱了,这边还需要做什么吗?

image

@joway
Copy link
Member

joway commented Dec 11, 2023

image @BinaryOracle 你提交commit用的邮箱应该不是你github的邮箱

@BinaryOracle
Copy link
Contributor Author

image @BinaryOracle 你提交commit用的邮箱应该不是你github的邮箱

提交代码的邮箱没认证,目前已经认证通过了

@joway joway merged commit a6a5330 into cloudwego:develop Jan 23, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants