5

I have a class that wraps a big array of bytes that are network packets. The class implements a queue and provides (among others) front() function that returns a const vector of bytes that constitute the oldest packet in the queue. 

class Buffer{
  unsigned char data[65536];
  unsigned int offset;
  unsigned int length;
  [...]//other fields for maintaining write ptr etc.

public:
  const std::vector<unsigned char> front(){
    return std::vector<unsigned char>(data + offset, data + offset + length);
  }

  //other methods for accessing the queue like
  //pop(), push(), clean() and so forth...
  [...]
}

The performance of above implementation of front() function suffers from unnecessary copying bytes from the range occupied by the current packet. Since the vector is const, there is no need of making a copy of the data. What I want is to create a vector on the data that are already stored in the buffer. Of course destructor of the vector should not deallocate the memory.

Patryk
  • 1,421
  • 8
  • 21
  • How are you going to use `front()` function? Maybe it will be better to declare two functions returning const iterators: `std::vector::const_iterator front_begin() const` and `front_end()`? – gudok Apr 25 '16 at 11:19
  • 1
    Do you really need to return a `std::vector` ? Couldn't you just return a view into the buffer? – Sean Apr 25 '16 at 11:22
  • In fact, at the moment I have two functions: readPtr() returing data + offset and front() returning just length. I just wanted to simplify that by returning a vector, so by calling front() user will get everything. – Patryk Apr 25 '16 at 11:24
  • You might be able to do this with a std::vector, with some scary allocator magic. The vector wants to allocate memory, deallocate memory, set its own size, and (in some of its constructors) copy data. You can stop the deallocation with your custom allocator, and you can (I think?) safely fake the "allocation" of your chosen memory location, also with the allocator. You can probably prevent the copying by using a constructor that doesn't put any items into the vector (the default one might work, I guess). I don't know how you'd handle the setting of the size though... – mjwach Apr 25 '16 at 11:38
  • 1
    Oh wait, maybe the allocator's construct method could be made to not initialize vector elements? Then you could use a size-setting constructor, or vector::resize, maybe? Haha it would be quite silly to attempt these things, in most cases. You probably should not use a std::vector for this. – mjwach Apr 25 '16 at 11:43
  • Related: http://stackoverflow.com/q/29007753/2642059 – Jonathan Mee Apr 25 '16 at 11:48
  • The view idea is good but you need to find a way to invalidate the view in case you overwrite the area once new data arrives. – Juan Leni Apr 25 '16 at 11:58
  • I would generate a shared_ptr and create a view using a weak_ptr. If you ever go through that area again you can invalidate the views as the weak_ptr will not be valid anymore. Encapsulate into another class if you want/need – Juan Leni Apr 25 '16 at 12:00
  • The standard library works most naturally with iterators, not containers. You can do the same. Have your function return a `pair` of `Buffer::iterator`, where `iterator` is a typedef or any other way you want to implement it. The important point is that it's obvious that ownership of the data remains with the original container. – Mark Ransom Apr 26 '16 at 04:46

2 Answers2

2

You have some options available to you:

  1. Rather than returning a vector, just return a const char*:
const char* front() {
    return data;
}
  1. Consider using a standard container, such as a string data as your Buffer member. This will allow you to do:
const string& front() {
    return data;
}
  1. The best option though is if you have C++17 or access to experimental::string_view you could just do:
const string_view front() {
    return string_view(data);
}

Just a convention comment, there is going to be an expectation of front that it will behave like other standard containers, which:

Returns a reference to the first element in the container.
Calling front on an empty container is undefined.

[source]

Bringing front to apply to bare on fixed size arrays was also discussed by the C++ standards committee: front and back Proposal for iterators Library

As it is this method more closely resembles data, which:

Returns a pointer to the block of memory containing the elements of the container.

[source]

Community
  • 1
  • 1
Jonathan Mee
  • 37,899
  • 23
  • 129
  • 288
  • I have to return length as well. That's one of reasons why I want to use vector. Using string would be confusing to user and I have no access to c++17. Regarding naming - as I wrote the class implements a queue, so using name `front` is quite legitimate in my opinion. Thanks for suggestions anyway. – Patryk Apr 25 '16 at 12:02
  • @Patryk Can you help me understand how, using a standard container for your `data` member would be "confusing to the user"? – Jonathan Mee Apr 25 '16 at 12:28
  • Using string for raw data is confusing, at least for me. For instance, I wouldn't know intuitively as a user, what would be returned by length() function - will it be length of data, number of bytes until first `\0` or even something else. – Patryk Apr 26 '16 at 08:04
  • @Patryk If none of the methods that a `string` provides beyond `vector` are of added benefit to you, then using a `vector` makes perfect sense. But if that is the case then my option **2** suggests that you use: "a standard container... as your `Buffer` member" so perhaps `vector data`? – Jonathan Mee Apr 26 '16 at 11:11
  • but I do not want to return entire data, just a subset of it in a convenient form of a const vector, in the way that I do not do copy and alloc on create and dealloc on destroy. – Patryk Apr 26 '16 at 12:36
  • @Patryk A subset is a harder problem than what's in your question. If that's the case I'd suggest that you return a `pair`. (Though the actual best solution is still `string_view`.) If you edit your question to update the requirements and comment to me when it's done, I'll update my answer as well. – Jonathan Mee Apr 26 '16 at 12:49
0

If you're looking to avoid unnecessary copying then you'll need to return a view into the data. You can either provide a front_begin() and front_end() set of functions:

const char *front_begin() const
{
  return data + offset;
}

const char *front_end() const
{
  return data + offset + length;
}

Or write a wrapper class:

class Data
{
private:
  const char *m_Begin;
  const char *m_End;

public:
  Data(const char *begin, const char *end) : m_Begin(begin), m_End(end)
  {
  }

  const char *begin() const
  {
    return m_Begin;
  }

  const char *end() const
  {
    return m_End;
  }
}

And have your front() method return one of these:

Data front()
{
  return Data(data + offset, data + offset + length)
}

If you're using C++11 then you can use a Data instance in a ranged based for loop:

Data data = buffer.front();
for(char c : data)
{
  // Do something with the data
}
Sean
  • 60,939
  • 11
  • 97
  • 136