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

Wrap firewall errors in a common struct #899

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

mjholub
Copy link
Contributor

@mjholub mjholub commented Apr 1, 2023

This allows a little bit of more flexibility since now there is only one return value instead of two. The underlying logic stays the same.
Also unrelated, but I've noticed that you're not tracking go.sum. This isn't a good practice.

@mjholub mjholub mentioned this pull request Apr 1, 2023
@gustavo-iniguez-goya
Copy link
Collaborator

there're a couple of errors here:

if _, err4 = core.Exec(ipt.bin, rule); err4 != nil {

if _, err6 = core.Exec(ipt.bin6, rule); err6 != nil {

err4 and err6 are not defined. Something in the line of var err4, err6 error would solve it.

Also "not enough return values" error here (func expects 2 values: nil, nil):

and "common" package is not used error:

"github.com/evilsocket/opensnitch/daemon/firewall/common"

@mjholub
Copy link
Contributor Author

mjholub commented Apr 9, 2023

Seems like I've botched something when bisecting this with my other feature branch. It's my fault and I'll fix this. My apologies.

@luzpaz
Copy link
Contributor

luzpaz commented Feb 12, 2024

Any progress here ?

@mjholub
Copy link
Contributor Author

mjholub commented Feb 13, 2024

I'll take a look

@mjholub
Copy link
Contributor Author

mjholub commented Feb 16, 2024

Fixed. Ran the tests locally, they fail in the UI package, not sure why as I haven't touched it and they only print the cgo warninigs about deprecated dependencies

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants