----- 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 _______________________________________________ p11-glue mailing list p11-glue@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/p11-glue