-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Bindgen more 7762 v8.3.1 #14579
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Bindgen more 7762 v8.3.1 #14579
Conversation
It does not return anything. The only known implementation is in dns where it does not return. And the C code does not bother to check a return value anyways.
That is what bindgen will do
just use an u32 out of the box. Will make bindgen like easier
Will make bindgen life easier As the fn prototype should not have istate: &mut u64 but a *mut AppLayerGetTxIterState which is an union with a u64
Will make bindgen life easier As the fn prototype should not have istate: &mut u64 but a *mut AppLayerGetTxIterState which is an union with a u64
As will be needed such as AppLayerTxData
So, fix the fn prototype in rust ParseFn
d190072 to
f508e02
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #14579 +/- ##
==========================================
- Coverage 82.16% 82.16% -0.01%
==========================================
Files 1013 1013
Lines 262387 262345 -42
==========================================
- Hits 215598 215544 -54
- Misses 46789 46801 +12
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
Information: QA ran without warnings. Pipeline = 29020 |
1 similar comment
|
Information: QA ran without warnings. Pipeline = 29020 |
|
Next steps
|
| --disable-header-comment \ | ||
| --default-enum-style rust \ | ||
| --with-derive-default \ | ||
| --with-derive-eq \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But we don't need it yet right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, we need it with last commit of #14556
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just looks out of place in this PR where it has no usage.
Link to ticket: https://redmine.openinfosecfoundation.org/issues/
https://redmine.openinfosecfoundation.org/issues/7762
Describe changes:
#14555 with mut local storage