-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Warn integers downcast 64to32 6186 v1 #9105
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -60,7 +60,7 @@ typedef struct Frame { | |
typedef struct Frames { | ||
uint16_t cnt; | ||
uint16_t dyn_size; /**< size in elements of `dframes` */ | ||
uint32_t left_edge_rel; | ||
uint64_t left_edge_rel; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @victorjulien is this correct for frames ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no it's u32 intentionally, it's relative to something, I think the stream base seq. So when used it's added to that other thing. |
||
uint64_t base_id; | ||
Frame sframes[FRAMES_STATIC_CNT]; /**< static frames */ | ||
Frame *dframes; /**< dynamically allocated space for more frames */ | ||
|
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.
Should
AppLayerProtoDetectProbingParserGetMask
return a u64 ?But then flow structure needs to grow 2 u32 into 2 u64...
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.
Why an
u64
?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.
Because we will soon have more than 32 app-layer protocols, right ?