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

Replace p11.Slot struct with interface #144

Open
JeremyRand opened this issue Nov 19, 2021 · 4 comments
Open

Replace p11.Slot struct with interface #144

JeremyRand opened this issue Nov 19, 2021 · 4 comments

Comments

@JeremyRand
Copy link
Contributor

p11.Session is an interface, which is helpful for use cases where a downstream user needs to re-implement the API. Alas, p11.Slot is not an interface, which makes such use cases rather unpleasant. Would a PR be accepted that makes p11.Slot an unexported struct and adds a public p11.Slot interface to replace it?

@miekg
Copy link
Owner

miekg commented Nov 19, 2021 via email

@JeremyRand
Copy link
Contributor Author

What's your preferred way of gauging opinions? Just wait some time to see if anyone comments in this issue? If, hypothetically, it were desired to make some of the other structs interfaces as well (the others aren't as high priority for me, but might be a bit helpful in the future), would it be preferable to roll them all into one PR to minimize the number of breaking change events?

@miekg
Copy link
Owner

miekg commented Nov 19, 2021 via email

@JeremyRand
Copy link
Contributor Author

Okay, so if we're going to roll all the breaking changes into one release, here are my suggestions for additional API changes:

  • Convert pkcs11.Ctx and p11.Object to interfaces as well; same motivation as p11.Slot.
  • Split out convenience functions such as p11.Object.Value and p11.Session.Login into separate interfaces, so that types that implement the underlying functions (p11.Object.Attribute and p11.Session.LoginAs) don't need to re-implement those convenience functions. So e.g. there could be a p11.Object interface which has both Attribute and Value functions, and a p11.ObjectCore interface that has Attribute but not Value, with a p11.ObjectFromCore function that returns an Object that uses the given ObjectCore.
  • I guess FindObjects bool is always false #26 can be rolled in too?
  • Once a PR is ready, ideally I'd like to run that PR in production for a few months before it's merged, so I can identify whether there are any other API changes that my use cases would benefit from; this reduces the risk that I'll ask for another breaking change later.

JeremyRand pushed a commit to JeremyRand/pkcs11 that referenced this issue Oct 13, 2022
JeremyRand pushed a commit to JeremyRand/pkcs11 that referenced this issue Oct 13, 2022
JeremyRand pushed a commit to JeremyRand/pkcs11 that referenced this issue Oct 13, 2022
JeremyRand pushed a commit to JeremyRand/pkcs11 that referenced this issue Oct 13, 2022
JeremyRand pushed a commit to JeremyRand/pkcs11 that referenced this issue Oct 13, 2022
JeremyRand pushed a commit to JeremyRand/pkcs11 that referenced this issue Oct 13, 2022
JeremyRand pushed a commit to namecoin/pkcs11 that referenced this issue Oct 13, 2022
JeremyRand pushed a commit to JeremyRand/pkcs11 that referenced this issue Nov 3, 2022
Expecting the user to manually cast Objects to Keys is counterintuitive,
and also blocks converting the Object and Key structs into interfaces
(since the cast targets will not be exported).  Adding an explicit
method that does the casting allows us to abstract this from the user.

Refs miekg#144
JeremyRand pushed a commit to namecoin/pkcs11 that referenced this issue Nov 3, 2022
JeremyRand pushed a commit to namecoin/pkcs11 that referenced this issue Nov 3, 2022
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

No branches or pull requests

2 participants