Remove noop from change key provider #283
Conversation
GetKeyProviderByName() will not be able to find a match if the name it's supposed to search for is longer than allowed.
AndersAstrand
force-pushed
the
tde/remove-noop-from-change-key-provider
branch
from
d26ae0f to
8cbfc11
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
❌ Your project status has failed because the head coverage (78.95%) is below the target coverage (90.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## TDE_REL_17_STABLE #283 +/- ##
=====================================================
+ Coverage 78.93% 78.95% +0.01%
=====================================================
Files 22 22
Lines 2464 2461 -3
Branches 385 384 -1
=====================================================
- Hits 1945 1943 -2
+ Misses 444 443 -1
Partials 75 75
🚀 New features to boost your workflow:
|
| @@ -235,7 +228,7 @@ pg_tde_change_key_provider_internal(PG_FUNCTION_ARGS, Oid dbOid) | |||
| /* Struct will be saved to disk so keep clean */ | |||
| memset(&provider, 0, sizeof(provider)); | |||
| provider.provider_id = keyring->keyring_id; | |||
| memcpy(provider.provider_name, provider_name, nlen); | |||
| memcpy(provider.provider_name, provider_name, strlen(provider_name)); | |||
There was a problem hiding this comment.
Shouldn't we instead copy from keyring.provider_name to make the intent clearer plus not change the case just because we change the values?
There was a problem hiding this comment.
Yes we should. but changing that behavior is out of scope for this PR. I intended to do that separately as well, but with a test for it included. Just didn't have time before the standup.
There was a problem hiding this comment.
This PR was mostly to protect #267 from questions about why an unrelated error message wasn't updated 😆 .
There was a problem hiding this comment.
I would have done it as part of this PR if I were you but this is an improvment either way.
GetKeyProviderByName() will not be able to find a match if the name it's supposed to search for is longer than allowed.