Skip to content

Commit

Permalink
Handle partial writes correctly.
Browse files Browse the repository at this point in the history
QIODevice::write() can theoretically write any number of bytes less than
the amount that was requested. If the result was >= 0, we haven't hit EOF
or an actual error yet and should continue attempting to write. Note that
the docs[1] for QIODevice::writeData() specify that writeData should write
all available data before returning or else QDataStream won't work, but
that's no guarantee that all implementors of QIODevice will actually do so.

Also note that this does not call QIODevice::waitForBytesWritten after
writing, which would negate the benefits of buffering for buffered devices.
As a side-effect, that means this will spin if the writes don't complete.

[1] http://doc.qt.io/qt-5/qiodevice.html#writeData
  • Loading branch information
mtstickney authored and Matthew Stickney committed Feb 16, 2017
1 parent bec0d71 commit 067c727
Showing 1 changed file with 24 additions and 15 deletions.
39 changes: 24 additions & 15 deletions src/msgpackstream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ MsgPackStream &MsgPackStream::operator<<(bool b)
CHECK_STREAM_WRITE_PRECOND(*this);
quint8 m = b == true ?
MsgPack::FirstByte::MTRUE : MsgPack::FirstByte::MFALSE;
if (dev->write((char *)&m, 1) != 1)
if (writeBytes((char *)&m, 1) != 1)
setStatus(WriteFailed);
return *this;
}
Expand All @@ -395,7 +395,7 @@ MsgPackStream &MsgPackStream::operator<<(quint32 u32)
CHECK_STREAM_WRITE_PRECOND(*this);
quint8 p[5];
quint8 sz = MsgPackPrivate::pack_uint(u32, p, true) - p;
if (dev->write((char *)p, sz) != sz)
if (writeBytes((char *)p, sz) != sz)
setStatus(WriteFailed);
return *this;
}
Expand All @@ -405,7 +405,7 @@ MsgPackStream &MsgPackStream::operator<<(quint64 u64)
CHECK_STREAM_WRITE_PRECOND(*this);
quint8 p[9];
quint8 sz = MsgPackPrivate::pack_ulonglong(u64, p, true) - p;
if (dev->write((char *)p, sz) != sz)
if (writeBytes((char *)p, sz) != sz)
setStatus(WriteFailed);
return *this;
}
Expand All @@ -415,7 +415,7 @@ MsgPackStream &MsgPackStream::operator<<(qint32 i32)
CHECK_STREAM_WRITE_PRECOND(*this);
quint8 p[5];
quint8 sz = MsgPackPrivate::pack_int(i32, p, true) - p;
if (dev->write((char *)p, sz) != sz)
if (writeBytes((char *)p, sz) != sz)
setStatus(WriteFailed);
return *this;
}
Expand All @@ -425,7 +425,7 @@ MsgPackStream &MsgPackStream::operator<<(qint64 i64)
CHECK_STREAM_WRITE_PRECOND(*this);
quint8 p[9];
quint8 sz = MsgPackPrivate::pack_longlong(i64, p, true) - p;
if (dev->write((char *)p, sz) != sz)
if (writeBytes((char *)p, sz) != sz)
setStatus(WriteFailed);
return *this;
}
Expand All @@ -435,7 +435,7 @@ MsgPackStream &MsgPackStream::operator<<(float f)
CHECK_STREAM_WRITE_PRECOND(*this);
quint8 p[5];
quint8 sz = MsgPackPrivate::pack_float(f, p, true) - p;
if (dev->write((char *)p, sz) != sz)
if (writeBytes((char *)p, sz) != sz)
setStatus(WriteFailed);
return *this;
}
Expand All @@ -445,7 +445,7 @@ MsgPackStream &MsgPackStream::operator<<(double d)
CHECK_STREAM_WRITE_PRECOND(*this);
quint8 p[9];
quint8 sz = MsgPackPrivate::pack_double(d, p, true) - p;
if (dev->write((char *)p, sz) != sz)
if (writeBytes((char *)p, sz) != sz)
setStatus(WriteFailed);
return *this;
}
Expand All @@ -457,7 +457,7 @@ MsgPackStream &MsgPackStream::operator<<(QString str)
quint32 sz = MsgPackPrivate::pack_string(str, p, false) - p;
quint8 *data = new quint8[sz];
MsgPackPrivate::pack_string(str, data, true);
if (dev->write((char *)data, sz) != sz)
if (writeBytes((char *)data, sz) != sz)
setStatus(WriteFailed);
delete[] data;
return *this;
Expand All @@ -471,7 +471,7 @@ MsgPackStream &MsgPackStream::operator<<(const char *str)
quint32 sz = MsgPackPrivate::pack_string_raw(str, str_len, p, false) - p;
quint8 *data = new quint8[sz];
MsgPackPrivate::pack_string_raw(str, str_len, data, true);
if (dev->write((char *)data, sz) != sz)
if (writeBytes((char *)data, sz) != sz)
setStatus(WriteFailed);
delete[] data;
return *this;
Expand All @@ -483,21 +483,30 @@ MsgPackStream &MsgPackStream::operator<<(QByteArray array)
quint8 p[5];
quint32 len = array.length();
quint8 header_len = MsgPackPrivate::pack_bin_header(len, p, true) - p;
if (dev->write((char *)p, header_len) != header_len) {
if (writeBytes((char *)p, header_len) != header_len) {
setStatus(WriteFailed);
return *this;
}
if (dev->write(array.data(), len) != len)
if (writeBytes(array.data(), len) != len)
setStatus(WriteFailed);
return *this;
}

bool MsgPackStream::writeBytes(const char *data, uint len)
{
CHECK_STREAM_WRITE_PRECOND(false);
if (dev->write(data, len) != len) {
setStatus(WriteFailed);
return false;
uint written = 0;
uint thisWrite;
while (written < len) {
thisWrite = dev->write(data, len - written);
if (thisWrite < 0) {
setStatus(WriteFailed);
return false;
}

/* Increment the write pointer and the total byte count. */
data += thisWrite;
written += thisWrite;
}
return true;
}
Expand Down Expand Up @@ -534,7 +543,7 @@ bool MsgPackStream::writeExtHeader(quint32 len, qint8 msgpackType)
d[5] = msgpackType;
sz = 6;
}
if (dev->write((const char *)d, sz) != sz) {
if (writeBytes((const char *)d, sz) != sz) {
setStatus(WriteFailed);
return false;
}
Expand Down

0 comments on commit 067c727

Please sign in to comment.