[cmake-developers] [PATCH] Avoid bad alloc for large files

Domen Vrankar domen.vrankar at gmail.com
Mon Dec 22 14:51:51 EST 2014


>>> I received a bad alloc when uploading a large file with CTest. The patch
>>> below resolved this.

I just took a look at the code and noticed that it is quite memory
consumption heavy ((2 * encoded_buffer_size) + file_buffer_size).
This implementation could be used instead (not tested):

std::string cmCTest::Base64EncodeFile(const std::string& file)
{
  const size_t len = cmSystemTools::FileLength(file);
  cmsys::ifstream ifs(file.c_str(), std::ios::in
#ifdef _WIN32
    | std::ios::binary
#endif
    );
  std::string base64; // set to empty string by default

  // section for throwing encoded_buffer out of scope as soon as possible to
  // use up less memory
  {
    const size_t output_size = ((len - 1) / 3) * 4 + 4;
    const size_t final_size = output_size + (output_size / 64) * 2;
    std::vector<unsigned char> encoded_buffer;
    encoded_buffer.resize(final_size);

    // section for throwing file_buffer out of scope as soon as possible to
    // use up less memory
    {
      std::vector<unsigned char> file_buffer;
      file_buffer.resize(len);
      ifs.read(reinterpret_cast<char*>(file_buffer), len);
      ifs.close();

      unsigned long rlen
        = cmsysBase64_Encode(&file_buffer[0], len, &encoded_buffer[0], 1);
    }

    // consume less memory by swapping tmp string buffer with base64 variable
    std::string(encoded_buffer.begin(), encoded_buffer.begin() +
rlen).swap(base64);
  }

  return base64; // this will produce another copy in C++98
}

It lowers memory consumption to approximately (2 *
encoded_buffer_size) and it also makes memory deallocation exception
safe (file_buffer and encoded_buffer variables).
A better solution would be to rewrite the function to:
void cmCTest::Base64EncodeFile(const std::string& file, std::string&
base64) to prevent one more copy at return but since this changes the
interface we could go a step further and simply rewrite the entire
function and cmsysBase64_Encode function to use streams and thereby
lower the memory consumption even further (less copying between
different containers and more readable than the above version).

Thoughts?

p.s. I think that using std::ios::binary would work on all platforms
so I guess that the function does not need ifdef at the top.

Regards,
Domen


More information about the cmake-developers mailing list