Skip to content

Add clean failure for EVFILT_PROC usage #23

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

Closed
wants to merge 1 commit into from

Conversation

seabaylea
Copy link
Contributor

The pull request does a couple of things:

  1. Enables the dispatch_proc test by resolving the compile issue around the usage of POSIX_SPAWN_START_SUSPENDED by wrapping it in a #if preprocessor
  2. Modifies source.c to return NULL for calls to dispatch_source_create() with DISPATCH_SOURCE_TYPE_PROC if the underlying usage of EVFILT_PROC isn't supported by the kqueue implementation available.
    The second part is implemented in autoconf by using AC_RUN_PROGRAM.

I looked at whether the ENOSYS reported by kevent64() could be percolated up to dispatch_source_create() at runtime, but I couldn't seen an elegant way to so so.

Additionally, EVFILT_PROC is correctly defined in queue\sys\event.h even though its usage is not supported, so it wasn't possible to do a simple definition check.

Let me know what you think, and if you have any suggestions for alternative approaches.

@MadCoder
Copy link
Contributor

No we should instead remove the DISPATCH_SOURCE_PROC_TYPE altogether. Having a build time failure as opposed to a runtime failure is better. In general anything that has no chance to be ported to linux because the underlying features are not readily present and close enough should be removed.

EVFILT_PROC is the poster child of this, where on paper linux could probably have some support, but for now it's only available on netlink for root, and there probably are some security concerns that need to be resolved first and have the kernel have support and interest to maintain that support.

IMO the dispatch_proc test should just not even be built for linux or skipped on linux, either way works.

@MadCoder
Copy link
Contributor

As to how to do that, I think anything EVFILT_PROC should be hidden from public (and private) headers with a #ifndef __linux__ until we figure out a nicer way (as in more systematic) to do this for ports, and your HAVE_EVFILT_PROC configure thing is perfectly fine to comment out anything in c files that needs be.

@seabaylea
Copy link
Contributor Author

Is it worth hiding DISPATCH_SOURCE_TYPE_PROC under HAVE_EVFILT_PROCrather than ifndef __linux__? That might be slightly more portable to other platforms and slightly more future proof should EVFILT_PROC ever be implemented.

@MadCoder
Copy link
Contributor

I had that discussion already, the problem with HAVE_EVFILT_PROC in a shipping header is that you need to ship config.h and these aren't namespaced, so that's bad.

We will discuss a better way to do this in the future, but for now that will do.

I think we'll have some kind of <dispatch/shim.h> or something similar that defines clean macros that are used to hide some features without using platform specific things like __APPLE__ or __linux__ (these will likely be used in that shim.h thing but that will be updated when a new platform is supported and be limited to that file).

@MadCoder
Copy link
Contributor

But I want to discuss that solution with @das and with vacations and stuff this will have to wait until after the break.

@dgrove-oss
Copy link
Contributor

#96 replaces this pull request.

@dgrove-oss dgrove-oss closed this Jun 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants