patch: isolate a pkcs11 module

Nikos Mavrogiannopoulos nmav at redhat.com
Fri Dec 19 02:25:11 PST 2014


----- Original Message -----
> On 02.12.2014 10:29, Nikos Mavrogiannopoulos wrote:
> > On Mon, 2014-11-10 at 11:41 +0100, Stef Walter wrote:
> >> On 03.11.2014 13:09, Nikos Mavrogiannopoulos wrote:
> >>> The attached patch allows to use p11-kit to run and use an isolated
> >>> PKCS #11 module. The performance cost seems to be quite limited.
> >>> I've tested it with softhsm (isolated) + lighttpd2 and a
> >>> pseudo-benchmark (run in the same pc) shows:
> >>
> >> This is great! Nice work. I'd like to get this in. Some review below
> >> that would need to be fixed first. Happy to have discussion about any
> >> points that aren't clear or where I've misunderstood things.
> > 
> > Attached is an update to the original patch. What is handled is
> > discussed in the comments inline.
> 
> Thanks.
> 
> > -#ifdef _GNU_SOURCE
> > -#error Make the crap stop. _GNU_SOURCE is completely unportable and
> breaks all sorts of behavior
> > -#endif
> 
> Could you document the logic for removing this? In particular some
> functions like strerror_r() behave completely differently when
> _GNU_SOURCE is defined. How do you account for this in your patch?

It is required by your code unix credentials.
In that file I've added:
+/* for ucred */
+#ifdef __linux__
+# define _GNU_SOURCE
+#endif

You rely on the ucred structure on linux, but this structure is not 
available if you don't use _GNU_SOURCE (that was the case in old glibcs).
We could avoid it by not using ucred on linux, but I'm not sure it's
better.

> > > The reason for this code isn't commented or documented anywhere.
> > > Would prefer if it was a separate commit with its own commit
> > > message, test, etc.
> > I've added a special test case.
> Hmmm, are you suggesting we merge all of this as one commit? If so, I
> guess I have to put in the extra work to clean it up. It's a valuable
> feature and worth the work.
> 
> But putting all this in one commit makes it hard to review and
> understand. For example, refactoring code (eg: moving code to
> rpc-transport-cli.c) is combined with everything else. And some of the
> other "drive by" fixes, fixing spelling, adding EINTER signal safety,
> are thrown into the mix.
> Do you by any chance have this as a distinct set of patches, which you
> may have squashed into a code dump to send here?

No unfortunately not. It was developed in a single patch.

regards,
Nikos


More information about the p11-glue mailing list