I have a function in C which reads the contents of a HTTP request body into a std::string. I came up with the following code:

void handle_request_body(int connfd, HttpRequest &req) {
  unsigned long size_to_read;
  try {
    size_to_read = std::stoul(req.headers().at("content-length"));
  } catch (std::out_of_range const &) {
    return;
  }
  char *buf = new char[size_to_read   1];
  memset(buf, 0, size_to_read   1);
  read(connfd, buf, size_to_read);
  req._body.append(buf);
  delete[] buf;
}

This is a little ugly to me as I have to use new since variable-sized arrays are not allowed. I then tried to read directly to a string instead with the following code:

void handle_request_body(int connfd, HttpRequest &req) {
  unsigned long size_to_read;
  try {
    size_to_read = std::stoul(req.headers().at("content-length"));
  } catch (std::out_of_range const &) {
    return;
  }
  std::string buf(size_to_read   1, 0);
  read(connfd, buf.data(), size_to_read);
  req._body = buf;
}

I find the second method much cleaner, but I'm worried as to whether it is considered bad practice to read directly into a std::string using its data() method.

Is there a better way to do this? Any insight is much appreciated!

CodePudding user response：

Really depends what your read function does under the hood.

If you have control over the read function, I strongly suggest you don't use a pointer, but rather a class reference to a std container. The resizable std containers don't guarantee that the pointers will keep pointing at the same memory i.e. if it reallocates it's size, your pointer will no longer be valid. Which is fine in this example because no one else is touching it, but in a lot of other applications this would be extremely unsafe!

Something like:

void read(int id, std::string& dest, int readLength){
      //whatever code gets the data stream
      dest  = data;
}

If it has to be a C-Style buffer for some OS API call probably best to use a unique pointer of chars, to let the memory clean up after itself.

 std::unique_ptr<char[]> buffer = std::make_unique<char[]>(size   1);
 memset(&buffer[0], 0, size   1);

I don't recommend reading from the OS char by char as this usually has a huge performance overhead for every call, compared to reading it all at once.

CodePudding user response：

As @Galik commented. This is opinion based.

But what we can state is:

• In C we should not use raw pointers for owned memory
• new and delete should be avoided
• C-Style arrays should be avoided
• Reading into data() will work, but is not that nice (my opinion). Anyway, better than using new and creating a memory leak.

You could, in a simple for loop read one byte after the other from the file and then add each byte to the std::string with =. But this is also very clumsy.

Best would be, if you were allowed to use C language and libraries.

BTW. std::string is also a "variable-sized array"

Recommendation: Go with your second solution