-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[libc] replace for loops with a call to memcpy in File #165219
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Shreeyash Pandey <[email protected]>
|
@llvm/pr-subscribers-libc Author: Shreeyash Pandey (bojle) ChangesAddresses Full diff: https://github.com/llvm/llvm-project/pull/165219.diff 1 Files Affected:
diff --git a/libc/src/__support/File/file.cpp b/libc/src/__support/File/file.cpp
index 4217e73828388..15ec1a23e2b8d 100644
--- a/libc/src/__support/File/file.cpp
+++ b/libc/src/__support/File/file.cpp
@@ -15,6 +15,7 @@
#include "src/__support/CPP/span.h"
#include "src/__support/libc_errno.h" // For error macros
#include "src/__support/macros/config.h"
+#include "src/string/memory_utils/inline_memcpy.h"
namespace LIBC_NAMESPACE_DECL {
@@ -85,9 +86,7 @@ FileIOResult File::write_unlocked_fbf(const uint8_t *data, size_t len) {
cpp::span<uint8_t> bufref(static_cast<uint8_t *>(buf), bufsize);
// Copy the first piece into the buffer.
- // TODO: Replace the for loop below with a call to internal memcpy.
- for (size_t i = 0; i < primary.size(); ++i)
- bufref[pos + i] = primary[i];
+ inline_memcpy(bufref.data() + pos, primary.data(), primary.size());
pos += primary.size();
// If there is no remainder, we can return early, since the first piece has
@@ -115,9 +114,7 @@ FileIOResult File::write_unlocked_fbf(const uint8_t *data, size_t len) {
// know that if the second piece has data in it then the buffer has been
// flushed, meaning that pos is always 0.
if (remainder.size() < bufsize) {
- // TODO: Replace the for loop below with a call to internal memcpy.
- for (size_t i = 0; i < remainder.size(); ++i)
- bufref[i] = remainder[i];
+ inline_memcpy(bufref.data(), remainder.data(), remainder.size());
pos = remainder.size();
} else {
@@ -209,9 +206,7 @@ size_t File::copy_data_from_buf(uint8_t *data, size_t len) {
// available_data is never a wrapped around value.
size_t available_data = read_limit - pos;
if (len <= available_data) {
- // TODO: Replace the for loop below with a call to internal memcpy.
- for (size_t i = 0; i < len; ++i)
- dataref[i] = bufref[i + pos];
+ inline_memcpy(dataref.data(), bufref.data() + pos, len);
pos += len;
return len;
}
@@ -255,8 +250,7 @@ FileIOResult File::read_unlocked_fbf(uint8_t *data, size_t len) {
size_t fetched_size = result.value;
read_limit += fetched_size;
size_t transfer_size = fetched_size >= to_fetch ? to_fetch : fetched_size;
- for (size_t i = 0; i < transfer_size; ++i)
- dataref[i] = buf[i];
+ inline_memcpy(dataref.data(), buf, transfer_size);
pos += transfer_size;
if (result.has_error() || fetched_size < to_fetch) {
if (!result.has_error())
|
michaelrj-google
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the cleanup!
SchrodingerZhu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. As a side note, there are also other similar patterns in old code.
|
Thanks for approving. I am looking to port libc to macos. Will do these drive-by PRs as I find it. If you can spare some time, can you take a look at this post I made on discourse? Thanks |
Addresses
TODOs in file.cpp by replacing data copies via for loops with calls to inline_memcpy.