-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Warnint 64to32 6186 v23.2 #11705
Warnint 64to32 6186 v23.2 #11705
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #11705 +/- ##
==========================================
- Coverage 82.63% 82.63% -0.01%
==========================================
Files 919 919
Lines 248925 248913 -12
==========================================
- Hits 205703 205684 -19
- Misses 43222 43229 +7
Flags with carried forward coverage won't be shown. Click here to find out more. |
ERROR: ERROR: QA failed on SURI_TLPR1_alerts_cmp. Pipeline 22390 |
Information: QA ran without warnings. Pipeline 22430 |
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.
Thanks for this cleanup. I have a few comments inline.
@@ -294,19 +295,12 @@ static void EveHttpLogJSONExtended(JsonBuilder *js, htp_tx_t *tx) | |||
const int resp = tx->response_status_number; | |||
if (resp > 0) { | |||
jb_set_uint(js, "status", (uint32_t)resp); | |||
} else if (tx->response_status != NULL) { |
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.
This belongs in it's own commit
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.
and own issue and SV test with https://redmine.openinfosecfoundation.org/issues/7311
@@ -1107,9 +1107,9 @@ static bool GetAppBuffer(const TcpStream *stream, const uint8_t **data, uint32_t | |||
"got data at %"PRIu64". GAP of size %"PRIu64, | |||
offset, blk->offset, blk->offset - offset); | |||
*data = NULL; | |||
*data_len = blk->offset - offset; | |||
*data_len = (uint32_t)(blk->offset - offset); |
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.
not actually sure if we can't get a gap > 4GiB... It seems unlikely for sure. I guess we'd get all confused anyway due to sequence number wrapping around.
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.
How could we tell that the GAP is over 4Gbytes with TCP ?
I guess we'd get all confused anyway due to sequence number wrapping around.
Indeed
Next in #11904 |
Link to redmine ticket:
https://redmine.openinfosecfoundation.org/issues/6186
Describe changes:
-Wshorten-64-to-32
warnings for some files : output, streamSome commits of #9840
#11580 next batch
#11700 with code review taken into account
Still to do afterwards :
Victor, I let you review the stream stuff ;-)