fix(cpp): build and repair asset::AssetContext - #580
Merged
Conversation
`longbridge.hpp` has always included `asset_context.hpp`, so C++ users could see `asset::AssetContext` — but `cpp/src/asset_context.cpp` was never listed in `cpp/CMakeLists.txt`, so the class was declared and then failed to link. It had never compiled either; three separate problems had accumulated behind the missing build entry. **`lb_statement_item_t` was not in the generated header.** `CStatementItem` is only reachable through the `void*` async-result pointer, so cbindgen never emitted it and no caller could read what `lb_asset_context_statements` returns. Exported it the same way the GridContext payloads are. `CAssetContext` was also missing from the rename map — every other context type was mapped, so the header exposed the raw Rust name (`const struct CAssetContext *lb_asset_context_new(…)`) while the C++ side forward-declares `lb_asset_context_t`. **`asset_context.cpp` did not include the C header.** Every other context source includes `longbridge.h`; this one included only its own `.hpp`, which forward-declares `lb_asset_context_t` as an opaque struct and nothing else — so `lb_asset_context_retain` and friends were undeclared. **`statement_download_url` decoded the wrong shape.** It read `res->data` as a `lb_statement_download_url_response_t*` and took `->url`. No such type exists anywhere in the C layer: `lb_asset_context_download_url` resolves a `CString`, so the callback receives the URL as a bare `const char*` — the same convention as `QuoteContext::quote_level`. Verified against a live environment: `statements` returns the same empty list the Rust core returns for the same account, and the symbols are present in `liblongbridge_cpp.so`. `statement_download_url` is compile- and link-checked only — the test account has no statement to supply a `file_key`.
sunli829
added a commit
that referenced
this pull request
Aug 26, 2026
## The bug
`std::vector::data()` is allowed to return `nullptr` for an empty
vector, and that is exactly what the C++ binding passes when a list
argument is omitted. The C layer fed the pointer straight to
`std::slice::from_raw_parts`, which requires a non-null, aligned pointer
**even for a zero-length slice** — so the call was undefined behaviour
and aborted the process:
```
thread '<unnamed>' panicked at c/src/quote_context/context.rs:784:24:
unsafe precondition(s) violated: slice::from_raw_parts requires the pointer to be
aligned and non-null, and the total size of the slice not to exceed `isize::MAX`
This indicates a bug in the program. This Undefined Behavior check is optional,
and cannot be relied on for safety.
thread caused non-unwinding panic. aborting.
```
Reproduced live by calling `QuoteContext::warrant_list` from C++ with no
filters — the most natural way to call it:
```cpp
ctx.warrant_list("700.HK", WarrantSortBy::LastDone, SortOrderType::Descending,
{}, {}, {}, {}, {}, // <-- every filter omitted
[](auto res) { ... });
```
## The fix
All 17 `from_raw_parts` call sites had this shape, so they now go
through a single null-tolerant helper in `c/src/types/mod.rs`:
```rust
pub(crate) unsafe fn slice_from_raw_parts<'a, T>(data: *const T, len: usize) -> &'a [T] {
if len == 0 || data.is_null() {
&[]
} else {
std::slice::from_raw_parts(data, len)
}
}
```
Covers `quote_context` (8 sites), `trade_context` (5), `agent_context`
(2), `alert_context` (1) and `cstr_array_to_rust` in `types` (1).
## Notes
- **Internal only.** The generated `longbridge.h` is byte-for-byte
unchanged, so the C and C++ public APIs are untouched.
- Release builds do not have the debug UB check, so the abort only shows
up in debug — but the undefined behaviour is there either way.
- Verified with `cargo clippy --all --all-features`, `cargo +nightly fmt
--all`, and a full CMake build.
- Independent of #580 — both branch off `main` and touch disjoint files,
except that both add an entry at the top of `CHANGELOG.md`, so whichever
merges second will need a trivial conflict resolution.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Collaborator
Author
|
@copilot resolve the merge conflicts in this pull request |
Co-authored-by: sunli829 <20092316+sunli829@users.noreply.github.com>
Contributor
Resolved by merging |
sunli829
added a commit
that referenced
this pull request
Aug 26, 2026
`QuoteContext.warrant_list` failed outright whenever the server included a
placeholder row. On `700.HK` with no filters, 324 of the 746 warrants
returned are placeholders — only `symbol`, `name` and `last_done` carry a
value, everything else is an empty string:
```
FilterWarrant { symbol: "68463.HK", name: "HS#TENCTRC2702E",
last_done: "0.244", expiry_date: "", strike_price: "",
premium: "", …, status: 0 }
```
The decimal fields already had `.parse().unwrap_or_default()` fallbacks.
Two strict fields did not, and either one aborted the conversion of the
*entire* list:
- `expiry_date: ""` → `parse field: expiry_date: the 'year' component
could not be parsed`
- `status: 0` → `No discriminant in enum WarrantStatus matches the value
0` (the enum only had `Suspend = 2` / `PrepareList = 3` / `Normal = 4`)
## Breaking changes
`WarrantInfo.expiry_date` is now optional: Rust `Option<Date>`, Python
`Optional[date]`, Node.js `NaiveDate | null`, Java nullable `LocalDate`,
C `const lb_date_t*` (NULL when absent), C++ `std::optional<Date>`.
`WarrantStatus` gains an `Unknown` variant, placed first to match
`WarrantType`. An unrecognized discriminant now maps to it rather than
failing, so a future unknown status cannot take out the whole list again.
Adding a variant shifts the ordinal of the existing ones in the C, C++,
Java, Node.js and Python bindings.
Placeholder rows now come through as `expiry_date: None, status: Unknown`
instead of being dropped or aborting the call. Verified live from both
Rust and C++: 746 rows returned, 324 of them placeholders.
Also carries the two fixes already submitted separately against `main` as
#579 (null pointers for empty list arguments) and #580 (building and
repairing `asset::AssetContext`) — both were needed to exercise this
change from C++.
sunli829
added a commit
that referenced
this pull request
Aug 26, 2026
`QuoteContext.warrant_list` failed outright whenever the server included a
placeholder row. On `700.HK` with no filters, 324 of the 746 warrants
returned are placeholders — only `symbol`, `name` and `last_done` carry a
value, everything else is an empty string:
```
FilterWarrant { symbol: "68463.HK", name: "HS#TENCTRC2702E",
last_done: "0.244", expiry_date: "", strike_price: "",
premium: "", …, status: 0 }
```
The decimal fields already had `.parse().unwrap_or_default()` fallbacks.
Two strict fields did not, and either one aborted the conversion of the
*entire* list:
- `expiry_date: ""` → `parse field: expiry_date: the 'year' component
could not be parsed`
- `status: 0` → `No discriminant in enum WarrantStatus matches the value
0` (the enum only had `Suspend = 2` / `PrepareList = 3` / `Normal = 4`)
## Breaking changes
`WarrantInfo.expiry_date` is now optional: Rust `Option<Date>`, Python
`Optional[date]`, Node.js `NaiveDate | null`, Java nullable `LocalDate`,
C `const lb_date_t*` (NULL when absent), C++ `std::optional<Date>`.
`WarrantStatus` gains an `Unknown` variant, placed first to match
`WarrantType`. An unrecognized discriminant now maps to it rather than
failing, so a future unknown status cannot take out the whole list again.
Adding a variant shifts the ordinal of the existing ones in the C, C++,
Java, Node.js and Python bindings.
Placeholder rows now come through as `expiry_date: None, status: Unknown`
instead of being dropped or aborting the call. Verified live from both
Rust and C++: 746 rows returned, 324 of them placeholders.
Also carries the two fixes already submitted separately against `main` as
#579 (null pointers for empty list arguments) and #580 (building and
repairing `asset::AssetContext`) — both were needed to exercise this
change from C++.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The bug
longbridge.hpphas always includedasset_context.hpp, so C++ users could seeasset::AssetContext— butcpp/src/asset_context.cppwas never listed incpp/CMakeLists.txt. The class was declared to users and then failed to link.Because it was never built, three separate problems had accumulated behind the missing build entry.
1.
lb_statement_item_twas not in the generated headerCStatementItemis only reachable through thevoid*async-result pointer, so cbindgen never emitted it and no C or C++ caller could read whatlb_asset_context_statementsreturns. Exported it the same way the GridContext payloads already are.CAssetContextwas also missing from the rename map — every other context type was mapped, so the header exposed the raw Rust name while the C++ side forward-declareslb_asset_context_t:2.
asset_context.cppdid not include the C headerEvery other context source includes
longbridge.h. This one included only its own.hpp, which forward-declareslb_asset_context_tas an opaque struct and nothing else — solb_asset_context_retainand friends were undeclared:3.
statement_download_urldecoded the wrong shapeIt read
res->dataas alb_statement_download_url_response_t*and took->url. No such type exists anywhere in the C layer.lb_asset_context_download_urlresolves aCString, so the callback receives the URL as a bareconst char*— the same convention asQuoteContext::quote_level:Verification
asset_context.cpp.onow builds, andliblongbridge_cpp.soexports theasset::AssetContextsymbols.statementschecked against a live environment: returns the same empty list the Rust core returns for the same account.statement_download_urlis compile- and link-checked only — the test account has no statement, so there is nofile_keyto exercise it with. It follows the same(const char*)res->dataconvention asquote_level, but this has not been confirmed against a real response.cargo clippy --all --all-featuresandcargo +nightly fmt --allclean; full CMake build with 0 errors.Notes
Independent of #579 — both branch off
mainand touch disjoint files, except that both add an entry at the top ofCHANGELOG.md, so whichever merges second will need a trivial conflict resolution.🤖 Generated with Claude Code