Description: In LibreOffice core, there are many places where a block of memory is copied from one place to another. In this case, two functions are used: 1. Older C style memcpy(), which is defined in <string.h> (alternatively <cstring>) and is used as: memcpy( to, from, count ); // to and from are void* pointers https://en.cppreference.com/w/cpp/string/byte/memcpy 2. Newer C++ style std::copy, which is defined in <algorithm> and is used with pointers and iterators as: std::copy( from, from + size, to ); // pointers std::copy( from_iter_first, from_iter_last, to_iter_first ); // iterators https://en.cppreference.com/w/cpp/algorithm/copy The goal here is to refactor the code and replace the usages of memcpy() with equivalent std::copy(). Finding instances: You can find instances of memcpy() usage in C++ code with: $ git grep -w memcpy *.hxx *.cxx Currently there are 720 instances of using memcpy in the code. Notes: 1. Correctly setting the parameters is important, as they are different in std::copy() and memcpy(). 2. You have to make sure that the code behavior does not change with your patch. If classes with user-defined copy constructor are used instead of basic data types, you have to make sure that std::copy which invokes copy constructor does not make any difference. Example: You can see the two std::copy() calls (commented) equivalent to memcpy() in this code snippet: #include <iostream> #include <vector> #include <cstring> #include <algorithm> int main() { std::vector<int> from {1,2,3,4,5}, to(5); std::memcpy(to.data(), from.data(), 5 * sizeof(int)); // std::copy(from.data(), from.data() + 5, to.data()); // std::copy(from.begin(), from.end(), to.begin()); for (int num : to) std::cout << num << " "; std::cout << "\n"; return 0; }
Also, memmove (usable with overlapping memory) may be replaced with either std::copy (when end of dest range is inside of source range, i.e. copying to the left), or std::copy_backward (when start of dest range is inside of source range, i.e. copying to the right). Note also, that e.g. cppreference states, that "implementations of std::copy ... use bulk copy functions such as std::memmove", so not perfectly identical to memcpy, which can't work on overlapping ranges, but is most efficient.
Hey! I provided a patch for this: https://gerrit.libreoffice.org/c/core/+/178954
Alin Andrei Abahnencei committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/8391bd0ea4fb3a7f4cad4149ec63c980eab5808d tdf#163691 Use std::copy() instead of memmove() It will be available in 25.8.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
This is a multi-hacker EasyHack. Please do not assign it to yourself, but rather start working on it.
I provided a patch for this: https://gerrit.libreoffice.org/c/core/+/182787
Kukee Thoo committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/bff3d755c8c11721054f4ff40a3d5f723b0c6b96 tdf#163691 Use std::copy() instead of memcpy() It will be available in 25.8.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Akshay Kumar Dubey committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/90490e7436ab8d823994b50472622d1c83fa118e tdf#163691 Use std::copy() instead of memcpy(). It will be available in 25.8.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Patch submitted to Gerrit: https://gerrit.libreoffice.org/c/core/+/183284 Replaced `memcpy` with `std::copy` in `basmgr.cxx` and `image.cxx`.
I provided a patch for this: https://gerrit.libreoffice.org/c/core/+/183305
Kukee Thoo committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/d40097edc58d93d823d2892e0b12a8ce43669b83 tdf#163691 Use std::copy() instead of memcpy() in docpasswordhelper.cxx It will be available in 25.8.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Haokun Wu committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/fa357e4a0a44471637373302c4391e6b0c2f1a20 tdf#163691 Replace memcpy with std::copy in basmgr.cxx and image.cxx It will be available in 25.8.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Devashish Gupta committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/1d04721870ab60cb6d66d99dc62ee21eee02232f tdf#163691 Replace memcpy / memmove with std::copy / std::copy_backward It will be available in 25.8.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Hi, I provided a patch for this: https://gerrit.libreoffice.org/c/core/+/183512
Ilmari Lauhakangas committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/f336fc1835e89175257058ac7397a02b805e8170 Revert "tdf#163691 Replace memcpy with std::copy in basmgr.cxx and image.cxx" It will be available in 25.8.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
FerMeza committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/f3c89726d2234c0803c005dc87392f94147a481f tdf#163691 Replace memcpy with std::copy in basmgr.cxx It will be available in 25.8.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Don't assign multi-hacker tasks to yourself, simply work on them.
Rohan Sinha committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/fda2cfeb8e69ec95bb464301140999486d06b984 tdf#163691 replace memcpy with std::copy in xml_byteseq It will be available in 25.8.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Hello! I provided a patch for this issue: https://gerrit.libreoffice.org/c/core/+/185422
Hello! I have provided two patches as requested. https://gerrit.libreoffice.org/c/core/+/185441 https://gerrit.libreoffice.org/c/core/+/185442
Hi! I provided another patch for this: https://gerrit.libreoffice.org/c/core/+/186792
Alin Andrei Abahnencei committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/1f597d3d377d97fb333d98a6fefd82ffbdf87853 tdf#163691 Use std::copy() instead of memcpy() It will be available in 26.2.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
codemaestro committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/a91ce74ddcf2ff410515b794273cf070e7d4a4ac tdf#163691: Use std::copy() instead of memcpy() It will be available in 26.2.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Sergey Anisimov committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/629dab07c8b9a5d3717211db5b0126333f2b9873 tdf#163691 - Use std::copy_n() instead of memcpy() It will be available in 26.2.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Daniel Lee committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/e7e6ef7e7afac217f2c436d9cc2c8cb22082a515 tdf#163691 Replace memcpy() with std::copy() It will be available in 26.2.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Note that the idea is not to replace every occurrence of memcpy with std::copy. memcpy is a low-level memory copy utility; it is most efficient in what it is intended for: low-level memory copy. It should stay where it is reasonable. How to tell? One diagnostic is, when you have to use reinterpret_cast to replace memcpy with std::copy. As soon as you do that, you effectively try to disguise a genuine low-level copy operation into something more high-level. This is NOT needed. The end result MUST be an improvement over the old code, not a "replacement just for the sake of replacement". There are plenty of improvements possible, like improved readability by using object size implicitly (note that wherever you need to use "iterator + length" in std::copy, you better use copy_n) and safety (using iterators often gives runtime out-of-bounds checks in debug builds). If your change doesn't provide any upside, please avoid that.
Neacsa Leonard committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/9f25059ea15b039c68aed2aa91819bf62a9044cf tdf#163691 Use std::copy() and std::copy_n instead of memcpy() It will be available in 26.8.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.