You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kvrocks.apache.org by GitBox <gi...@apache.org> on 2022/10/25 16:16:37 UTC
[GitHub] [incubator-kvrocks] xiaobiaozhao opened a new pull request, #1045: rewrite *write function
xiaobiaozhao opened a new pull request, #1045:
URL: https://github.com/apache/incubator-kvrocks/pull/1045
1. Resolve compile-time warning
2. Improves code robustness
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #1045: rewrite *write function
Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on code in PR #1045:
URL: https://github.com/apache/incubator-kvrocks/pull/1045#discussion_r1005153565
##########
src/common/util.cc:
##########
@@ -672,4 +672,28 @@ int aeWait(int fd, int mask, uint64_t timeout) {
return retval;
}
}
+
+Status Write(int fd, const std::string &data) {
+ ssize_t n = 0;
+ while (n < static_cast<ssize_t>(data.size())) {
+ ssize_t nwritten = write(fd, data.c_str() + n, data.size() - n);
+ if (nwritten == -1) {
+ return Status(Status::NotOK, strerror(errno));
+ }
+ n += nwritten;
+ }
+ return Status::OK();
+}
+
+Status Pwrite(int fd, const std::string &data, std::istream::off_type offset) {
Review Comment:
```suggestion
Status Pwrite(int fd, const std::string &data, off_t offset) {
```
This function is not related to `std::istream`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #1045: rewrite *write function
Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on code in PR #1045:
URL: https://github.com/apache/incubator-kvrocks/pull/1045#discussion_r1005156297
##########
src/common/util.cc:
##########
@@ -672,4 +672,28 @@ int aeWait(int fd, int mask, uint64_t timeout) {
return retval;
}
}
+
+Status Write(int fd, const std::string &data) {
Review Comment:
Do we really use the flag provided by `send`?
Currently `SockSend` has a totally same function body as this function. It is weird.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] xiaobiaozhao commented on a diff in pull request #1045: rewrite *write function
Posted by GitBox <gi...@apache.org>.
xiaobiaozhao commented on code in PR #1045:
URL: https://github.com/apache/incubator-kvrocks/pull/1045#discussion_r1005149002
##########
src/common/util.cc:
##########
@@ -672,4 +672,28 @@ int aeWait(int fd, int mask, uint64_t timeout) {
return retval;
}
}
+
+Status Write(int fd, const std::string &data) {
Review Comment:
I'm going to create a pr that fixes the socksend function and replaces write with send. Becase send has more flags than write.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #1045: rewrite *write function
Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on code in PR #1045:
URL: https://github.com/apache/incubator-kvrocks/pull/1045#discussion_r1005137576
##########
src/common/util.cc:
##########
@@ -672,4 +672,28 @@ int aeWait(int fd, int mask, uint64_t timeout) {
return retval;
}
}
+
+Status Write(int fd, const std::string &data) {
Review Comment:
I wonder if there is any difference between `SockSend` and this new function.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] git-hulk commented on pull request #1045: rewrite *write function
Posted by GitBox <gi...@apache.org>.
git-hulk commented on PR #1045:
URL: https://github.com/apache/incubator-kvrocks/pull/1045#issuecomment-1294342994
Thanks all, merging..
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] git-hulk merged pull request #1045: rewrite *write function
Posted by GitBox <gi...@apache.org>.
git-hulk merged PR #1045:
URL: https://github.com/apache/incubator-kvrocks/pull/1045
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] xiaobiaozhao commented on a diff in pull request #1045: rewrite *write function
Posted by GitBox <gi...@apache.org>.
xiaobiaozhao commented on code in PR #1045:
URL: https://github.com/apache/incubator-kvrocks/pull/1045#discussion_r1005701394
##########
src/common/util.cc:
##########
@@ -672,4 +672,28 @@ int aeWait(int fd, int mask, uint64_t timeout) {
return retval;
}
}
+
+Status Write(int fd, const std::string &data) {
+ ssize_t n = 0;
+ while (n < static_cast<ssize_t>(data.size())) {
+ ssize_t nwritten = write(fd, data.c_str() + n, data.size() - n);
+ if (nwritten == -1) {
+ return Status(Status::NotOK, strerror(errno));
+ }
+ n += nwritten;
+ }
+ return Status::OK();
+}
+
+Status Pwrite(int fd, const std::string &data, std::istream::off_type offset) {
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #1045: rewrite *write function
Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on code in PR #1045:
URL: https://github.com/apache/incubator-kvrocks/pull/1045#discussion_r1005157208
##########
src/common/util.h:
##########
@@ -75,5 +75,7 @@ auto GetTimeStamp() {
}
inline uint64_t GetTimeStampMS() { return GetTimeStamp<std::chrono::milliseconds>(); }
inline uint64_t GetTimeStampUS() { return GetTimeStamp<std::chrono::microseconds>(); }
-
+// file util
+Status Write(int fd, const std::string &data);
+Status Pwrite(int fd, const std::string &data, std::istream::off_type offset);
Review Comment:
Same as above.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org