snap-seccomp: remove use of x/net/bpf from tests #3779

Merged
merged 7 commits into from Sep 1, 2017

Conversation

Projects
None yet
4 participants
Collaborator

mvo5 commented Aug 22, 2017

Based on https://github.com/snapcore/snapd/pull/3502/files

The bpf code of seccomp uses native endian. The x/net/bpf VM
always uses the network endian. This means we can not currently
simulate our geneated bpf with the bpf.VM. There is a open bug
at golang/go#20556

Given that we now also test the generated bpf against the in-kernel
seccomp implementation we can retire the bpf.VM tests (which test
exactly the same).

snap-seccomp: remove use of x/net/bpf from tests
The bpf code of seccomp uses native endian. The x/net/bpf VM
always uses the network endian. This means we can not currently
simulate our geneated bpf with the bpf.VM. There is a open bug
at golang/go#20556

Given that we now also test the generated bpf against the in-kernel
seccomp implementation we can retire the bpf.VM tests (which test
exactly the same).

codecov-io commented Aug 22, 2017

Codecov Report

Merging #3779 into master will decrease coverage by 0.05%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3779      +/-   ##
==========================================
- Coverage   75.74%   75.68%   -0.06%     
==========================================
  Files         409      409              
  Lines       35388    35388              
==========================================
- Hits        26804    26784      -20     
- Misses       6688     6708      +20     
  Partials     1896     1896
Impacted Files Coverage Δ
cmd/snap-seccomp/main.go 51.35% <0%> (-6.05%) ⬇️
interfaces/sorting.go 98.71% <0%> (-1.29%) ⬇️
cmd/snap/cmd_aliases.go 95% <0%> (+1.66%) ⬆️

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 73bbe43...06032a9. Read the comment docs.

mvo5 added some commits Aug 30, 2017

Looks good! Just one comment.

cmd/snap-seccomp/main_test.go
- }
- }
+ rc = sc_apply_seccomp_bpf(argv[1]);
+ if (rc || argc == 2)
@stolowski

stolowski Aug 30, 2017

Contributor

sc_apply_seccomp_bpf doesn't seem to be reporting any errors by return value, it exits right away or returns 0. I think returning instead of exiting makes it more friendly for testing, but not insisting on that. In any case, this conditional seems inconsistent with what the function is doing.

@mvo5

mvo5 Sep 1, 2017

Collaborator

Thanks, I addressed this now.

@stolowski

stolowski Sep 1, 2017

Contributor

Thanks! Is there a particular reason you return negative value from the function (only to return -rc from main)?

lgtm

@stolowski stolowski merged commit 51aaaa5 into snapcore:master Sep 1, 2017

7 checks passed

artful-amd64 autopkgtest finished (success)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
xenial-amd64 autopkgtest finished (success)
Details
xenial-i386 autopkgtest finished (success)
Details
xenial-ppc64el autopkgtest finished (success)
Details
yakkety-amd64 autopkgtest finished (success)
Details
zesty-amd64 autopkgtest finished (success)
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment