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

Address comments related to SharedCrypto #59

Closed
matthewfcarlson opened this issue Apr 17, 2019 · 1 comment
Closed

Address comments related to SharedCrypto #59

matthewfcarlson opened this issue Apr 17, 2019 · 1 comment

Comments

@matthewfcarlson
Copy link

matthewfcarlson commented Apr 17, 2019

On this PR, microsoft/mu_plus#22 some comments were made that weren't addressed but it was agreed they would be addressed in a future PR.

Mike K:
The use of the term 'Shared' 'SHARED' should be removed from all types, variables, functions, library, and module names. The private protocol/ppi/smm protocol are an abstraction for the CryptoPkg/Include/BaseCryptLib.h, so the name of the protocol and the modules should make this relationship obvious. Perhaps using the names listed below for includes. The use of 'Base' in the name of the library class is unfortunate. The term 'Base' in the name of a lib instance implies module compatibility with all module types. Private/Protocol/BaseCrypt.h EDKII_BASE_CRYPT_PROTOCOL_GUID EDKII_BASE_CRYPT_PROTOCOL gEdkiiBaseCryptProtocolGuid Private/Protocol/SmmBaseCrypt.h EDKII_SMM_BASE_CRYPT_PROTOCOL_GUID EDKII_SMM_BASE_CRYPT_PROTOCOL gEdkiiSmmBaseCryptProtocolGuid Private/Ppi/BaseCrypt.h EDKII_PEI_BASE_CRYPT_PROTOCOL_GUID EDKII_PEI_BASE_CRYPT_PROTOCOL gEdkiiPeiBaseCryptProtocolGuid Library Instances (not using keyword 'Base') DxeCryptLibOnProtocol SmmCryptLibOnProtocol PeiCryptLibOnPpi Drivers CryptDxe CryptSmm CryptPei We could define a few feature flag PCDs that select between a NULL value and function pointer value when filling in the structure of crypto services. This can reduce the number of INFs and C files. Can we also consider making this a pull request against CryptoPkg in mu_tiano_plus?

There was also the issue of moving the publishing tools outside the code itself

@spbrogan
Copy link
Member

spbrogan commented May 1, 2020

shared crypto has gone thru lots of new review and changes. Existing workstreams continue to converge Mu and edk2

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