Project

General

Profile

Actions

Optimization #1046

closed

replace pcre_get_substring with pcre_copy_substring

Added by Victor Julien almost 9 years ago. Updated over 1 year ago.

Status:
Closed
Priority:
Normal
Target version:
Effort:
Difficulty:
Label:

Description

The get variant returns newly alloc'd memory, which is unnecessary most of the time.

We should replace the get usage in all rule parsing code.


Related issues 1 (0 open1 closed)

Related to Bug #3566: rules: minor memory leak involving pcre_get_substringClosedJeff LucovskyActions
Actions #1

Updated by Victor Julien almost 9 years ago

  • Priority changed from Normal to Low
  • Target version changed from 2.0beta2 to 2.0rc1
  • % Done changed from 10 to 20

There are still quite a few places where this is used, the hot spots have been addressed.

Actions #2

Updated by Victor Julien over 8 years ago

  • Target version changed from 2.0rc1 to 3.0RC2
Actions #3

Updated by Victor Julien over 7 years ago

  • Target version changed from 3.0RC2 to 70
Actions #4

Updated by Victor Julien over 6 years ago

  • Assignee changed from Victor Julien to Andreas Herz
  • Priority changed from Low to Normal

Fixes part of the leaks listed here: #1659#note-1

Actions #5

Updated by Andreas Herz over 6 years ago

You changed several spots but you left "SC_ERR_PCRE_GET_SUBSTRING" for SCLogError, is this intended?

I also saw the discussion at https://github.com/inliniac/suricata/pull/900 so is there any usecase where we have some sort of advantage when we still use pcre_get_substring?

Actions #6

Updated by Victor Julien over 6 years ago

Andreas Herz wrote:

You changed several spots but you left "SC_ERR_PCRE_GET_SUBSTRING" for SCLogError, is this intended?

I didn't really care about being that specific, but feel free to add one for copy as well (append to enum to keep existing values the contant)

I also saw the discussion at https://github.com/inliniac/suricata/pull/900 so is there any usecase where we have some sort of advantage when we still use pcre_get_substring?

Yeah if you need a new allocation of what the regex captures then _get_substring would be OK.

Actions #7

Updated by Andreas Herz about 4 years ago

  • Assignee changed from Andreas Herz to Anonymous
Actions #8

Updated by Andreas Herz about 4 years ago

  • Status changed from Assigned to New
Actions #9

Updated by Andreas Herz over 3 years ago

  • Assignee set to Community Ticket
Actions #10

Updated by Victor Julien over 3 years ago

These files are left:

detect-base64-decode.c
detect-bytetest.c
detect-dce-opnum.c
detect-detection-filter.c
detect-fast-pattern.c
detect-filesize.c
detect-filestore.c
detect-flags.c
detect-flowint.c
detect-flowvar.c
detect-fragbits.c
detect-fragoffset.c
detect-icmp-id.c
detect-icmp-seq.c
detect-icode.c
detect-ipproto.c
detect-iprep.c
detect-isdataat.c
detect-itype.c
detect-mark.c
detect-modbus.c
detect-pcre.c
detect-pktvar.c
detect-reference.c
detect-rpc.c
detect-ssh-proto-version.c
detect-ssh-software-version.c
detect-ssl-state.c
detect-ssl-version.c
detect-stream_size.c
detect-tag.c
detect-target.c
detect-template2.c
detect-threshold.c
detect-tls.c
detect-tls-version.c
detect-urilen.c
util-host-info.c
util-threshold-config.c

Of these, threshold, tag, urilen, reference, isdataat, bytetest and flags are the most commonly used.

Actions #11

Updated by Victor Julien over 2 years ago

  • Related to Bug #3566: rules: minor memory leak involving pcre_get_substring added
Actions #12

Updated by Victor Julien about 2 years ago

  • Target version changed from 70 to TBD
Actions #13

Updated by Philippe Antoine over 1 year ago

  • Status changed from New to Closed

Replaced by #3194

Actions

Also available in: Atom PDF