Basic Encoding Rules (BER) class for embedded system

I'm implementing a communications protocol in an embedded system and the protocol uses a subset of the ASN.1 BER (Basic Encoding Rules).

In a nutshell, this class simply represents a TLV triplet (Tag, Length, Value) in which the Tag is some label identifying the type of data, the Length is the length of the Value (in bytes) and the Value is simply a set of bytes.

Because it's going to be running on an embedded system, it will not be using any of the following:

• exceptions

• Runtime Type Identification (RTTI)

• most of the Standard Template Library (STL)

• any of the Boost library components

• C++14 (the compiler's not yet smart enough)

Additionally, there are some restrictions built into the project which are important for understanding the context of this class:

• all Tag values fit in a single 8-bit byte

• the maximum message size is 32k; the user of this class must assure this

• minimizing total memory footprint is very important

• assuring adequate performance (speed) is important

• the underlying microprocessor may be 16-bit, or 32-bit or 64-bit

• the underlying microprocessor may be little-endian or big-endian

• there is a macro (not shown) that correctly defines SHORT_IS_MORE_THAN_TWO_BYTES for the target architecture.

• the Ber class will not be derived from (which is why the destructor is not virtual)

I'm interested in a general review, but particularly for clarity and performance of the code. The test code is only for convenience on a computer; the embedded system has no iostream support.

bertest.cpp

```
#include
#include
#include "ber.h"

std::ostream& operator<<(std::ostream &out, const Ber &b)
{
unsigned short len = b.totallength(b.msglength());
out << "totallength = " << std::dec << len << '\n';
for (unsigned short i=0; i < len; ++i) {
out << std::setfill('0') << std::setw(2) <<
std::hex << (unsigned short)b[i] << ' ';
if (0xf == (i & 0xf)) out << '\n';
}
return out;
}

int

• You said Ber will not be derived from. Consider writing class Ber final{...} to make inheriting from Ber a compilation error.

• Remove useless comments such as //! constructor. Either people already know that from looking at the code or they do not understand the comment.

• Flesh out existing comments such as //! append the passed Ber data to the existing Ber object for operator +=. What is Ber data? The message? If you append data from one Ber-object to another, did you effectively concatenate the messages or replace them? If it is not totally obvious what a user defined operator does it is better to use a named member function instead.

• Ber(uint8_t tag, unsigned short length=0, const uint8_t message=nullptr); allows calling it like Ber ber(42, 8); which should be a compilation error since one should not have a nonzero length and nullptr as the message. You can fix that with minimal code duplication by writing 2 constructors, Ber(uint8_t tag, unsigned short length, const uint8_t message); and Ber(uint8_t tag) : Ber(tag, 0, nullptr){}.

• Your operator + behaves different than normal which is bad. Usually you have an expression such as a = b + c; where a gets a value and b and c are not changed. Your operator, however, changes b, so you could just write b + c; which looks like it has no effect, but instead it does the same as b += c;. My bad. Thanks to glampert for pointing it out.

• new [] goes together with delete [], you only use delete in your destructor. Consider using std::unique_ptr to manage the memory for you.

• Replace // assert(sizeof(unsigned short)>2); with something like static_assert(sizeof(unsigned short)>2, "Macro SHORT_IS_MORE_THAN_TWO_BYTES failed");.

• lencode will have a less scary and error prone implementation if you change the declaration of ptr to uint8 *&ptr.

I did not look at encodedlen, msglength and totallength in detail, but feel that it can be expressed simpler. Also without really checking I have a feeling that if you append a message and then make a copy, the appended message is gone.

StackExchange Code Review Q#66924, answer score: 2