Skip to content

Conversation

@maxtropets
Copy link
Collaborator

@maxtropets maxtropets commented Sep 4, 2025

Because why not?..


Do not merge before:

  • Confirm perf OK

@maxtropets maxtropets self-assigned this Sep 4, 2025
@maxtropets maxtropets force-pushed the f/wrap-sha256-context-raii branch from 1f9175a to 6fe6f69 Compare September 4, 2025 10:55
@maxtropets maxtropets changed the title Wrap csha256 init/shutdown in a RAII wrapper [DRAFT] Wrap csha256 init/shutdown in a RAII wrapper Sep 4, 2025
@maxtropets maxtropets requested a review from Copilot September 4, 2025 12:44
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a RAII wrapper (Sha256ContextGuard) to replace manual initialization and cleanup calls for the OpenSSL SHA256 context management, improving resource safety and reducing boilerplate code.

  • Replaced manual ccf::crypto::openssl_sha256_init() and ccf::crypto::openssl_sha256_shutdown() calls with RAII pattern
  • Added Sha256ContextGuard class that handles initialization in constructor and cleanup in destructor
  • Updated documentation to reflect the new API usage pattern

Reviewed Changes

Copilot reviewed 28 out of 28 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
include/ccf/crypto/openssl_init.h Defines new Sha256ContextGuard RAII wrapper class
src/crypto/openssl/hash.cpp Implements the guard constructor/destructor and improves error handling
tests/perf-system/submitter/submit.cpp Migrates from manual init/shutdown calls to RAII guard
src/pal/test/snp_ioctl_test.cpp Migrates test code to use the new RAII pattern
src/pal/test/snp_attestation_validation.cpp Migrates test code to use the new RAII pattern
src/node/test/* Multiple test files migrated to use the RAII pattern
src/kv/test/* Multiple test files migrated to use the RAII pattern
src/indexing/test/indexing.cpp Migrates test code to use the RAII pattern
src/host/test/ledger.cpp Migrates test code to use the RAII pattern
src/host/run.cpp Migrates main application code to use the RAII pattern
src/enclave/enclave.h Migrates enclave initialization to use the RAII pattern
src/crypto/test/* Multiple crypto test files migrated to use the RAII pattern
doc/build_apps/get_started.rst Updates documentation to reflect new API usage
CHANGELOG.md Documents the API change
Comments suppressed due to low confidence (1)

src/node/test/history_bench.cpp:1

  • This line should call openssl_sha256_shutdown() instead of openssl_sha256_init(). The original code had a shutdown call here, but the replacement is incorrect.
// Copyright (c) Microsoft Corporation. All rights reserved.

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@maxtropets maxtropets force-pushed the f/wrap-sha256-context-raii branch from 6fe6f69 to 61e3e53 Compare September 5, 2025 12:56
@maxtropets maxtropets changed the title [DRAFT] Wrap csha256 init/shutdown in a RAII wrapper Wrap csha256 init/shutdown in a RAII wrapper Sep 5, 2025
@maxtropets maxtropets added run-long-test Run Long Test job bench-ab labels Sep 5, 2025
@maxtropets maxtropets marked this pull request as ready for review September 5, 2025 13:11
@maxtropets maxtropets requested a review from a team as a code owner September 5, 2025 13:11
@maxtropets maxtropets changed the title Wrap csha256 init/shutdown in a RAII wrapper Put csha256 init/shutdown under a RAII wrapper Sep 5, 2025
@maxtropets maxtropets changed the title Put csha256 init/shutdown under a RAII wrapper Put sha256 context init/shutdown under a RAII wrapper Sep 5, 2025
@maxtropets maxtropets added this pull request to the merge queue Sep 5, 2025
Merged via the queue into microsoft:main with commit fe9cb0c Sep 5, 2025
29 of 30 checks passed
@maxtropets maxtropets deleted the f/wrap-sha256-context-raii branch September 5, 2025 16:41
eddyashton pushed a commit to eddyashton/CCF that referenced this pull request Oct 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bench-ab run-long-test Run Long Test job

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants