diff --git a/CHANGES b/CHANGES index b75e8d8292..b7d85de927 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,40 @@ +3.2.0-dev.98 | 2020-02-21 20:04:30 -0800 + + * Val: use C++ initializers (Max Kellermann) + + * Val: add BroValUnion constructors (Max Kellermann) + + * Val: reduce duplicate code by using delegating constructors (Max Kellermann) + + * Val: remove unused default constructors and `friend` declarations (Max Kellermann) + + * Val: remove the unnecessary BroValUnion typedef (Max Kellermann) + + * Type: remove unnecessary enum typedefs (Max Kellermann) + + * Type: use C++ initializers (Max Kellermann) + + * Type: move code from BroType::BroType() to constexpr functions (Max Kellermann) + + Prepare to inline the constructor, which will one day be `constexpr` + (requires moving the `std::string name` field somewhere else). + + * Type: remove useless BroType destructor (Max Kellermann) + + * Obj: disallow copying BroObj (Max Kellermann) + + 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. + + * Obj: use C++ initializers (Max Kellermann) + + * Obj: make `no_location` constexpr (Max Kellermann) + + This ensures that the variable is initialized at compile time and may + allow the compiler to apply more initializations. + 3.2.0-dev.85 | 2020-02-21 15:29:45 -0800 * threading/MsgThread: add [[noreturn]] to InternalError() (Max Kellermann) diff --git a/VERSION b/VERSION index 10af60b395..27d09350e7 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -3.2.0-dev.85 +3.2.0-dev.98 diff --git a/src/Obj.cc b/src/Obj.cc index 91a9349d46..61ce530e08 100644 --- a/src/Obj.cc +++ b/src/Obj.cc @@ -10,7 +10,6 @@ #include "File.h" #include "plugin/Manager.h" -Location no_location("", 0, 0, 0, 0); Location start_location("", 0, 0, 0, 0); Location end_location("", 0, 0, 0, 0); diff --git a/src/Obj.h b/src/Obj.h index e50b705511..57b346efde 100644 --- a/src/Obj.h +++ b/src/Obj.h @@ -31,7 +31,7 @@ typedef Location yyltype; YYLTYPE GetCurrentLocation(); // Used to mean "no location associated with this object". -extern Location no_location; +inline constexpr Location no_location("", 0, 0, 0, 0); // Current start/end location. extern Location start_location; @@ -53,9 +53,6 @@ class BroObj { public: BroObj() { - ref_cnt = 1; - notify_plugins = false; - // A bit of a hack. We'd like to associate location // information with every object created when parsing, // since for them, the location is generally well-defined. @@ -76,6 +73,10 @@ public: virtual ~BroObj(); + /* disallow copying */ + BroObj(const BroObj &) = delete; + BroObj &operator=(const BroObj &) = delete; + // Report user warnings/errors. If obj2 is given, then it's // included in the message, though if pinpoint_only is non-zero, // then obj2 is only used to pinpoint the location. @@ -141,8 +142,8 @@ private: friend inline void Ref(BroObj* o); friend inline void Unref(BroObj* o); - bool notify_plugins; - int ref_cnt; + bool notify_plugins = false; + int ref_cnt = 1; // If non-zero, do not print runtime errors. Useful for // speculative evaluation. diff --git a/src/Type.cc b/src/Type.cc index f9bb36db01..88fffa4724 100644 --- a/src/Type.cc +++ b/src/Type.cc @@ -61,70 +61,10 @@ const char* type_name(TypeTag t) } BroType::BroType(TypeTag t, bool arg_base_type) + : tag(t), internal_tag(to_internal_type_tag(tag)), + is_network_order(::is_network_order(t)), + base_type(arg_base_type) { - tag = t; - is_network_order = 0; - base_type = arg_base_type; - - switch ( tag ) { - case TYPE_VOID: - internal_tag = TYPE_INTERNAL_VOID; - break; - - case TYPE_BOOL: - case TYPE_INT: - case TYPE_ENUM: - internal_tag = TYPE_INTERNAL_INT; - break; - - case TYPE_COUNT: - case TYPE_COUNTER: - internal_tag = TYPE_INTERNAL_UNSIGNED; - break; - - case TYPE_PORT: - internal_tag = TYPE_INTERNAL_UNSIGNED; - is_network_order = 1; - break; - - case TYPE_DOUBLE: - case TYPE_TIME: - case TYPE_INTERVAL: - internal_tag = TYPE_INTERNAL_DOUBLE; - break; - - case TYPE_STRING: - internal_tag = TYPE_INTERNAL_STRING; - break; - - case TYPE_ADDR: - internal_tag = TYPE_INTERNAL_ADDR; - break; - - case TYPE_SUBNET: - internal_tag = TYPE_INTERNAL_SUBNET; - break; - - case TYPE_PATTERN: - case TYPE_TIMER: - case TYPE_ANY: - case TYPE_TABLE: - case TYPE_UNION: - case TYPE_RECORD: - case TYPE_LIST: - case TYPE_FUNC: - case TYPE_FILE: - case TYPE_OPAQUE: - case TYPE_VECTOR: - case TYPE_TYPE: - internal_tag = TYPE_INTERNAL_OTHER; - break; - - case TYPE_ERROR: - internal_tag = TYPE_INTERNAL_ERROR; - break; - } - } BroType* BroType::ShallowClone() diff --git a/src/Type.h b/src/Type.h index 4ea3caf318..740fdbe330 100644 --- a/src/Type.h +++ b/src/Type.h @@ -14,7 +14,7 @@ // BRO types. -typedef enum { +enum TypeTag { TYPE_VOID, // 0 TYPE_BOOL, // 1 TYPE_INT, // 2 @@ -42,20 +42,77 @@ typedef enum { TYPE_TYPE, // 24 TYPE_ERROR // 25 #define NUM_TYPES (int(TYPE_ERROR) + 1) -} TypeTag; +}; -typedef enum { +constexpr bool is_network_order(TypeTag tag) noexcept + { + return tag == TYPE_PORT; + } + +enum function_flavor { FUNC_FLAVOR_FUNCTION, FUNC_FLAVOR_EVENT, FUNC_FLAVOR_HOOK -} function_flavor; +}; -typedef enum { +enum InternalTypeTag { TYPE_INTERNAL_VOID, TYPE_INTERNAL_INT, TYPE_INTERNAL_UNSIGNED, TYPE_INTERNAL_DOUBLE, TYPE_INTERNAL_STRING, TYPE_INTERNAL_ADDR, TYPE_INTERNAL_SUBNET, TYPE_INTERNAL_OTHER, TYPE_INTERNAL_ERROR -} InternalTypeTag; +}; + +constexpr InternalTypeTag to_internal_type_tag(TypeTag tag) noexcept + { + switch ( tag ) { + case TYPE_VOID: + return TYPE_INTERNAL_VOID; + + case TYPE_BOOL: + case TYPE_INT: + case TYPE_ENUM: + return TYPE_INTERNAL_INT; + + case TYPE_COUNT: + case TYPE_COUNTER: + case TYPE_PORT: + return TYPE_INTERNAL_UNSIGNED; + + case TYPE_DOUBLE: + case TYPE_TIME: + case TYPE_INTERVAL: + return TYPE_INTERNAL_DOUBLE; + + case TYPE_STRING: + return TYPE_INTERNAL_STRING; + + case TYPE_ADDR: + return TYPE_INTERNAL_ADDR; + + case TYPE_SUBNET: + return TYPE_INTERNAL_SUBNET; + + case TYPE_PATTERN: + case TYPE_TIMER: + case TYPE_ANY: + case TYPE_TABLE: + case TYPE_UNION: + case TYPE_RECORD: + case TYPE_LIST: + case TYPE_FUNC: + case TYPE_FILE: + case TYPE_OPAQUE: + case TYPE_VECTOR: + case TYPE_TYPE: + return TYPE_INTERNAL_OTHER; + + case TYPE_ERROR: + return TYPE_INTERNAL_ERROR; + } + + /* this should be unreachable */ + return TYPE_INTERNAL_VOID; + } // Returns the name of the type. extern const char* type_name(TypeTag t); @@ -83,7 +140,6 @@ const int MATCHES_INDEX_VECTOR = 2; class BroType : public BroObj { public: explicit BroType(TypeTag tag, bool base_type = false); - ~BroType() override { } // Performs a shallow clone operation of the Bro type. // This especially means that especially for tables the types diff --git a/src/Val.cc b/src/Val.cc index 66462e5e79..4f7fee8333 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -38,29 +38,23 @@ #include "threading/formatters/JSON.h" Val::Val(Func* f) + : val(f), type(f->FType()->Ref()) { - val.func_val = f; ::Ref(val.func_val); - type = f->FType()->Ref(); -#ifdef DEBUG - bound_id = 0; -#endif } -Val::Val(BroFile* f) +static FileType* GetStringFileType() noexcept { static FileType* string_file_type = 0; if ( ! string_file_type ) string_file_type = new FileType(base_type(TYPE_STRING)); + return string_file_type; + } - val.file_val = f; - +Val::Val(BroFile* f) + : val(f), type(GetStringFileType()->Ref()) + { assert(f->FType()->Tag() == TYPE_STRING); - type = string_file_type->Ref(); - -#ifdef DEBUG - bound_id = 0; -#endif } Val::~Val() @@ -771,9 +765,8 @@ uint32_t PortVal::Mask(uint32_t port_num, TransportProto port_type) return port_num; } -PortVal::PortVal(uint32_t p) : Val(TYPE_PORT) +PortVal::PortVal(uint32_t p) : Val(bro_uint_t(p), TYPE_PORT) { - val.uint_val = static_cast(p); } uint32_t PortVal::Port() const @@ -823,30 +816,25 @@ Val* PortVal::DoClone(CloneState* state) return Ref(); } -AddrVal::AddrVal(const char* text) : Val(TYPE_ADDR) +AddrVal::AddrVal(const char* text) : Val(new IPAddr(text), TYPE_ADDR) { - val.addr_val = new IPAddr(text); } -AddrVal::AddrVal(const std::string& text) : Val(TYPE_ADDR) +AddrVal::AddrVal(const std::string& text) : AddrVal(text.c_str()) { - val.addr_val = new IPAddr(text); } -AddrVal::AddrVal(uint32_t addr) : Val(TYPE_ADDR) +AddrVal::AddrVal(uint32_t addr) : Val(new IPAddr(IPv4, &addr, IPAddr::Network), TYPE_ADDR) { // ### perhaps do gethostbyaddr here? - val.addr_val = new IPAddr(IPv4, &addr, IPAddr::Network); } -AddrVal::AddrVal(const uint32_t addr[4]) : Val(TYPE_ADDR) +AddrVal::AddrVal(const uint32_t addr[4]) : Val(new IPAddr(IPv6, addr, IPAddr::Network), TYPE_ADDR) { - val.addr_val = new IPAddr(IPv6, addr, IPAddr::Network); } -AddrVal::AddrVal(const IPAddr& addr) : Val(TYPE_ADDR) +AddrVal::AddrVal(const IPAddr& addr) : Val(new IPAddr(addr), TYPE_ADDR) { - val.addr_val = new IPAddr(addr); } AddrVal::~AddrVal() @@ -873,39 +861,30 @@ Val* AddrVal::DoClone(CloneState* state) return Ref(); } -SubNetVal::SubNetVal(const char* text) : Val(TYPE_SUBNET) +SubNetVal::SubNetVal(const char* text) : Val(new IPPrefix(), TYPE_SUBNET) { - val.subnet_val = new IPPrefix(); - if ( ! IPPrefix::ConvertString(text, val.subnet_val) ) reporter->Error("Bad string in SubNetVal ctor: %s", text); } -SubNetVal::SubNetVal(const char* text, int width) : Val(TYPE_SUBNET) +SubNetVal::SubNetVal(const char* text, int width) : Val(new IPPrefix(text, width), TYPE_SUBNET) { - val.subnet_val = new IPPrefix(text, width); } -SubNetVal::SubNetVal(uint32_t addr, int width) : Val(TYPE_SUBNET) +SubNetVal::SubNetVal(uint32_t addr, int width) : SubNetVal(IPAddr{IPv4, &addr, IPAddr::Network}, width) { - IPAddr a(IPv4, &addr, IPAddr::Network); - val.subnet_val = new IPPrefix(a, width); } -SubNetVal::SubNetVal(const uint32_t* addr, int width) : Val(TYPE_SUBNET) +SubNetVal::SubNetVal(const uint32_t* addr, int width) : SubNetVal(IPAddr{IPv6, addr, IPAddr::Network}, width) { - IPAddr a(IPv6, addr, IPAddr::Network); - val.subnet_val = new IPPrefix(a, width); } -SubNetVal::SubNetVal(const IPAddr& addr, int width) : Val(TYPE_SUBNET) +SubNetVal::SubNetVal(const IPAddr& addr, int width) : Val(new IPPrefix(addr, width), TYPE_SUBNET) { - val.subnet_val = new IPPrefix(addr, width); } -SubNetVal::SubNetVal(const IPPrefix& prefix) : Val(TYPE_SUBNET) +SubNetVal::SubNetVal(const IPPrefix& prefix) : Val(new IPPrefix(prefix), TYPE_SUBNET) { - val.subnet_val = new IPPrefix(prefix); } SubNetVal::~SubNetVal() @@ -979,25 +958,22 @@ Val* SubNetVal::DoClone(CloneState* state) return Ref(); } -StringVal::StringVal(BroString* s) : Val(TYPE_STRING) +StringVal::StringVal(BroString* s) : Val(s, TYPE_STRING) { - val.string_val = s; } -StringVal::StringVal(int length, const char* s) : Val(TYPE_STRING) +// The following adds a NUL at the end. +StringVal::StringVal(int length, const char* s) + : StringVal(new BroString(reinterpret_cast(s), length, 1)) { - // The following adds a NUL at the end. - val.string_val = new BroString((const u_char*) s, length, 1); } -StringVal::StringVal(const char* s) : Val(TYPE_STRING) +StringVal::StringVal(const char* s) : StringVal(new BroString(s)) { - val.string_val = new BroString(s); } -StringVal::StringVal(const string& s) : Val(TYPE_STRING) +StringVal::StringVal(const string& s) : StringVal(s.length(), s.data()) { - val.string_val = new BroString(reinterpret_cast(s.data()), s.length(), 1); } Val* StringVal::SizeVal() const diff --git a/src/Val.h b/src/Val.h index 79172c3f55..5bd3e2e49a 100644 --- a/src/Val.h +++ b/src/Val.h @@ -61,7 +61,7 @@ class TableEntryVal; class RE_Matcher; -typedef union { +union BroValUnion { // Used for bool, int, enum. bro_int_t int_val; @@ -86,17 +86,50 @@ typedef union { vector* vector_val; -} BroValUnion; + BroValUnion() = default; + + constexpr BroValUnion(bro_int_t value) noexcept + : int_val(value) {} + + constexpr BroValUnion(bro_uint_t value) noexcept + : uint_val(value) {} + + constexpr BroValUnion(IPAddr* value) noexcept + : addr_val(value) {} + + constexpr BroValUnion(IPPrefix* value) noexcept + : subnet_val(value) {} + + constexpr BroValUnion(double value) noexcept + : double_val(value) {} + + constexpr BroValUnion(BroString* value) noexcept + : string_val(value) {} + + constexpr BroValUnion(Func* value) noexcept + : func_val(value) {} + + constexpr BroValUnion(BroFile* value) noexcept + : file_val(value) {} + + constexpr BroValUnion(RE_Matcher* value) noexcept + : re_val(value) {} + + constexpr BroValUnion(PDict* value) noexcept + : table_val(value) {} + + constexpr BroValUnion(val_list* value) noexcept + : val_list_val(value) {} + + constexpr BroValUnion(vector *value) noexcept + : vector_val(value) {} +}; class Val : public BroObj { public: Val(double d, TypeTag t) + : val(d), type(base_type(t)) { - val.double_val = d; - type = base_type(t); -#ifdef DEBUG - bound_id = 0; -#endif } explicit Val(Func* f); @@ -105,21 +138,15 @@ public: // class has ref'd it. explicit Val(BroFile* f); - Val(BroType* t, bool type_type) // Extra arg to differentiate from protected version. + // Extra arg to differentiate from protected version. + Val(BroType* t, bool type_type) + : type(new TypeType(t->Ref())) { - type = new TypeType(t->Ref()); -#ifdef DEBUG - bound_id = 0; -#endif } Val() + : val(bro_int_t(0)), type(base_type(TYPE_ERROR)) { - val.int_val = 0; - type = base_type(TYPE_ERROR); -#ifdef DEBUG - bound_id = 0; -#endif } ~Val() override; @@ -312,39 +339,34 @@ protected: static Val* MakeBool(bool b) { - auto rval = new Val(TYPE_BOOL); - rval->val.int_val = b; - return rval; + return new Val(bro_int_t(b), TYPE_BOOL); } static Val* MakeInt(bro_int_t i) { - auto rval = new Val(TYPE_INT); - rval->val.int_val = i; - return rval; + return new Val(i, TYPE_INT); } static Val* MakeCount(bro_uint_t u) { - auto rval = new Val(TYPE_COUNT); - rval->val.uint_val = u; - return rval; + return new Val(u, TYPE_COUNT); } - explicit Val(TypeTag t) + template + Val(V &&v, TypeTag t) noexcept + : val(std::forward(v)), type(base_type(t)) + { + } + + template + Val(V &&v, BroType* t) noexcept + : val(std::forward(v)), type(t->Ref()) { - type = base_type(t); -#ifdef DEBUG - bound_id = 0; -#endif } explicit Val(BroType* t) + : type(t->Ref()) { - type = t->Ref(); -#ifdef DEBUG - bound_id = 0; -#endif } ACCESSOR(TYPE_TABLE, PDict*, table_val, AsNonConstTable) @@ -372,7 +394,7 @@ protected: #ifdef DEBUG // For debugging, we keep the name of the ID to which a Val is bound. - const char* bound_id; + const char* bound_id = nullptr; #endif }; @@ -479,9 +501,7 @@ public: static uint32_t Mask(uint32_t port_num, TransportProto port_type); protected: - friend class Val; friend class ValManager; - PortVal() {} PortVal(uint32_t p); void ValDescribe(ODesc* d) const override; @@ -504,11 +524,6 @@ public: unsigned int MemoryAllocation() const override; protected: - friend class Val; - AddrVal() {} - explicit AddrVal(TypeTag t) : Val(t) { } - explicit AddrVal(BroType* t) : Val(t) { } - Val* DoClone(CloneState* state) override; }; @@ -533,9 +548,6 @@ public: unsigned int MemoryAllocation() const override; protected: - friend class Val; - SubNetVal() {} - void ValDescribe(ODesc* d) const override; Val* DoClone(CloneState* state) override; }; @@ -566,9 +578,6 @@ public: Val* Substitute(RE_Matcher* re, StringVal* repl, bool do_all); protected: - friend class Val; - StringVal() {} - void ValDescribe(ODesc* d) const override; Val* DoClone(CloneState* state) override; }; @@ -585,9 +594,6 @@ public: unsigned int MemoryAllocation() const override; protected: - friend class Val; - PatternVal() {} - void ValDescribe(ODesc* d) const override; Val* DoClone(CloneState* state) override; }; @@ -630,9 +636,6 @@ public: unsigned int MemoryAllocation() const override; protected: - friend class Val; - ListVal() {} - Val* DoClone(CloneState* state) override; val_list vals; @@ -819,9 +822,6 @@ public: notifier::Modifiable* Modifiable() override { return this; } protected: - friend class Val; - TableVal() {} - void Init(TableType* t); void CheckExpireAttr(attr_tag at); @@ -925,9 +925,6 @@ public: static void ResizeParseTimeRecords(); protected: - friend class Val; - RecordVal() {} - Val* DoClone(CloneState* state) override; BroObj* origin; @@ -943,13 +940,10 @@ protected: friend class Val; friend class EnumType; - EnumVal(EnumType* t, int i) : Val(t) + EnumVal(EnumType* t, int i) : Val(bro_int_t(i), t) { - val.int_val = i; } - EnumVal() {} - void ValDescribe(ODesc* d) const override; Val* DoClone(CloneState* state) override; }; @@ -1012,9 +1006,6 @@ public: bool Remove(unsigned int index); protected: - friend class Val; - VectorVal() { } - void ValDescribe(ODesc* d) const override; Val* DoClone(CloneState* state) override;