-
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
Warnint 64to32 6186 v23.1 #11700
Warnint 64to32 6186 v23.1 #11700
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #11700 +/- ##
==========================================
- Coverage 82.63% 82.62% -0.01%
==========================================
Files 919 919
Lines 248925 248927 +2
==========================================
- Hits 205703 205684 -19
- Misses 43222 43243 +21
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 22323 |
jb_set_uint(js, "status", val); | ||
uint32_t val; | ||
if (ByteExtractStringUint32(&val, 10, 0, status_string) == 0) { | ||
jb_set_uint(js, "status", val); |
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 we use htp_tx_t::response_status_number
instead?
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.
response_status_number is used in the regular case when it is > 0
Trying to drop this whole else
case
@@ -931,7 +921,8 @@ int OutputJSONMemBufferCallback(const char *str, size_t size, void *data) | |||
MemBufferExpand(memb, wrapper->expand_by); | |||
} | |||
|
|||
MemBufferWriteRaw((*memb), (const uint8_t *)str, size); | |||
DEBUG_VALIDATE_BUG_ON(size > UINT32_MAX); |
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.
can we change the size_t
to uint32_t
instead? size_t
width depends on arch but we don't need that here
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.
Nope, because libjannson json_dump_callback
wants size_t
:-/
Continued in #11705 |
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
Still to do afterwards :
Victor, I let you review the stream stuff ;-)