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.

Sign up for GitHub

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

Jump to bottom

util/thash: decrease memuse if array was allocated #11425

Closed
wants to merge 1 commit into from
Closed

util/thash: decrease memuse if array was allocated #11425

wants to merge 1 commit into from

Conversation

catenacyber
Copy link
Contributor

Link to ticket: https://redmine.openinfosecfoundation.org/issues/
https://redmine.openinfosecfoundation.org/issues/7135

Describe changes:

  • util/thash: decrease memuse only if array was allocated

@catenacyber
util/thash: decrease memuse if array was allocated
e4c20d0 
THashInitConfig may not allocate array and increase memuse.
Such a failure leads to THashShutdown which should not decrease
the memuse.

Ticket: 7135
@catenacyber catenacyber requested a review from victorjulien as a code owner July 4, 2024 11:59
@victorjulien victorjulien requested a review from inashivb July 4, 2024 12:02
@catenacyber
Copy link
Contributor Author

cc @inashivb commit f1b44ca #11402

inashivb
inashivb approved these changes Jul 4, 2024
View reviewed changes
Copy link
Member

@inashivb inashivb left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

Please note that this was a pre-existing issue just discovered due to assertion check added in f1b44ca
Thank you! :)

@catenacyber
Copy link
Contributor Author

LGTM 🚀

Please note that this was a pre-existing issue just discovered due to assertion check added in f1b44ca Thank you! :)

Indeed 👍

@victorjulien victorjulien added this to the 8.0 milestone Jul 4, 2024
This was referenced Jul 4, 2024
Closed
Closed
@codecov Codecov
Copy link

codecov bot commented Jul 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.44%. Comparing base (a7af371) to head (e4c20d0).

Additional details and impacted files 
@@           Coverage Diff           @@
##           master   #11425   +/-   ##
=======================================
  Coverage   82.44%   82.44%           
=======================================
  Files         938      938           
  Lines      248086   248086           
=======================================
+ Hits       204534   204535    +1     
+ Misses      43552    43551    -1
Flag Coverage Δ
fuzzcorpus 60.10% <100.00%> (ø)
livemode 18.72% <100.00%> (-0.01%) ⬇️
pcap 43.72% <100.00%> (-0.07%) ⬇️
suricata-verify 61.43% <100.00%> (+0.02%) ⬆️
unittests 59.43% <100.00%> (-0.01%) ⬇️

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

@victorjulien victorjulien mentioned this pull request Jul 4, 2024
Merged
@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 21371

@victorjulien
Copy link
Member

Merged in #11431, thanks!

@catenacyber catenacyber mentioned this pull request Jul 7, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@inashivb inashivb inashivb approved these changes

@victorjulien victorjulien victorjulien approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
8.0
Development

Successfully merging this pull request may close these issues.
4 participants
@catenacyber @suricata-qa @victorjulien @inashivb