gfxstream: make sure we consistently use uint32_t instead of int32_t to read the packet size.
A negative packet size doesn't make sense, and the code currently doesn't try to handle that scenario (ie: all 4 stream decoders do `ptr += packetLen` and as such would move backwards if the length was negative.
Further, on the encoder side, uint32_t is used (e.g.:
http://cs/android/device/generic/goldfish-opengl/system/vulkan_enc/VkEncoder.cpp;l=122;rcl=b0df973dd65f81645707f65a12e8a196a5310ef2)
Test: compiles, runs
Change-Id: Iaa0ac3fe57a3c20b419ceebe80d3b8ccec4083d7
diff --git a/stream-servers/ReadBuffer.cpp b/stream-servers/ReadBuffer.cpp
index 8bc5237..a108cca 100644
--- a/stream-servers/ReadBuffer.cpp
+++ b/stream-servers/ReadBuffer.cpp
@@ -36,21 +36,21 @@
free(m_buf);
}
-void ReadBuffer::setNeededFreeTailSize(int size) {
+void ReadBuffer::setNeededFreeTailSize(size_t size) {
m_neededFreeTailSize = size;
}
-int ReadBuffer::getData(IOStream* stream, int minSize) {
+int ReadBuffer::getData(IOStream* stream, size_t minSize) {
assert(stream);
- assert(minSize > (int)m_validData);
+ assert(minSize > m_validData);
- const int minSizeToRead = minSize - m_validData;
- const int neededFreeTailThisTime =
+ const size_t minSizeToRead = minSize - m_validData;
+ const size_t neededFreeTailThisTime =
std::max(minSizeToRead,
m_neededFreeTailSize);
- int maxSizeToRead;
- const int freeTailSize = m_buf + m_size - (m_readPtr + m_validData);
+ size_t maxSizeToRead;
+ const size_t freeTailSize = m_buf + m_size - (m_readPtr + m_validData);
if (freeTailSize >= neededFreeTailThisTime) {
maxSizeToRead = freeTailSize;
} else {
@@ -63,10 +63,8 @@
// Note: make sure we can fit at least two of the requested packets
// into the new buffer to minimize the reallocations and
// memmove()-ing stuff around.
- size_t new_size = std::max<size_t>(
- 2 * minSizeToRead + m_validData,
- 2 * m_size);
- if (new_size < m_size) { // overflow check
+ size_t new_size = std::max(2 * minSizeToRead + m_validData, 2 * m_size);
+ if (new_size < m_size) { // overflow check
new_size = INT_MAX;
}
diff --git a/stream-servers/ReadBuffer.h b/stream-servers/ReadBuffer.h
index b7a970c..da40856 100644
--- a/stream-servers/ReadBuffer.h
+++ b/stream-servers/ReadBuffer.h
@@ -24,10 +24,10 @@
explicit ReadBuffer(size_t bufSize);
~ReadBuffer();
- void setNeededFreeTailSize(int size);
- int getData(IOStream *stream, int minSize); // get fresh data from the stream
+ void setNeededFreeTailSize(size_t size);
+ int getData(IOStream *stream, size_t minSize); // get fresh data from the stream
unsigned char *buf() { return m_readPtr; } // return the next read location
- size_t validData() { return m_validData; } // return the amount of valid data in readptr
+ size_t validData() const { return m_validData; } // return the amount of valid data in readptr
void consume(size_t amount); // notify that 'amount' data has been consumed;
void onLoad(android::base::Stream* stream);
@@ -41,7 +41,7 @@
size_t m_validData;
uint64_t m_tailMoveTimeUs = 0;
- int m_neededFreeTailSize = 0;
+ size_t m_neededFreeTailSize = 0;
};
} // namespace emugl
diff --git a/stream-servers/RenderThread.cpp b/stream-servers/RenderThread.cpp
index 56557aa..87a810e 100644
--- a/stream-servers/RenderThread.cpp
+++ b/stream-servers/RenderThread.cpp
@@ -334,12 +334,12 @@
uint32_t* seqnoPtr = nullptr;
- while (1) {
+ while (true) {
// Let's make sure we read enough data for at least some processing.
- int packetSize;
+ uint32_t packetSize;
if (readBuf.validData() >= 8) {
// We know that packet size is the second int32_t from the start.
- packetSize = *(const int32_t*)(readBuf.buf() + 4);
+ packetSize = *(uint32_t*)(readBuf.buf() + 4);
if (!packetSize) {
// Emulator will get live-stuck here if packet size is read to be zero;
// crash right away so we can see these events.
@@ -353,7 +353,7 @@
}
int stat = 0;
- if (packetSize > (int)readBuf.validData()) {
+ if (packetSize > readBuf.validData()) {
stat = readBuf.getData(ioStream, packetSize);
if (stat <= 0) {
if (doSnapshotOperation(snapshotObjects, SnapshotState::StartSaving)) {
@@ -380,9 +380,9 @@
}
}
- DD("render thread read %d bytes, op %d, packet size %d",
- (int)readBuf.validData(), *(int32_t*)readBuf.buf(),
- *(int32_t*)(readBuf.buf() + 4));
+ DD("render thread read %i bytes, op %i, packet size %i",
+ readBuf.validData(), *(uint32_t*)readBuf.buf(),
+ *(uint32_t*)(readBuf.buf() + 4));
//
// log received bandwidth statistics
diff --git a/stream-servers/gles1_dec/gles1_dec.cpp b/stream-servers/gles1_dec/gles1_dec.cpp
index 16808f9..ecb93ad 100644
--- a/stream-servers/gles1_dec/gles1_dec.cpp
+++ b/stream-servers/gles1_dec/gles1_dec.cpp
@@ -34,7 +34,7 @@
const bool useChecksum = checksumSize > 0;
while (end - ptr >= 8) {
uint32_t opcode = *(uint32_t *)ptr;
- int32_t packetLen = *(int32_t *)(ptr + 4);
+ uint32_t packetLen = *(uint32_t *)(ptr + 4);
if (end - ptr < packetLen) return ptr - (unsigned char*)buf;
switch(opcode) {
case OP_glAlphaFunc: {
diff --git a/stream-servers/gles2_dec/gles2_dec.cpp b/stream-servers/gles2_dec/gles2_dec.cpp
index 2677030..425d472 100644
--- a/stream-servers/gles2_dec/gles2_dec.cpp
+++ b/stream-servers/gles2_dec/gles2_dec.cpp
@@ -34,7 +34,7 @@
const bool useChecksum = checksumSize > 0;
while (end - ptr >= 8) {
uint32_t opcode = *(uint32_t *)ptr;
- int32_t packetLen = *(int32_t *)(ptr + 4);
+ uint32_t packetLen = *(uint32_t *)(ptr + 4);
if (end - ptr < packetLen) return ptr - (unsigned char*)buf;
switch(opcode) {
case OP_glActiveTexture: {
diff --git a/stream-servers/renderControl_dec/renderControl_dec.cpp b/stream-servers/renderControl_dec/renderControl_dec.cpp
index 61c8f14..c6c3a6f 100644
--- a/stream-servers/renderControl_dec/renderControl_dec.cpp
+++ b/stream-servers/renderControl_dec/renderControl_dec.cpp
@@ -32,7 +32,7 @@
const unsigned char* const end = (const unsigned char*)buf + len;
while (end - ptr >= 8) {
uint32_t opcode = *(uint32_t *)ptr;
- int32_t packetLen = *(int32_t *)(ptr + 4);
+ uint32_t packetLen = *(uint32_t *)(ptr + 4);
if (end - ptr < packetLen) return ptr - (unsigned char*)buf;
// Do this on every iteration, as some commands may change the checksum
// calculation parameters.
diff --git a/stream-servers/vulkan/VkDecoder.cpp b/stream-servers/vulkan/VkDecoder.cpp
index 83ed0a6..58099d4 100644
--- a/stream-servers/vulkan/VkDecoder.cpp
+++ b/stream-servers/vulkan/VkDecoder.cpp
@@ -114,7 +114,7 @@
while (end - ptr >= 8)
{
uint32_t opcode = *(uint32_t *)ptr;
- int32_t packetLen = *(int32_t *)(ptr + 4);
+ uint32_t packetLen = *(uint32_t *)(ptr + 4);
if (end - ptr < packetLen) return ptr - (unsigned char*)buf;
stream()->setStream(ioStream);
VulkanStream* vkStream = stream();
diff --git a/stream-servers/vulkan/VkSubDecoder.cpp b/stream-servers/vulkan/VkSubDecoder.cpp
index a46005f..1597945 100644
--- a/stream-servers/vulkan/VkSubDecoder.cpp
+++ b/stream-servers/vulkan/VkSubDecoder.cpp
@@ -33,7 +33,7 @@
while (end - ptr >= 8)
{
uint32_t opcode = *(uint32_t *)ptr;
- int32_t packetLen = *(int32_t *)(ptr + 4);
+ uint32_t packetLen = *(uint32_t *)(ptr + 4);
if (end - ptr < packetLen) return ptr - (unsigned char*)buf;
readStream->setBuf((uint8_t*)(ptr + 8));
uint8_t* readStreamPtr = readStream->getBuf(); uint8_t** readStreamPtrPtr = &readStreamPtr;
