Skip to content

Commit a44e7e7

Browse files
Peng Chenmeta-codesync[bot]
authored andcommitted
comms/uniflow/controller: take a span in sendAllVec (#3908)
Summary: Pull Request resolved: #3908 sendAllVec took (iovec*, int) and tracked its position with a separate idx cursor, so the loop carried `iov + idx`, `iovCnt - idx` and `iov[idx]` arithmetic. std::span carries the count with the pointer, which lets the cursor go away entirely: the retire step becomes iov = iov.subspan(1) and the loop condition becomes !iov.empty(). That is the real win rather than the shorter signature. The arithmetic being deleted lives in the short-write branch, which the comment there notes is not reachable on a blocking socket except via a signal mid-transfer and is not covered by tests -- so it is the least safe place in the function to keep hand-rolled index math. It also drops the static_cast<size_t> on msg_iovlen, since size() is already size_t, and matches the idiom the rest of the interface uses: std::span<const uint8_t> on send, std::span<uint8_t> on recv. <span> was already included and sendAllVec is private with one call site, so there is no ABI consideration. The call site's count becomes size_t and passes std::span{iov}.first(iovCnt), so the cast is removed rather than relocated to the caller. Kept the doc comment. A non-const std::span<iovec> conveys no more about mutation than a non-const iovec* did; what the comment carries is that the mutation is destructive bookkeeping -- the iov must not be reused after the call -- and no signature expresses that. Reviewed By: yexiangd Differential Revision: D117414473 fbshipit-source-id: 0d0b7add6233ae74216abecf4a25865465eacbe0
1 parent 44b6410 commit a44e7e7

2 files changed

Lines changed: 14 additions & 14 deletions

File tree

comms/uniflow/controller/TcpController.cpp

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -381,12 +381,11 @@ bool TcpConn<IOPolicy>::sendAll(const void* buf, size_t len) {
381381
// for these sockets, and widening it here would hide a non-blocking data socket
382382
// rather than fix one.
383383
template <typename IOPolicy>
384-
bool TcpConn<IOPolicy>::sendAllVec(iovec* iov, int iovCnt) {
385-
int idx = 0;
386-
while (idx < iovCnt) {
384+
bool TcpConn<IOPolicy>::sendAllVec(std::span<iovec> iov) {
385+
while (!iov.empty()) {
387386
msghdr msg{};
388-
msg.msg_iov = iov + idx;
389-
msg.msg_iovlen = static_cast<size_t>(iovCnt - idx);
387+
msg.msg_iov = iov.data();
388+
msg.msg_iovlen = iov.size();
390389
ssize_t n = ::sendmsg(sock_, &msg, MSG_NOSIGNAL);
391390
if (n < 0) {
392391
if (errno == EINTR) {
@@ -407,13 +406,14 @@ bool TcpConn<IOPolicy>::sendAllVec(iovec* iov, int iovCnt) {
407406
// covered by tests -- an attempt to force it with an oversized payload did
408407
// not reach this branch.
409408
auto consumed = static_cast<size_t>(n);
410-
while (idx < iovCnt && consumed >= iov[idx].iov_len) {
411-
consumed -= iov[idx].iov_len;
412-
++idx;
409+
while (!iov.empty() && consumed >= iov.front().iov_len) {
410+
consumed -= iov.front().iov_len;
411+
iov = iov.subspan(1);
413412
}
414-
if (idx < iovCnt && consumed > 0) {
415-
iov[idx].iov_base = static_cast<uint8_t*>(iov[idx].iov_base) + consumed;
416-
iov[idx].iov_len -= consumed;
413+
if (!iov.empty() && consumed > 0) {
414+
iov.front().iov_base =
415+
static_cast<uint8_t*>(iov.front().iov_base) + consumed;
416+
iov.front().iov_len -= consumed;
417417
}
418418
}
419419
return true;
@@ -541,14 +541,14 @@ Result<size_t> TcpConn<IOPolicy>::syncSend(std::span<const uint8_t> data) {
541541
iovec iov[2];
542542
iov[0].iov_base = &len;
543543
iov[0].iov_len = sizeof(len);
544-
int iovCnt = 1;
544+
size_t iovCnt = 1;
545545
if (!data.empty()) {
546546
// const_cast: iovec has no const variant, and sendmsg only reads.
547547
iov[1].iov_base = const_cast<uint8_t*>(data.data());
548548
iov[1].iov_len = data.size();
549549
iovCnt = 2;
550550
}
551-
if (!sendAllVec(iov, iovCnt)) {
551+
if (!sendAllVec(std::span{iov}.first(iovCnt))) {
552552
return Err(
553553
ErrCode::ConnectionFailed,
554554
"send frame failed: " + std::system_category().message(errno));

comms/uniflow/controller/TcpController.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ class TcpConn : public Conn {
145145
bool sendAll(const void* buf, size_t len);
146146
/// Vectored sendAll: one syscall for the length prefix plus payload. Mutates
147147
/// @p iov to track partial writes, so it must not be reused by the caller.
148-
bool sendAllVec(iovec* iov, int iovCnt);
148+
bool sendAllVec(std::span<iovec> iov);
149149
bool recvAll(void* buf, size_t len);
150150
bool exchangeMagic();
151151
Result<size_t> syncSend(std::span<const uint8_t> data);

0 commit comments

Comments
 (0)