gvforwarder: code cleanups#552
Conversation
df8c348 to
02dbc97
Compare
The `contains` helper is now part of the `slices` package Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
This error path returned from `rx` but did not report an error as all the other error paths. Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
This should help understand what gvforwarder is doing when it does not work as expected. Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
02dbc97 to
9be4413
Compare
|
Looks good to me! /approve |
| @@ -177,7 +175,7 @@ func rx(conn net.Conn, tap *water.Interface, errCh chan error, mtu int) { | |||
| } | |||
|
|
|||
| if n < 0 || n > math.MaxUint16 { | |||
There was a problem hiding this comment.
i wonder if it is actually possible for n to be less than 0 or we can remove it from the if clause.
By reading the func description it seems not "It returns the number of bytes read (0 <= n <= len(p)) and any error encountered."
There was a problem hiding this comment.
This check is needed to make static code checkers happy, they are not sophisticated enough to know what tap.Read can return. If I remove the if n < 0 check:
$ git diff
diff --git a/cmd/vm/main_linux.go b/cmd/vm/main_linux.go
index 550e0418..cd53c724 100644
--- a/cmd/vm/main_linux.go
+++ b/cmd/vm/main_linux.go
@@ -174,7 +174,7 @@ func rx(conn net.Conn, tap *water.Interface, errCh chan error, mtu int) {
log.Info(packet.String())
}
- if n < 0 || n > math.MaxUint16 {
+ if n > math.MaxUint16 {
errCh <- fmt.Errorf("invalid frame length (%d > %d)", n, math.MaxUint16)
return
}
$ make cross-lint
GOOS=linux make lint
make[1] : on entre dans le répertoire « /var/home/teuf/dev/gvisor-tap-vsock »
"/var/home/teuf/dev/gvisor-tap-vsock/tools/bin"/golangci-lint run
cmd/vm/main_linux.go:181:45: G115: integer overflow conversion int -> uint16 (gosec)
binary.LittleEndian.PutUint16(size, uint16(n))
^
1 issues:
* gosec: 1
make[1]: *** [Makefile:49: lint] Error 1
make[1] : on quitte le répertoire « /var/home/teuf/dev/gvisor-tap-vsock »
make: *** [Makefile:53: cross-lint] Error 2
|
(This needs a |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cfergeau, lstocchi, vyasgun The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This PR does minor cleanups to gvforwarder code, mainly to use newer features available in recent go versions.
It also add missing error reporting in one code path.