use golang.org/x/sys/unix instead of syscall #2927

marten-seemann · 2020-12-04T03:09:50Z

As suggested by @mvdan in #2896 (review). An excellent suggestion, as it allows us to get rid of two OS-specific files.

dswarbrick · 2020-12-04T09:41:28Z

conn_ecn.go

@@ -29,7 +31,7 @@ func inspectReadBuffer(c net.PacketConn) (int, error) {
 	var size int
 	var serr error
 	if err := rawConn.Control(func(fd uintptr) {
-		size, serr = syscall.GetsockoptInt(int(fd), syscall.SOL_SOCKET, syscall.SO_RCVBUF)
+		size, serr = unix.GetsockoptInt(int(fd), syscall.SOL_SOCKET, syscall.SO_RCVBUF)


These constants exist in the unix package also, for consistency.

Suggested change

size, serr = unix.GetsockoptInt(int(fd), syscall.SOL_SOCKET, syscall.SO_RCVBUF)

size, serr = unix.GetsockoptInt(int(fd), unix.SOL_SOCKET, unix.SO_RCVBUF)

Thank you, missed that one!

codecov · 2020-12-04T15:13:16Z

Codecov Report

Merging #2927 (f59cd92) into master (5a0ce24) will increase coverage by 0.37%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #2927      +/-   ##
==========================================
+ Coverage   86.24%   86.61%   +0.37%     
==========================================
  Files         133      132       -1     
  Lines        9143     9378     +235     
==========================================
+ Hits         7885     8122     +237     
+ Misses        910      907       -3     
- Partials      348      349       +1

Impacted Files	Coverage Δ
conn_ecn.go	`70.97% <100.00%> (ø)`
conn_windows.go	`60.00% <100.00%> (ø)`
qlog/frame.go	`98.47% <0.00%> (+0.20%)`	⬆️
qlog/event.go	`98.23% <0.00%> (+0.57%)`	⬆️
qlog/qlog.go	`96.96% <0.00%> (+1.29%)`	⬆️
qlog/packet_header.go	`98.46% <0.00%> (+3.72%)`	⬆️
qlog/trace.go	`96.77% <0.00%> (+5.47%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5a0ce24...f59cd92. Read the comment docs.

marten-seemann · 2020-12-06T03:18:40Z

Looks like I was too optimistic here.
On Linux, the Type of the TOS control message is unix.IP_TOS, whereas on Darwin it's unix.IP_RECVTOS 🤦🏻‍♂️:
https://github.com/lucas-clemente/quic-go/blob/5a0ce242654c153128ca8679e8fe19fc3be966d3/conn_ecn.go#L96-L105

That means we'll still need OS-specific files to set the msgTypeIPTOS constant.

marten-seemann requested a review from lucas-clemente December 4, 2020 03:09

mvdan approved these changes Dec 4, 2020

View reviewed changes

dswarbrick reviewed Dec 4, 2020

View reviewed changes

marten-seemann force-pushed the avoid-syscall branch 2 times, most recently from 48bc453 to 6eca82a Compare December 6, 2020 03:10

marten-seemann force-pushed the avoid-syscall branch from 6eca82a to 0716410 Compare December 6, 2020 03:23

use golang.org/x/sys/unix instead of syscall

f59cd92

marten-seemann force-pushed the avoid-syscall branch from 0716410 to f59cd92 Compare December 6, 2020 05:06

marten-seemann merged commit 325bc16 into master Dec 6, 2020

marten-seemann deleted the avoid-syscall branch December 6, 2020 05:19

aschmahmann mentioned this pull request May 14, 2021

Release v0.9.0 ipfs/kubo#8058

Closed

71 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

use golang.org/x/sys/unix instead of syscall #2927

use golang.org/x/sys/unix instead of syscall #2927

marten-seemann commented Dec 4, 2020

dswarbrick Dec 4, 2020

marten-seemann Dec 6, 2020

codecov bot commented Dec 4, 2020 •

edited

Loading

marten-seemann commented Dec 6, 2020

	size, serr = unix.GetsockoptInt(int(fd), syscall.SOL_SOCKET, syscall.SO_RCVBUF)
	size, serr = unix.GetsockoptInt(int(fd), unix.SOL_SOCKET, unix.SO_RCVBUF)

use golang.org/x/sys/unix instead of syscall #2927

use golang.org/x/sys/unix instead of syscall #2927

Conversation

marten-seemann commented Dec 4, 2020

dswarbrick Dec 4, 2020

Choose a reason for hiding this comment

marten-seemann Dec 6, 2020

Choose a reason for hiding this comment

codecov bot commented Dec 4, 2020 • edited Loading

Codecov Report

marten-seemann commented Dec 6, 2020

codecov bot commented Dec 4, 2020 •

edited

Loading