Skip to content
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 v19.5 #11247

Closed

Conversation

catenacyber
Copy link
Contributor

[Link to redmine ticket:
https://redmine.openinfosecfoundation.org/issues/6186

Describe changes:

  • fix -Wshorten-64-to-32 warnings for some files (a*)

First commit of #9840
#10899 with needed rebase

There must be some stuff to review more carefully

Ticket: 6186

Warnings about downcast from 64 to 32 bits

Generic fixes required to get app-layer clean
Ticket: OISF#6186

Warnings about downcast from 64 to 32 bits
Copy link

codecov bot commented Jun 5, 2024

Codecov Report

Attention: Patch coverage is 84.88372% with 13 lines in your changes missing coverage. Please review.

Project coverage is 82.98%. Comparing base (8781e93) to head (ea77629).

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #11247      +/-   ##
==========================================
- Coverage   82.99%   82.98%   -0.01%     
==========================================
  Files         942      942              
  Lines      249358   249377      +19     
==========================================
- Hits       206951   206946       -5     
- Misses      42407    42431      +24     
Flag Coverage Δ
fuzzcorpus 61.09% <83.72%> (-0.02%) ⬇️
livemode 18.79% <4.65%> (-0.05%) ⬇️
pcap 44.31% <74.41%> (-0.01%) ⬇️
suricata-verify 61.67% <81.39%> (-0.01%) ⬇️
unittests 60.50% <38.37%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@@ -368,7 +368,9 @@ static int FrameSlide(const char *ds, Frames *frames, const TcpStream *stream, c
}
frames->cnt = x;
uint64_t o = STREAM_BASE_OFFSET(stream) + slide;
frames->left_edge_rel = le - (STREAM_BASE_OFFSET(stream) + slide);
BUG_ON(o > le);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should these be DEBUG_VALIDATE_BUG_ON?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@@ -790,15 +791,16 @@ static void FramePrune(Frames *frames, const TcpStream *stream, const bool eof)
}
}
frames->cnt = x;
frames->left_edge_rel = le - STREAM_BASE_OFFSET(stream);
BUG_ON(le < STREAM_BASE_OFFSET(stream));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DEBUG_VALIDATE_BUG_ON?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one was your code :

git blame -v src/app-layer-frames.c | grep BUG_ON

1556e86c7db (Victor Julien 2021-12-03 07:40:56 +0100 801) BUG_ON(le < STREAM_BASE_OFFSET(stream));

@@ -40,7 +40,7 @@ typedef struct HttpRangeContainerBuffer {
/** offset of bytes written in buffer (relative to the start of the range) */
uint64_t offset;
/** number of gaped bytes */
uint64_t gap;
uint32_t gap;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a file can be larger than 4GiB, so a gap in it's data could also exceed 4GiB

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, will saturate it...

@@ -63,7 +63,7 @@ typedef struct HttpRangeContainerFile {
/** key length */
uint32_t len;
/** expire time in epoch */
uint32_t expire;
uint64_t expire;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SCTime?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

Copy link
Member

@victorjulien victorjulien left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some feedback and suggestions inline.

@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 20970

@catenacyber
Copy link
Contributor Author

Continued in #11257

@catenacyber catenacyber closed this Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants