Issue #70: Disable SVP with build flag #71
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull request overview
This PR adds a build-time flag to disable SVP (Secure Video Path) support for environments that don't have SVP capabilities. The flag ENABLE_SVP defaults to ON, maintaining backward compatibility.
Changes:
- Added
ENABLE_SVPCMake option (defaults to ON) to control SVP support at build time - Guarded all SVP-related code with
#ifdef ENABLE_SVPdirectives - Provided stub implementations that return
SEC_RESULT_UNIMPLEMENTED_FEATUREwhen SVP is disabled - Updated test code to check for SVP capability support at runtime
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| CMakeLists.txt | Adds ENABLE_SVP build option (default ON) and sets corresponding compiler flags |
| include/sec_security_svp.h | Guards SVP-specific types, structs, and function declarations with ENABLE_SVP |
| src/sec_adapter_svp.c | Guards SVP implementation and adds stub functions returning unimplemented errors when disabled |
| src/sec_adapter_processor.c | Guards SVP buffer cleanup code and SVP-related command processing |
| src/sec_adapter_cipher.c | Guards all opaque buffer cipher operations to return unimplemented errors when SVP is disabled |
| test/openssl/src/test_creds_soc.cpp | Returns false for CAPABILITY_SVP when ENABLE_SVP is not defined |
| test/main/cpp/sec_api_utest_main.cpp | Wraps SVP-dependent test in runtime capability check |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mhabrat
requested changes
Feb 6, 2026
mhabrat
left a comment
mhabrat
left a comment
There was a problem hiding this comment.
- Update and extend copyright years.
- Is there any reason to include sec_security_svp.h in any file? I'd add the ifdef around its inclusion.
| case SA_CRYPTO_RANDOM: | ||
| // Doesn't require a handle so called directly and not in the invoke thread. | ||
| case SA_GET_NAME: | ||
| case SA_SVP_BUFFER_ALLOC: |
| @@ -411,6 +411,10 @@ Sec_Result SecCipher_ProcessFragmented(Sec_CipherHandle* cipherHandle, SEC_BYTE* | |||
| */ | |||
| Sec_Result SecCipher_ProcessOpaque(Sec_CipherHandle* cipherHandle, Sec_OpaqueBufferHandle* inOpaqueBufferHandle, | |||
There was a problem hiding this comment.
Why include this function at all since it is specific to SVP?
| // Stub implementations when SVP is disabled | ||
|
|
||
| // Deprecated | ||
| Sec_Result Sec_OpaqueBufferMalloc(SEC_SIZE bufLength, void** handle, void* params) { |
There was a problem hiding this comment.
Why include these functions in the non-SVP code base? They are specific to SVP operation.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue #70: Disable SVP with build flag (it will still be enabled by default)