Skip to content

Conversation

@kamilsa
Copy link
Contributor

@kamilsa kamilsa commented Nov 7, 2025

No description provided.

Copilot AI review requested due to automatic review settings November 7, 2025 07:06
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 adds C bindings for a post-quantum cryptography library (hash-sig), enabling C/C++ applications to use XMSS-based hash signature schemes. The implementation provides a complete FFI layer with memory management, serialization, and comprehensive test coverage.

Key Changes:

  • New C FFI crate (c_hash_sig) with 20 exported functions for key generation, signing, verification, and serialization
  • Complete example program demonstrating the API usage
  • Build infrastructure integration with CMake and Makefile support

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 12 comments.

Show a summary per file
File Description
crates/c_hash_sig/src/lib.rs Core FFI implementation with opaque pointer wrappers, memory management, and signature operations
crates/c_hash_sig/example.c Comprehensive C example demonstrating all API features
crates/c_hash_sig/build.rs Build script to generate C header files via cbindgen
crates/c_hash_sig/README.md Documentation covering API usage, building, and security considerations
crates/c_hash_sig/Makefile Build automation for library and example compilation
crates/c_hash_sig/Cargo.toml Crate configuration with dependencies and FFI settings
CMakeLists.txt Integration of new crate into existing CMake build system

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


```bash
# Linux
gcc -o example example.c -I. -L./target/release -lpq_bindings_c_rust -lpthread -ldl -lm
Copy link

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

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

The gcc command references -lpq_bindings_c_rust, but the actual library name in Cargo.toml is c_hash_sig_crust. This should be -lc_hash_sig_crust to match the library name, otherwise linking will fail.

Suggested change
gcc -o example example.c -I. -L./target/release -lpq_bindings_c_rust -lpthread -ldl -lm
gcc -o example example.c -I. -L./target/release -lc_hash_sig_crust -lpthread -ldl -lm

Copilot uses AI. Check for mistakes.
gcc -o example example.c \
-I. \
-L./target/release \
-lpq_bindings_c_rust \
Copy link

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

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

The Makefile references -lpq_bindings_c_rust, but the actual library name in Cargo.toml is c_hash_sig_crust. This should be -lc_hash_sig_crust to match the library name, otherwise the example compilation will fail.

Suggested change
-lpq_bindings_c_rust \
-lc_hash_sig_crust \

Copilot uses AI. Check for mistakes.
After building:
- Library: `target/release/libpq_bindings_c_rust.so` (Linux) or `target/release/libpq_bindings_c_rust.dylib` (macOS) or `target/release/pq_bindings_c_rust.dll` (Windows)
- Static library: `target/release/libpq_bindings_c_rust.a`
- Header file: `include/pq-bindings-c-rust.h` (automatically generated)
Copy link

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

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

The header file name pq-bindings-c-rust.h is inconsistent with the library name c_hash_sig_crust defined in Cargo.toml. The generated header file will be based on the actual crate name, not this reference.

Suggested change
- Header file: `include/pq-bindings-c-rust.h` (automatically generated)
- Header file: `include/c_hash_sig_crust.h` (automatically generated)

Copilot uses AI. Check for mistakes.
Ok(bytes) => {
if bytes.len() > buffer_len {
*written_len = bytes.len();
return PQSigningError::UnknownError; // Буфер слишком мал
Copy link

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

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

This comment contains Russian text ("Буфер слишком мал" meaning "Buffer too small"). The comment should be in English to maintain consistency with the rest of the codebase.

Suggested change
return PQSigningError::UnknownError; // Буфер слишком мал
return PQSigningError::UnknownError; // Buffer too small

Copilot uses AI. Check for mistakes.
#include <stdio.h>
#include <stdint.h>
#include <string.h>
#include "include/pq-bindings-c-rust.h"
Copy link

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

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

The header file name pq-bindings-c-rust.h is inconsistent with the library name c_hash_sig_crust defined in Cargo.toml. The generated header file will likely be named c-hash-sig-crust.h or similar based on the actual crate name. This will cause compilation failures when including this header.

Suggested change
#include "include/pq-bindings-c-rust.h"
#include "include/c-hash-sig-crust.h"

Copilot uses AI. Check for mistakes.
```c
#include <stdio.h>
#include <stdint.h>
#include "include/pq-bindings-c-rust.h"
Copy link

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

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

The header file name pq-bindings-c-rust.h is inconsistent with the library name c_hash_sig_crust defined in Cargo.toml. The generated header file will be based on the actual crate name, not this reference.

Copilot uses AI. Check for mistakes.

/// Frees memory allocated for signature
/// # Safety
/// Pointer must be valid and created via pq_sign
Copy link

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

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

The documentation states the signature must be "created via pq_sign", but signatures can also be created via pq_signature_deserialize. Update the documentation to include both creation methods: "created via pq_sign or pq_signature_deserialize".

Suggested change
/// Pointer must be valid and created via pq_sign
/// Pointer must be valid and created via pq_sign or pq_signature_deserialize

Copilot uses AI. Check for mistakes.
Comment on lines +100 to +102
/// Frees memory allocated for public key
/// # Safety
/// Pointer must be valid and created via pq_key_gen
Copy link

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

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

The documentation states keys must be "created via pq_key_gen", but keys can also be created via deserialization functions (pq_public_key_deserialize and pq_secret_key_deserialize). Update the documentation to include both creation methods.

Copilot uses AI. Check for mistakes.
Comment on lines 118 to 119
- Library: `target/release/libpq_bindings_c_rust.so` (Linux) or `target/release/libpq_bindings_c_rust.dylib` (macOS) or `target/release/pq_bindings_c_rust.dll` (Windows)
- Static library: `target/release/libpq_bindings_c_rust.a`
Copy link

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

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

The README references library names containing pq_bindings_c_rust, but the actual library name defined in Cargo.toml is c_hash_sig_crust. This inconsistency will cause linking failures. The library filename will actually be libc_hash_sig_crust.so (Linux), libc_hash_sig_crust.dylib (macOS), or c_hash_sig_crust.dll (Windows), not libpq_bindings_c_rust.* as stated here.

Suggested change
- Library: `target/release/libpq_bindings_c_rust.so` (Linux) or `target/release/libpq_bindings_c_rust.dylib` (macOS) or `target/release/pq_bindings_c_rust.dll` (Windows)
- Static library: `target/release/libpq_bindings_c_rust.a`
- Library: `target/release/libc_hash_sig_crust.so` (Linux) or `target/release/libc_hash_sig_crust.dylib` (macOS) or `target/release/c_hash_sig_crust.dll` (Windows)
- Static library: `target/release/libc_hash_sig_crust.a`

Copilot uses AI. Check for mistakes.
@kamilsa kamilsa requested review from Harrm and iceseer November 10, 2025 06:53
@kamilsa kamilsa merged commit a39b486 into master Nov 17, 2025
1 check passed
@kamilsa kamilsa deleted the pq-sig branch November 17, 2025 11:30
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

Successfully merging this pull request may close these issues.

4 participants