larroy commented on a change in pull request #9370: Fix crash when opening an image, fix exception safety. URL: https://github.com/apache/incubator-mxnet/pull/9370#discussion_r161572998
########## File path: src/io/image_io.cc ########## @@ -218,29 +218,29 @@ void Imread(const nnvm::NodeAttrs& attrs, const auto& param = nnvm::get<ImreadParam>(attrs.parsed); std::ifstream file(param.filename, std::ios::binary | std::ios::ate); + // if file is not open we get bad alloc after tellg + CHECK(file.is_open()) << "Imread: Couldn't open file: " << param.filename; size_t fsize = file.tellg(); file.seekg(0, std::ios::beg); - auto buff = new uint8_t[fsize]; - file.read(reinterpret_cast<char*>(buff), fsize); + auto buff = std::make_shared<std::vector<uint8_t> >(fsize); Review comment: @cjolivier01 thank you. As said previously, I believe it needs to be a shared_ptr since the pointer is passed to the engine and deallocated in a different thread (as well as copied into the lambda). The array specialization of shared_ptr is not available until C++20, unlike your proposed solution with unique_ptr. Hence I suggest we accept the solution with a vector as a compromise right now. Agreed there's a small penalization on initializing the buffer, but this should not be a big deal given that we read from disk. Would you be willing to agree to this? ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
