Fix clang-tidy findings in the binpac lib code

tools/binpac/lib/binpac.h.in

@@ -9,16 +9,16 @@
 
 #cmakedefine HOST_BIGENDIAN
 #ifdef HOST_BIGENDIAN
-#  define HOST_BYTEORDER	bigendian
+#define HOST_BYTEORDER bigendian
 #else
-#  define HOST_BYTEORDER	littleendian
+#define HOST_BYTEORDER littleendian
 #endif
 
-#include <assert.h>
+#include <cassert>
-#include <stdarg.h>
+#include <cstdarg>
-#include <stdio.h>
+#include <cstdint>
+#include <cstdio>
 #include <string>
-#include <memory>
 
 // Expose C99 functionality from inttypes.h, which would otherwise not be
 // available in C++.
@@ -26,9 +26,7 @@
 #define __STDC_FORMAT_MACROS
 #endif
 
-#include <inttypes.h>
+static constexpr void BINPAC_ASSERT(bool val) { assert(val); }
-
-#define BINPAC_ASSERT(x)	assert(x)
 
 using namespace std;
 
@@ -41,23 +39,21 @@ const int unspecified_byteorder = -1;
 #ifndef pac_type_defs
 #define pac_type_defs
 
-typedef int8_t		int8;
+using int8 = int8_t;
-typedef int16_t		int16;
+using int16 = int16_t;
-typedef int32_t		int32;
+using int32 = int32_t;
-typedef int64_t		int64;
+using int64 = int64_t;
-typedef uint8_t		uint8;
+using uint8 = uint8_t;
-typedef uint16_t	uint16;
+using uint16 = uint16_t;
-typedef uint32_t	uint32;
+using uint32 = uint32_t;
-typedef uint64_t	uint64;
+using uint64 = uint64_t;
-typedef void		*nulptr;
+using nulptr = void*;
-typedef void		*voidptr;
+using voidptr = void*;
-typedef uint8		*byteptr;
+using byteptr = uint8*;
-typedef const uint8	*const_byteptr;
+using const_byteptr = const uint8*;
-typedef const char	*const_charptr;
+using const_charptr = const char*;
 
-#if @SIZEOF_UNSIGNED_INT@ != 4
+static_assert(sizeof(unsigned int) == 4, "Unexpected size of unsigned int");
-#error "unexpected size of unsigned int"
-#endif
 
 #endif /* pac_type_defs */
 
@@ -65,106 +61,93 @@ typedef const char	*const_charptr;
 
 namespace {
 
-inline uint16 pac_swap(const uint16 x)
+inline uint16 pac_swap(const uint16 x) { return (x >> 8) | ((x & 0xff) << 8); }
-	{
-	return (x >> 8) | ((x & 0xff) << 8);
-	}
 
-inline int16 pac_swap(const int16 x)
+inline int16 pac_swap(const int16 x) {
-	{
+    // Forward to unsigned version with argument/result casted
-	// Forward to unsigned version with argument/result casted
+    // appropriately.
-	// appropriately.
+    uint16 (*p)(const uint16) = &pac_swap;
-	uint16 (*p)(const uint16) = &pac_swap;
+    return (*p)(x);
-	return (*p)(x);
+}
-	}
 
-inline uint32 pac_swap(const uint32 x)
+inline uint32 pac_swap(const uint32 x) {
-	{
+    return (x >> 24) | ((x & 0xff0000) >> 8) | ((x & 0xff00) << 8) | ((x & 0xff) << 24);
-	return (x >> 24) |
+}
-	  ((x & 0xff0000) >> 8) |
-	  ((x & 0xff00) << 8) |
-	  ((x & 0xff) << 24);
-	}
 
-inline int32 pac_swap(const int32 x)
+inline int32 pac_swap(const int32 x) {
-	{
+    // Forward to unsigned version with argument/result casted
-	// Forward to unsigned version with argument/result casted
+    // appropriately.
-	// appropriately.
+    uint32 (*p)(const uint32) = &pac_swap;
-	uint32 (*p)(const uint32) = &pac_swap;
+    return (*p)(x);
-	return (*p)(x);
+}
-	}
 
-inline uint64 pac_swap(const uint64 x) 
+inline uint64 pac_swap(const uint64 x) {
-	{
+    return x >> 56 | (x & 0xff000000000000) >> 40 | (x & 0xff0000000000) >> 24 | (x & 0xff00000000) >> 8 |
-	return x >> 56 |
+           (x & 0xff000000) << 8 | (x & 0xff0000) << 24 | (x & 0xff00) << 40 | (x & 0xff) << 56;
-	  (x & 0xff000000000000) >> 40 |
+}
-	  (x & 0xff0000000000) >> 24 |
-	  (x & 0xff00000000) >> 8 |
-	  (x & 0xff000000) << 8 |
-	  (x & 0xff0000) << 24 |
-	  (x & 0xff00) << 40 |
-	  (x & 0xff) << 56;
-	}
 
-inline int64 pac_swap(const int64 x)
+inline int64 pac_swap(const int64 x) {
-	{
+    // Forward to unsigned version with argument/result casted
-	// Forward to unsigned version with argument/result casted
+    // appropriately.
-	// appropriately.
+    uint64 (*p)(const uint64) = &pac_swap;
-	uint64 (*p)(const uint64) = &pac_swap;
+    return (*p)(x);
-	return (*p)(x);
+}
-	}
 
-#define FixByteOrder(byteorder, x)	(byteorder == HOST_BYTEORDER ? (x) : pac_swap(x))
+template<typename T>
+static constexpr T FixByteOrder(int byteorder, T x) {
+    if ( byteorder == HOST_BYTEORDER )
+        return x;
 
-template <class T>
+    return static_cast<T>(pac_swap(x));
-inline T UnMarshall(const unsigned char *data, int byteorder)
+}
-	{
-	T result = 0;
-	for ( int i = 0; i < (int) sizeof(T); ++i )
-		result = ( result << 8 ) | 
-			data[byteorder == bigendian ? i : sizeof(T) - 1 - i];
-	return result;
-	}
 
-inline const char* do_fmt(const char* format, va_list ap)
+template<class T>
-	{
+inline T UnMarshall(const unsigned char* data, int byteorder) {
-	static char buf[1024];
+    T result = 0;
-	vsnprintf(buf, sizeof(buf), format, ap);
+    for ( int i = 0; i < (int)sizeof(T); ++i )
-	return buf;
+        result = (result << 8) | data[byteorder == bigendian ? i : sizeof(T) - 1 - i];
-	}
+    return result;
+}
 
-inline string strfmt(const char* format, ...)
+inline const char* do_fmt(const char* format, va_list ap) {
-	{
+    static char buf[1024];
-	va_list ap;
+    vsnprintf(buf, sizeof(buf), format, ap);
-	va_start(ap, format);
+    return buf;
-	const char* r = do_fmt(format, ap);
+}
-	va_end(ap);
+
-	return string(r);
+inline string strfmt(const char* format, ...) {
-	}
+    va_list ap;
+    va_start(ap, format);
+    const char* r = do_fmt(format, ap);
+    va_end(ap);
+    return {r};
+}
 
 } // anonymous namespace
 
+// NOLINTNEXTLINE(cppcoreguidelines-macro-usage)
 #define binpac_fmt(...) strfmt(__VA_ARGS__).c_str()
 
-class RefCount
+class RefCount {
-{
 public:
-	RefCount() 	{ count = 1; }
+    RefCount() { count = 1; }
-	virtual ~RefCount() {}
+    virtual ~RefCount() {}
-	void Ref() 	{ ++count; }
+    void Ref() { ++count; }
-	int Unref() 	{ BINPAC_ASSERT(count > 0); return --count; }
+    int Unref() {
+        BINPAC_ASSERT(count > 0);
+        return --count;
+    }
 
 private:
-	int count;
+    int count;
 };
 
 namespace {
-	inline void Unref(RefCount *x)
+inline void Unref(RefCount* x) {
-		{
+    if ( x && x->Unref() <= 0 )
-		if ( x && x->Unref() <= 0 )
+        delete x;
-			delete x;
+}
-		}
+} // anonymous namespace
-}  // anonymous namespace
 
 } // namespace binpac
 

tools/binpac/lib/binpac_buffer.h

@@ -17,7 +17,7 @@ public:
         int contract_threshold;
     };
 
-    enum LineBreakStyle {
+    enum LineBreakStyle : uint8_t {
         CR_OR_LF,     // CR or LF or CRLF
         STRICT_CRLF,  // CR followed by LF
         CR_LF_NUL,    // CR or LF or CR-LF or CR-NUL
@@ -142,13 +142,13 @@ protected:
     LineBreakStyle linebreak_style_default;
     unsigned char linebreaker_;
 
-    enum {
+    enum : uint8_t {
         UNKNOWN_MODE,
         LINE_MODE,
         FRAME_MODE,
     } mode_;
 
-    enum {
+    enum : uint8_t {
         CR_OR_LF_0,
         CR_OR_LF_1,
         STRICT_CRLF_0,
@@ -163,7 +163,7 @@ protected:
     static Policy policy;
 };
 
-typedef FlowBuffer* flow_buffer_t;
+using flow_buffer_t = FlowBuffer*;
 
 } // namespace binpac
 

tools/binpac/lib/binpac_bytestring.h

@@ -3,7 +3,7 @@
 #ifndef binpac_bytestring_h
 #define binpac_bytestring_h
 
-#include <string.h>
+#include <cstring>
 #include <string>
 
 #include "binpac.h"
@@ -16,7 +16,7 @@ class datastring;
 template<class T>
 class const_datastring {
 public:
-    const_datastring() : begin_(0), end_(0) {}
+    const_datastring() : begin_(nullptr), end_(nullptr) {}
 
     const_datastring(T const* data, int length) : begin_(data), end_(data + length) {}
 
@@ -49,7 +49,7 @@ private:
     T const* end_;
 };
 
-typedef const_datastring<uint8> const_bytestring;
+using const_bytestring = const_datastring<uint8>;
 
 template<class T>
 class datastring {
@@ -65,6 +65,9 @@ public:
     explicit datastring(const_datastring<T> const& x) { set_const(x.begin(), x.length()); }
 
     datastring const& operator=(datastring<T> const& x) {
+        if ( this == &x )
+            return *this;
+
         BINPAC_ASSERT(! data_);
         set(x.data(), x.length());
         return *this;
@@ -76,7 +79,7 @@ public:
     }
 
     void clear() {
-        data_ = 0;
+        data_ = nullptr;
         length_ = 0;
     }
 
@@ -119,13 +122,11 @@ private:
     int length_;
 };
 
-typedef datastring<uint8> bytestring;
+using bytestring = datastring<uint8>;
 
 inline const char* c_str(bytestring const& s) { return (const char*)s.begin(); }
 
-inline std::string std_str(const_bytestring const& s) {
+inline std::string std_str(const_bytestring const& s) { return {(const char*)s.begin(), (const char*)s.end()}; }
-    return std::string((const char*)s.begin(), (const char*)s.end());
-}
 
 inline bool operator==(bytestring const& s1, const char* s2) { return strcmp(c_str(s1), s2) == 0; }
 

tools/binpac/lib/binpac_exception.h

@@ -3,25 +3,26 @@
 #ifndef binpac_exception_h
 #define binpac_exception_h
 
-#include <inttypes.h>
+#include <cinttypes>
-#include <stdint.h>
+#include <cstdint>
+#include <string>
 
 namespace binpac {
 
 class Exception {
 public:
-    Exception(const char* m = 0) : msg_("binpac exception: ") {
+    Exception(const char* m = nullptr) : msg_("binpac exception: ") {
         if ( m )
             append(m);
         // abort();
     }
 
-    void append(string m) { msg_ += m; }
+    void append(std::string m) { msg_ += m; }
-    string msg() const { return msg_; }
+    std::string msg() const { return msg_; }
     const char* c_msg() const { return msg_.c_str(); }
 
 protected:
-    string msg_;
+    std::string msg_;
 };
 
 class ExceptionEnforceViolation : public Exception {
@@ -46,7 +47,7 @@ public:
 protected:
     const char* location_;
     int64_t index_;
-    string expected_;
+    std::string expected_;
 };
 
 class ExceptionInvalidCaseIndex : public Exception {

tools/binpac/lib/binpac_regex.h

@@ -1,6 +1,10 @@
+// See the file "COPYING" in the main distribution directory for copyright.
+
 #ifndef binpac_regex_h
 #define binpac_regex_h
 
+#include <vector>
+
 #include "zeek/RE.h"
 
 #include "binpac.h"
@@ -16,7 +20,7 @@ namespace binpac {
 // Note, this must be declared/defined here, and inline, because the RE
 // functionality can only be used when compiling from inside Zeek.
 // A copy is made of any FlowBuffer policy struct data passed.
-inline void init(FlowBuffer::Policy* fbp = 0);
+inline void init(FlowBuffer::Policy* fbp = nullptr);
 
 // Internal vector recording not yet compiled matchers.
 extern std::vector<zeek::RE_Matcher*>* uncompiled_re_matchers;
@@ -50,8 +54,8 @@ inline void RegExMatcher::init() {
     if ( ! uncompiled_re_matchers )
         return;
 
-    for ( size_t i = 0; i < uncompiled_re_matchers->size(); ++i ) {
+    for ( const auto& matcher : *uncompiled_re_matchers ) {
-        if ( ! (*uncompiled_re_matchers)[i]->Compile() ) {
+        if ( ! matcher->Compile() ) {
             fprintf(stderr, "binpac: cannot compile regular expression\n");
             exit(1);
         }

tools/binpac/src/pac_main.cc

@@ -176,9 +176,6 @@ int compile(const char* filename) {
 }
 
 void usage() {
-#ifdef BINPAC_VERSION
-    fprintf(stderr, "binpac version %s\n", BINPAC_VERSION);
-#endif
     fprintf(stderr, "usage: binpac [options] <pac files>\n");
     fprintf(stderr, "     <pac files>           | pac-language input files\n");
     fprintf(stderr, "     -d <dir>              | use given directory for compiler output\n");