From a06ba836c76ea8b35aeca9d09bd7d3b043a4c962 Mon Sep 17 00:00:00 2001
From: Nat Goodspeed <nat@lindenlab.com>
Date: Thu, 16 Feb 2012 17:35:34 -0500
Subject: [PATCH] Fix bug in LLProcess::ReadPipe::peek() substring computation.
Add unit tests for peek() with substring args, reimplemented contains(),
various forms of find(). (yay unit tests)
---
indra/llcommon/llprocess.cpp | 3 +-
indra/llcommon/tests/llprocess_test.cpp | 61 ++++++++++++++++++++++---
2 files changed, 57 insertions(+), 7 deletions(-)
diff --git a/indra/llcommon/llprocess.cpp b/indra/llcommon/llprocess.cpp
index aa22b3f8054..add1649ba52 100644
--- a/indra/llcommon/llprocess.cpp
+++ b/indra/llcommon/llprocess.cpp
@@ -230,7 +230,8 @@ class ReadPipeImpl: public LLProcess::ReadPipe
{
// Constrain caller's offset and len to overlap actual buffer content.
std::size_t real_offset = (std::min)(mStreambuf.size(), std::size_t(offset));
- std::size_t real_end = (std::min)(mStreambuf.size(), std::size_t(real_offset + len));
+ size_type want_end = (len == npos)? npos : (real_offset + len);
+ std::size_t real_end = (std::min)(mStreambuf.size(), std::size_t(want_end));
boost::asio::streambuf::const_buffers_type cbufs = mStreambuf.data();
return std::string(boost::asio::buffers_begin(cbufs) + real_offset,
boost::asio::buffers_begin(cbufs) + real_end);
diff --git a/indra/llcommon/tests/llprocess_test.cpp b/indra/llcommon/tests/llprocess_test.cpp
index 29233d44158..e5d873c8eec 100644
--- a/indra/llcommon/tests/llprocess_test.cpp
+++ b/indra/llcommon/tests/llprocess_test.cpp
@@ -1130,7 +1130,7 @@ namespace tut
set_test_name("setLimit()");
PythonProcessLauncher py("setLimit()",
"import sys\n"
- "print sys.argv[1]\n");
+ "sys.stdout.write(sys.argv[1])\n");
std::string abc("abcdefghijklmnopqrstuvwxyz");
py.mParams.args.add(abc);
py.mParams.files.add(LLProcess::FileParam()); // stdin
@@ -1148,16 +1148,65 @@ namespace tut
// For all we know, that data could have arrived in several different
// bursts... probably not, but anyway, only check the last one.
ensure_equals("event[\"len\"]",
- listener.mHistory.back()["len"].asInteger(),
- abc.length() + sizeof(EOL) - 1);
+ listener.mHistory.back()["len"].asInteger(), abc.length());
ensure_equals("length of setLimit(10) data",
listener.mHistory.back()["data"].asString().length(), 10);
}
+ template<> template<>
+ void object::test<19>()
+ {
+ set_test_name("peek() ReadPipe data");
+ PythonProcessLauncher py("peek() ReadPipe",
+ "import sys\n"
+ "sys.stdout.write(sys.argv[1])\n");
+ std::string abc("abcdefghijklmnopqrstuvwxyz");
+ py.mParams.args.add(abc);
+ py.mParams.files.add(LLProcess::FileParam()); // stdin
+ py.mParams.files.add(LLProcess::FileParam("pipe")); // stdout
+ py.launch();
+ LLProcess::ReadPipe& childout(py.mPy->getReadPipe(LLProcess::STDOUT));
+ // okay, pump I/O to pick up output from child
+ waitfor(*py.mPy);
+ // peek() with substr args
+ ensure_equals("peek()", childout.peek(), abc);
+ ensure_equals("peek(23)", childout.peek(23), abc.substr(23));
+ ensure_equals("peek(5, 3)", childout.peek(5, 3), abc.substr(5, 3));
+ ensure_equals("peek(27, 2)", childout.peek(27, 2), "");
+ ensure_equals("peek(23, 5)", childout.peek(23, 5), "xyz");
+ // contains() -- we don't exercise as thoroughly as find() because the
+ // contains() implementation is trivially (and visibly) based on find()
+ ensure("contains(\":\")", ! childout.contains(":"));
+ ensure("contains(':')", ! childout.contains(':'));
+ ensure("contains(\"d\")", childout.contains("d"));
+ ensure("contains('d')", childout.contains("d"));
+ ensure("contains(\"klm\")", childout.contains("klm"));
+ ensure("contains(\"klx\")", ! childout.contains("klx"));
+ // find()
+ ensure("find(\":\")", childout.find(":") == LLProcess::ReadPipe::npos);
+ ensure("find(':')", childout.find(':') == LLProcess::ReadPipe::npos);
+ ensure_equals("find(\"d\")", childout.find("d"), 3);
+ ensure_equals("find('d')", childout.find("d"), 3);
+ ensure_equals("find(\"d\", 3)", childout.find("d", 3), 3);
+ ensure_equals("find('d', 3)", childout.find("d", 3), 3);
+ ensure("find(\"d\", 4)", childout.find("d", 4) == LLProcess::ReadPipe::npos);
+ ensure("find('d', 4)", childout.find('d', 4) == LLProcess::ReadPipe::npos);
+ // The case of offset == end and offset > end are different. In the
+ // first case, we can form a valid (albeit empty) iterator range and
+ // search that. In the second, guard logic in the implementation must
+ // realize we can't form a valid iterator range.
+ ensure("find(\"d\", 26)", childout.find("d", 26) == LLProcess::ReadPipe::npos);
+ ensure("find('d', 26)", childout.find('d', 26) == LLProcess::ReadPipe::npos);
+ ensure("find(\"d\", 27)", childout.find("d", 27) == LLProcess::ReadPipe::npos);
+ ensure("find('d', 27)", childout.find('d', 27) == LLProcess::ReadPipe::npos);
+ ensure_equals("find(\"ghi\")", childout.find("ghi"), 6);
+ ensure_equals("find(\"ghi\", 6)", childout.find("ghi"), 6);
+ ensure("find(\"ghi\", 7)", childout.find("ghi", 7) == LLProcess::ReadPipe::npos);
+ ensure("find(\"ghi\", 26)", childout.find("ghi", 26) == LLProcess::ReadPipe::npos);
+ ensure("find(\"ghi\", 27)", childout.find("ghi", 27) == LLProcess::ReadPipe::npos);
+ }
+
// TODO:
// test EOF -- check logging
- // test peek() with substr
- // test contains(char)
- // test find(string, offset), find(char, offset), offset <, =, > size()
} // namespace tut
--
GitLab