I have started making a very basic networking library in C++. It is built on UDP, with both reliable and unreliable delivery options.So far, I have made the base messaging class, so I will show the header, and the code behind a few functions.Things to note:While this is thread safe, I'm not actually expecting this to be widely parallelized, hence the atomic variable lock, and not a mutex. There may be contention, but it will be in extremely rare cases.The code you see is not the complete object.I would primarily like to hear of any logical flaws, or coding errors. The functionality depicted is about all I will need. This library is for my own use, not for public, but I would still like to hear short notes of best practices for a public library. I would not like to hear about packet structure, because that really comes down to what you are using the library for.//So I can load just the header first, check if message has already been received,//and allocate appropriate length or discard duplicate messages etc.struct NetMessageHeader     {    union    {        struct        {            bool m_sequenced            : 1;            bool m_control              : 1;            bool m_fragment             : 1; //Dubious as to whether I need this            bool                        : 1; //Unused field            char m_length_high_nibble   : 4;            char m_length_low_byte      : 8;            int m_sequence_number       : 32; //Endianness - caution        };        struct        {            char m_raw_data[6];        };    };};class NetMessage{private:    atomic_flag m_locked;    NetMessageHeader *m_header;    char *m_message;    unsigned short m_capacity, m_read_position, m_write_position;    //write_position points to the next empty element.    //read_position points to the next element to read.public:    static atomic<unsigned int> maximum_message_length;    NetMessage();    ~NetMessage();    unsigned short GetLength();    bool GetSequence(unsigned int &dest);    bool _SetSequence(unsigned int value);    bool IsSequenced();    bool IsControl();    bool LoadHeader(char &headerBytes);    bool LoadData(char &data);  //Loads an array containing all data, including header.    bool ReserveMemory(unsigned short length);    bool ReadChar(char &dest);    bool ReadChars(char &dest, unsigned short length);    bool WriteChar(char &source);    bool WriteChars(char &source, unsigned short length);};These are the two methods I need to be sure are correct:bool NetMessage::ReadChars(char &dest, unsigned short length){bool data_read = false;//Spinlockwhile (m_locked.test_and_set(memory_order_seq_cst))    ;//Critical section beginsunsigned short remaining_bytes = m_write_position - m_read_position;if (remaining_bytes >= length){    memcpy((char *)dest, m_message, length);    m_read_position += length;    data_read = true;}//Critical section endsm_locked.clear(memory_order_release);return data_read;}bool NetMessage::WriteChars(char &source, unsigned short length){if ((length + HEADER_LENGTH) > maximum_message_length.load(memory_order_acquire))    return false;bool data_written = false;//Spinlockwhile (m_locked.test_and_set(memory_order_acquire))    ;//Critical section beginsif (m_message == nullptr)  //Empty message, create new one.{    m_capacity = NearestPowerOf2(HEADER_LENGTH + length);    m_message = new char(m_capacity);    memcpy(m_message + HEADER_LENGTH, (char*) &source, length);    m_write_position = HEADER_LENGTH + length;    data_written = true;}else if (m_write_position + length < maximum_message_length.load(memory_order_acquire)){    int remaining_bytes = m_capacity - m_write_position;    if (remaining_bytes < length)    {        do        {            m_capacity *= 2;    //Capacity can exceed maximum message length, write position cannot.            if (m_capacity & (m_capacity - 1))  //If capacity is not a power of two                m_capacity = NearestPowerOf2(m_capacity);        }        while (remaining_bytes < length);        char* resized_message = new char(m_capacity);        memcpy(resized_message, m_message, m_write_position);        delete[](m_message);        m_message = resized_message;        m_header = (NetMessageHeader *)resized_message;    }    memcpy(m_message + m_write_position, (char*) &source, 1);    m_write_position += length;    data_written = true;}//Critical section endsm_locked.clear(memory_order_release);}I am interested in hearing your comments; I've never posted here before, so I don't know (if?) just how bad my coding is...
Threadsafe network message
c++;thread safety;library;networking
Don't use your own spin lock (it is ah huge waste of resources).while (m_locked.test_and_set(memory_order_seq_cst))   ;Use a condition variable.Also empty blocks are hard to spot and understand if you are going to use them then put a comment that it is deliberately empty.Never do a manual set and release:m_locked.test_and_set(memory_order_seq_cst);// STUFFm_locked.clear(memory_order_release);This is not exception safe. Use RAII.Basically it looks like C code that happens to be wrapped in a class. As it stands not very good as it is easy to use incorrectly. The whole point of C++ is to design the class so it can not be misused (not just write C in a class).What I would expect as a C++ design. // OK. Its unlikely to be this simple but. //     Along these lines. class Message {     public:         void sendMessage(int fd)         {             write(fd, <Generic Header + Type Info>);             this->serialize(fd);         }         static std::unique_ptr<Message> getMessage(int fd)         {             read(fd, <Generic Header + Type Info>);             auto result Factory::getMessate(<TypeInfo>);             result->deserialize(fd);             return result;         }     private:         // Each message type knows how to send itself.         virtual void serialize(int fd) = 0;         virtual void deserialize(int fd) = 0; }; class Network {     public:        void sendMessage(Message const& m)        {              // network STUFF              m.sendMessage(fd);        }        std:unique_ptr<Message> getMessage()        {              // Network STUFF              return Message::getMessage(fd);        } };