As discussed in: https://lists.freedesktop.org/archives/p11-glue/2015-September/000576.html The first patch is to make the latter implementation simpler.
Created attachment 126241 [details] [review] build: Generate wrapper functions on the fly Generate wrapper functions in p11-kit/virtual.c from the definitions in common/pkcs11.h and common/pkcs11i.h, rather than hard-coding them.
Created attachment 126242 [details] [review] build: Generate fixed closures libffi's closure support may fail at run time, if the caller program is under a stricter SELinux policy. In that case, fallback to use a pre-compiled closures. https://bugzilla.redhat.com/show_bug.cgi?id=1271501
Comment on attachment 126241 [details] [review] build: Generate wrapper functions on the fly Review of attachment 126241 [details] [review]: ----------------------------------------------------------------- This introduces python as a build dependency. I'd like to keep it as a devel dependency. Would it be possible to distribute virtual-base.c and virtual-stack.c in the tarball, so that python is not necessary to build releases?
Comment on attachment 126242 [details] [review] build: Generate fixed closures Review of attachment 126242 [details] [review]: ----------------------------------------------------------------- Looks good. Just one thing that needs fixing. ::: p11-kit/modules.c @@ +1880,5 @@ > + *module = p11_virtual_wrap_fixed (virt, index, destroyer); > + if (*module) > + gl.fixed_closures[index] = *module; > + } > + } If there's not enough fixed modules ... then we fall through to the return_val_if_fail() below. These methods are used for programmer or unexpected system error. Can we handle this error case better? Print a legible failure message explaining as best we can about the failure case. Otherwise this is going to lead to hard to diagnose problems.
Created attachment 127064 [details] [review] build: Generate wrapper functions on the fly Generate wrapper functions in p11-kit/virtual.c from the definitions in common/pkcs11.h and common/pkcs11i.h, rather than hard-coding them.
Created attachment 127065 [details] [review] build: Generate fixed closures libffi's closure support may fail at run time, if the caller program is under a stricter SELinux policy. In that case, fallback to use a pre-compiled closures. https://bugzilla.redhat.com/show_bug.cgi?id=1271501
(In reply to Stef Walter from comment #3) > Comment on attachment 126241 [details] [review] [review] > build: Generate wrapper functions on the fly > > Review of attachment 126241 [details] [review] [review]: > ----------------------------------------------------------------- > > This introduces python as a build dependency. I'd like to keep it as a devel > dependency. Would it be possible to distribute virtual-base.c and > virtual-stack.c in the tarball, so that python is not necessary to build > releases? Yes, there was EXTRA_DIST += $(BUILT_SOURCES), but I made it more explicit. Also fixed AM_PATH_PYTHON invocation in configure.ac so it does not abort configure. (In reply to Stef Walter from comment #4) > Comment on attachment 126242 [details] [review] [review] > build: Generate fixed closures > > Review of attachment 126242 [details] [review] [review]: > ----------------------------------------------------------------- > > Looks good. Just one thing that needs fixing. > > ::: p11-kit/modules.c > @@ +1880,5 @@ > > + *module = p11_virtual_wrap_fixed (virt, index, destroyer); > > + if (*module) > > + gl.fixed_closures[index] = *module; > > + } > > + } > > If there's not enough fixed modules ... then we fall through to the > return_val_if_fail() below. These methods are used for programmer or > unexpected system error. > > Can we handle this error case better? Print a legible failure message > explaining as best we can about the failure case. Otherwise this is going to > lead to hard to diagnose problems. Added a log message there.
I realized that this could also make the libffi dependency optional while providing the "virtual" functionality. That might improve portability on older platforms. Opened a pull request doing that at: https://github.com/p11-glue/p11-kit/pull/9
Has this contribution moved completely to a GitHub pull request?
(In reply to Stef Walter from comment #9) > Has this contribution moved completely to a GitHub pull request? Yes, and a simplified version has just been checked in as: https://github.com/p11-glue/p11-kit/commit/9f632bed73c8800af16a69c97bd4c315bd350f8b
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.