The merge commit fixes reference counting issues introduced with the
changes to ListExpr::Assign() and IndexExpr::Assign(), but then also
several other pre-existing reference counting confusions in other
Assign() implementations/calls, some which were now directly observable
via new crashing behavior, others just from a cursory code audit.
* 'memleaks' of https://github.com/MaxKellermann/zeek:
input/Manager: fix memory leak in UnrollRecordType()
OpaqueVal: fix two memory leaks in BloomFilterVal::Merge()
DbgBreakpoint: fix memory leak
DebugCmds: fix memory leak
scan.l: fix crash bug in do_atif()
Expr: fix memory leak in RecordConstructorExpr::InitVal()
Expr: fix memory leaks in AssignExpr::EvalIntoAggregate()
Expr: fix memory leaks in CondExpr::Eval()
Expr: fix several memory leaks in BoolExpr::Eval()
Expr: fix various memory leaks in Assign()
Expr: fix memory leaks in BinaryExpr::Eval()
analyzer/protocol/http: fix potential memory leak
The `Val*` return by Expr::Eval() is never freed. Note that it must
be freed after the `Field` object has been created, because the
`secondary` variable points to data inside the `Val*`.
This is really a memory leak because the Unref() call is missing. But
since this usually returns a "stock" object (`ValManager::b_true` or
`ValManager::b_false`), nothing really leaks. But eventually, the
reference counter will overflow to `INT_MAX`, leading to a crash in
bad_ref().
No code path had any cleanup code, leaking all the local references.
More weird was however the result building code: it took elements from
one of the existing vectors without referencing them, and passed them
to VectorVal::Assign() which assumes that the caller-owned reference
is now owned by that VectorVal.
Even in the successful code path, no references were freed.
Everything was wrong with this method!
Copying a BroObj is dangerous, and should only be done with dedicated
(virtual) methods which are implemented by all derived classes. This
commit avoids unintentional copies.
If it were legal to call SendReplyOrRejectEvent() without an
EventHandlerPtr, then this would leak the `question_name` object. But
this method has just one caller, and it verifies the EventHandlerPtr.
If `dns_TSIG_addl` is not set, then the BroString allocated by
ExtractOctets() leaks. Therefore, don't ask ExtractOctets() to copy
the data to a BroString if we're not going to use it.
Yet another memory leak (out of way too many) which would have been
prevented by using smart pointers.
Using an overload that takes a va_list argument potentially causes
accidental misuse on platforms (e.g. 32-bit) where va_list is
implemented as a type that may collide with commonly-used argument
types.
For example:
char* c = copy_string("hi");
fmt("%s", (const char*)c);
fmt("%s", c);
The first fmt() call correctly goes through fmt(const char*, ...) first,
but the second mistakenly goes through fmt(const char*, va_list) first
because variadic function overloads have lower priority during overload
resolution and va_list on a 32-bit system happens to be defined as a
pointer type that can match with "char*" but not "const char*".
* MaxKellermann/includes:
broker: include cleanup
file_analysis: include cleanup
file_analysis/Analyzer: eliminate duplicate constructor
probabilistic/Topk: include cleanup
digest: eliminate the "Reporter.h" include
Val: eliminate the "RE.h" include
Val: eliminate the "BroString.h" include
Val: eliminate the "CompHash.h" include
Val: forward-declare class PDict, reduce includes
Val: eliminate the "Scope.h" include
* origin/topic/timw/dict-unit-tests:
Reset the number of entries in a dict when calling Clear()
Code cleanup in Dict.h
Add unit testing for the public Dictionary API
