Abusing pointers is not always fun

Or how to waste one hour(if lucky) in a debugger.

So a couple of days ago I was working on some network protocol stuff. The code looked something like this:

	struct header_foo {};
	struct header_bar {};
	struct payload {};
	class pdu {
	std::vector<uint8_t> buffer_;
	// used in the class for stuff and returned to the user via getters
	// so they can change various fields
	header_foo* ptr_foo_{nullptr};
	header_bar* ptr_bar_{nullptr};
	payload* ptr_payload_{nullptr};
	public:
	// ... code ...
	// resize the buffer to fit more data
	void expand_buffer(unsigned size) {
	// ... code ...
	}
	void push_header_foo() {
	expand_buffer(sizeof(header_foo));
	ptr_foo_ = reinterpret_cast<header_foo*>(&buffer_[0]);
	}
	void push_header_bar() {
	expand_buffer(sizeof(header_bar));
	ptr_bar_ = reinterpret_cast<header_bar*>(&buffer_[sizeof(header_foo)]);
	}
	void push_payload() {
	expand_buffer(sizeof(payload));
	ptr_payload_ = reinterpret_cast<payload*>(&buffer_[sizeof(header_bar)]);
	}
	};
	void f() {
	pdu p;
	p.push_header_foo();
	p.push_header_bar();
	p.push_payload();
	}

view raw pdu.cpp hosted with ❤ by GitHub

I'll give you 5 minutes to find the bug ... found it? No? Here we go.

All those pointers (ptr_foo_, ptr_bar_, ptr_payload_) are not updated when something new is pushed (given a second thought maybe "append()" would be a better name).

Why would we need to do that, to update those pointers?

Remember that when you call std::vector::reserve(), you simply ask the allocator to expand or contract the vector's internal memory block. When the new block is found the contents of the old block might be copied to the new block.

This means that after such a reallocation our pointers will point into some unknown memory region, which might be used by other parts of the program i.e. everytime we change stuff via those pointers we taint someones data.

Now you may ask how can we fix this? It's really simple, we just need to ditch those member pointers and replace them with functions(getters) that ALWAYS compute a pointer using buffer_ and an offset.

Looking back having those pointers as data members was just a dumb idea, not only causing problems but also increase the memory footprint of a pdu.

PS: I'm aware of my dumb structs not being packed so no need to point that. :)