Skip to content

Comments

Dataset string memcap 3910/v6#11124

Closed
inashivb wants to merge 4 commits intoOISF:masterfrom
inashivb:dataset-string-memcap-3910/v6
Closed

Dataset string memcap 3910/v6#11124
inashivb wants to merge 4 commits intoOISF:masterfrom
inashivb:dataset-string-memcap-3910/v6

Conversation

@inashivb
Copy link
Member

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

Previous PR: #11105

Changes since v5:

  • doc suggestion taken into account
  • reorganize commits better so it requires less changes while still making sense logically
  • rebased
  • minor cleanups whitespaces et al.

victorjulien and others added 4 commits May 23, 2024 15:51
Add a callback and helper function to handle data expiration.

Update datasets to explicitly not use expiration.
In order to have access to the length of datatypes with variable lengths
to correctly update memuse to calculate memcaps.

Bug 3910
So far, when the data size was passed to the THash API, it was sent as
a sizeof(Struct) which works fine for the other data types as they have
a fixed length but not for the StringType.
However, because of the sizeof construct, the length of a string type
dataset was always taken to be 16 Bytes which is only the size of the struct
itself. It did not accomodate the actual size of the string that the
StringType holds. Fix this so that the memuse that is used to determine
whether memcap was reached also takes into consideration the size of the
actual string.

Bug 3910
@codecov
Copy link

codecov bot commented May 23, 2024

Codecov Report

Attention: Patch coverage is 43.66197% with 40 lines in your changes are missing coverage. Please review.

Project coverage is 84.27%. Comparing base (b3eb1c4) to head (4d10ff9).

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #11124      +/-   ##
==========================================
+ Coverage   84.26%   84.27%   +0.01%     
==========================================
  Files         926      926              
  Lines      247387   247444      +57     
==========================================
+ Hits       208453   208540      +87     
+ Misses      38934    38904      -30     
Flag Coverage Δ
fuzzcorpus 64.17% <42.25%> (-0.02%) ⬇️
livemode 19.57% <22.53%> (-0.01%) ⬇️
pcap 46.48% <18.30%> (+0.01%) ⬆️
suricata-verify 63.12% <43.66%> (+0.01%) ⬆️
unittests 61.94% <0.00%> (-0.02%) ⬇️

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

@ct0br0
Copy link

ct0br0 commented May 23, 2024

qa tokens ran out. ignore the 'pipeline canceled' incoming

@inashivb inashivb marked this pull request as ready for review May 23, 2024 12:05
@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 20779

Copy link
Contributor

@jufajardini jufajardini left a comment

Choose a reason for hiding this comment

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

Doc change looks good to me :P

@inashivb inashivb closed this May 29, 2024
@inashivb inashivb deleted the dataset-string-memcap-3910/v6 branch May 29, 2024 04:01
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.

5 participants