embeddedmz / ftpclient-cpp

C++ client for making FTP requests
MIT License
211 stars 67 forks source link

Added download to memory option #9

Closed fireuse closed 4 years ago

fireuse commented 4 years ago

Added download to memory

embeddedmz commented 4 years ago

Thank you for you contribution. That's interesting !

Could you change this line (obvious memory bug) : std::vector<int>* vec = reinterpret_cast<std::vector<int>*>(data); to : std::vector<char>* vec = reinterpret_cast<std::vector<char>*>(data);

Maybe, it's better to use uint8_t instead of char, just add this C library header somewhere if you want.

If you have "clang-format" can you run it from the root directory ? I'll do it myself if you don't have it installed !

fireuse commented 4 years ago

Ok, i have run clang-format, but it altered nearly all source files. I think, changing char to uint8_t is not needed, on the majority of platforms char is defined as 8bit type.

embeddedmz commented 4 years ago

@fireuse I launched unit tests an there was a crash in CFTPClient::WriteToMemory because the vector must be resized before copying data from libcurl ! don't forget to update your branch with my master one.

Regards.

embeddedmz commented 4 years ago

@fireuse I think there's still a bug because WriteToMemory can be called many times by libcurl !

the solution is not to resize the vector "size * nmemb" each time WriteToMemory gets called, but to append the bytes to the vector with something like this (example from http://www.cplusplus.com/reference/iterator/back_insert_iterator/)

std::back_insert_iterator< std::vector<int> > back_it (foo);
std::copy (bar.begin(),bar.end(),back_it);

another solution (maybe more faster) is to change the vector capacity (via reserve method) by vector.size() + size * nmemb and start the copy from the end of the vector.

Ideally, the solution is to request the size of the file in bytes before downloading it, to resize the vector only once (CFTPClient::Info can be used to request the file size).

embeddedmz commented 4 years ago

@fireuse I used a std::back_insert_iterator in WriteToMemory and ensured that the vector is cleared in the DownloadFile method.

Regards.