Skip to content

Update sdigest.h#214

Merged
plexoos merged 3 commits intomainfrom
fix_snprintf_for_eic_cuda_container
Mar 6, 2026
Merged

Update sdigest.h#214
plexoos merged 3 commits intomainfrom
fix_snprintf_for_eic_cuda_container

Conversation

@ggalgoczi
Copy link
Copy Markdown
Contributor

Fix snprintf size argument in sdigest.h to avoid _FORTIFY_SOURCE abort in eic_cuda container as described in #213

Fix snprintf size argument in sdigest.h to avoid _FORTIFY_SOURCE abort in eic_cuda container as described in #213
@ggalgoczi ggalgoczi requested a review from plexoos March 6, 2026 17:14
@ggalgoczi ggalgoczi self-assigned this Mar 6, 2026
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Cpp-linter Review

Used clang-format v20.1.2

Click here for the full clang-format patch
diff --git a/sysrap/sdigest.h b/sysrap/sdigest.h
index 0ac1147..e7275fc 100644
--- a/sysrap/sdigest.h
+++ b/sysrap/sdigest.h
@@ -312 +312,2 @@ inline std::string sdigest::DescRaw( unsigned char* digest16 )
-    for (int n = 0; n < 16; ++n) std::snprintf( &buf[2*n], 3, "%02x", (unsigned int)digest16[n]) ;
+    for (int n = 0; n < 16; ++n)
+        std::snprintf(&buf[2 * n], 3, "%02x", (unsigned int)digest16[n]);
@@ -326 +327,2 @@ inline std::string sdigest::Finalize(MD5_CTX& c) // static
-    for (int n = 0; n < 16; ++n) std::snprintf( &buf[2*n], 3, "%02x", (unsigned int)digest[n]) ;
+    for (int n = 0; n < 16; ++n)
+        std::snprintf(&buf[2 * n], 3, "%02x", (unsigned int)digest[n]);

Have any feedback or feature suggestions? Share it here.

Comment thread sysrap/sdigest.h Outdated
Comment thread sysrap/sdigest.h Outdated
ggalgoczi and others added 2 commits March 6, 2026 12:16
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@github-actions github-actions Bot dismissed their stale review March 6, 2026 17:17

outdated suggestion

@plexoos plexoos requested a review from Copilot March 6, 2026 17:22
Copy link
Copy Markdown
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 fixes an _FORTIFY_SOURCE runtime abort by correcting the std::snprintf size argument used when formatting MD5 digests into a 32-character hex string in the header-only sdigest implementation (per issue #213).

Changes:

  • Adjust sdigest::DescRaw() to use a per-iteration buffer size of 3 bytes (2 hex chars + null) instead of the full buffer length.
  • Adjust sdigest::Finalize() similarly to avoid passing an oversized snprintf length for sub-slices of the output buffer.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@plexoos
Copy link
Copy Markdown
Member

plexoos commented Mar 6, 2026

LGTM. So, do you intend to propose this upstream?

@ggalgoczi
Copy link
Copy Markdown
Contributor Author

Yes, I think it is useful for everybody.

@plexoos plexoos merged commit 33e1117 into main Mar 6, 2026
7 checks passed
@plexoos plexoos deleted the fix_snprintf_for_eic_cuda_container branch March 6, 2026 17:50
@plexoos
Copy link
Copy Markdown
Member

plexoos commented Mar 6, 2026

Okay, looking forward to an upstream PR for this change. Otherwise, it may be overwritten in the next sync.

@ggalgoczi
Copy link
Copy Markdown
Contributor Author

Opened PR upstream: simoncblyth/opticks#10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants