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 error detection in pfring.NewRing #1087

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

EtienneMiret
Copy link

Fixes #147

The cgo documentation states:

Any C function (even void functions) may be called in a multiple assignment context to retrieve both the return value (if any) and the C errno variable as an error

See: https://pkg.go.dev/cmd/cgo

Furthermore, the manpage for errno states:

The value in errno is significant only when the return value of the call indicated an error (i.e., -1 from most system calls; -1 or NULL from most library functions); a function that succeeds is allowed to change errno. The value of errno is never set to zero by any system call or library function.

See: man 3 errno

Therefore, I believe the value of err MUST be ignored unless we have something else that reports an error.

@google-cla
Copy link

google-cla bot commented Jan 24, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

The cgo documentation states:

> Any C function (even void functions) may be called in a multiple
> assignment context to retrieve both the return value (if any) and the
> C errno variable as an error

See: <https://pkg.go.dev/cmd/cgo>

Furthermore, the manpage for errno states:

> The  value  in  errno is significant only when the return value of the
> call indicated an error (i.e., -1 from most system calls; -1 or NULL
> from most library functions); a function that succeeds is allowed to
> change errno. The value of errno is never set to zero by any system
> call or library function.

See: man 3 errno

Therefore, I believe the value of err MUST be ignored unless we have
something else that reports an error.
@EtienneMiret EtienneMiret force-pushed the pfring.NewRing-error-detection branch from cc94103 to fa9381e Compare January 24, 2023 15:45
@karlyeurl
Copy link

Hello,

If the error is to be ignored, isn't it better to avoid assigning it in the first place?

Something like cptr, _ := C.pfring_open(dev, C.u_int32_t(snaplen), C.u_int32_t(flags)) would make it clear that we do not care about the error returned by pfring_open().

@EtienneMiret
Copy link
Author

Hi @karlyeurl!

Actually, we do care about it when cptr is nil. This means the pfring_open() failed, and in that case we are allowed (and should) look at the C errno variable to have more details about which error occurred.

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.

"pfring NewRing error: no such device" when trying to open pfring ZC interface from zbalance_ipc
2 participants