diff --git a/init/Android.bp b/init/Android.bp index 9b02c381a..66427dcbb 100644 --- a/init/Android.bp +++ b/init/Android.bp @@ -448,6 +448,7 @@ cc_test { srcs: [ "devices_test.cpp", + "epoll_test.cpp", "firmware_handler_test.cpp", "init_test.cpp", "keychords_test.cpp", diff --git a/init/epoll.cpp b/init/epoll.cpp index 17d63fa5d..74d8aac96 100644 --- a/init/epoll.cpp +++ b/init/epoll.cpp @@ -38,11 +38,12 @@ Result Epoll::Open() { return {}; } -Result Epoll::RegisterHandler(int fd, std::function handler, uint32_t events) { +Result Epoll::RegisterHandler(int fd, Handler handler, uint32_t events) { if (!events) { return Error() << "Must specify events"; } - auto [it, inserted] = epoll_handlers_.emplace(fd, std::move(handler)); + auto sp = std::make_shared(std::move(handler)); + auto [it, inserted] = epoll_handlers_.emplace(fd, std::move(sp)); if (!inserted) { return Error() << "Cannot specify two epoll handlers for a given FD"; } @@ -69,7 +70,7 @@ Result Epoll::UnregisterHandler(int fd) { return {}; } -Result*>> Epoll::Wait( +Result>> Epoll::Wait( std::optional timeout) { int timeout_ms = -1; if (timeout && timeout->count() < INT_MAX) { @@ -81,9 +82,10 @@ Result*>> Epoll::Wait( if (num_events == -1) { return ErrnoError() << "epoll_wait failed"; } - std::vector*> pending_functions; + std::vector> pending_functions; for (int i = 0; i < num_events; ++i) { - pending_functions.emplace_back(reinterpret_cast*>(ev[i].data.ptr)); + auto sp = *reinterpret_cast*>(ev[i].data.ptr); + pending_functions.emplace_back(std::move(sp)); } return pending_functions; diff --git a/init/epoll.h b/init/epoll.h index c32a6614f..0df528935 100644 --- a/init/epoll.h +++ b/init/epoll.h @@ -22,6 +22,7 @@ #include #include #include +#include #include #include @@ -36,15 +37,17 @@ class Epoll { public: Epoll(); + typedef std::function Handler; + Result Open(); - Result RegisterHandler(int fd, std::function handler, uint32_t events = EPOLLIN); + Result RegisterHandler(int fd, Handler handler, uint32_t events = EPOLLIN); Result UnregisterHandler(int fd); - Result*>> Wait( + Result>> Wait( std::optional timeout); private: android::base::unique_fd epoll_fd_; - std::map> epoll_handlers_; + std::map> epoll_handlers_; }; } // namespace init diff --git a/init/epoll_test.cpp b/init/epoll_test.cpp new file mode 100644 index 000000000..9236cd53e --- /dev/null +++ b/init/epoll_test.cpp @@ -0,0 +1,76 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "epoll.h" + +#include + +#include + +#include +#include + +namespace android { +namespace init { + +std::unordered_set sValidObjects; + +class CatchDtor final { + public: + CatchDtor() { sValidObjects.emplace(this); } + CatchDtor(const CatchDtor&) { sValidObjects.emplace(this); } + ~CatchDtor() { + auto iter = sValidObjects.find(this); + if (iter != sValidObjects.end()) { + sValidObjects.erase(iter); + } + } +}; + +TEST(epoll, UnregisterHandler) { + Epoll epoll; + ASSERT_RESULT_OK(epoll.Open()); + + int fds[2]; + ASSERT_EQ(pipe(fds), 0); + + CatchDtor catch_dtor; + bool handler_invoked; + auto handler = [&, catch_dtor]() -> void { + auto result = epoll.UnregisterHandler(fds[0]); + ASSERT_EQ(result.ok(), !handler_invoked); + handler_invoked = true; + ASSERT_NE(sValidObjects.find((void*)&catch_dtor), sValidObjects.end()); + }; + + epoll.RegisterHandler(fds[0], std::move(handler)); + + uint8_t byte = 0xee; + ASSERT_TRUE(android::base::WriteFully(fds[1], &byte, sizeof(byte))); + + auto results = epoll.Wait({}); + ASSERT_RESULT_OK(results); + ASSERT_EQ(results->size(), size_t(1)); + + for (const auto& function : *results) { + (*function)(); + (*function)(); + } + ASSERT_TRUE(handler_invoked); +} + +} // namespace init +} // namespace android diff --git a/libutils/String16.cpp b/libutils/String16.cpp index c42cada97..68642d84f 100644 --- a/libutils/String16.cpp +++ b/libutils/String16.cpp @@ -199,99 +199,59 @@ status_t String16::setTo(const char16_t* other, size_t len) return NO_MEMORY; } -status_t String16::append(const String16& other) -{ - const size_t myLen = size(); - const size_t otherLen = other.size(); - if (myLen == 0) { - setTo(other); - return OK; - } else if (otherLen == 0) { - return OK; - } - - if (myLen >= SIZE_MAX / sizeof(char16_t) - otherLen) { - android_errorWriteLog(0x534e4554, "73826242"); - abort(); - } - - SharedBuffer* buf = - static_cast(editResize((myLen + otherLen + 1) * sizeof(char16_t))); - if (buf) { - char16_t* str = (char16_t*)buf->data(); - memcpy(str+myLen, other, (otherLen+1)*sizeof(char16_t)); - mString = str; - return OK; - } - return NO_MEMORY; +status_t String16::append(const String16& other) { + return append(other.string(), other.size()); } -status_t String16::append(const char16_t* chrs, size_t otherLen) -{ +status_t String16::append(const char16_t* chrs, size_t otherLen) { const size_t myLen = size(); - if (myLen == 0) { - setTo(chrs, otherLen); - return OK; - } else if (otherLen == 0) { - return OK; - } - if (myLen >= SIZE_MAX / sizeof(char16_t) - otherLen) { - android_errorWriteLog(0x534e4554, "73826242"); - abort(); - } + if (myLen == 0) return setTo(chrs, otherLen); - SharedBuffer* buf = - static_cast(editResize((myLen + otherLen + 1) * sizeof(char16_t))); - if (buf) { - char16_t* str = (char16_t*)buf->data(); - memcpy(str+myLen, chrs, otherLen*sizeof(char16_t)); - str[myLen+otherLen] = 0; - mString = str; - return OK; - } - return NO_MEMORY; + if (otherLen == 0) return OK; + + size_t size = myLen; + if (__builtin_add_overflow(size, otherLen, &size) || + __builtin_add_overflow(size, 1, &size) || + __builtin_mul_overflow(size, sizeof(char16_t), &size)) return NO_MEMORY; + + SharedBuffer* buf = static_cast(editResize(size)); + if (!buf) return NO_MEMORY; + + char16_t* str = static_cast(buf->data()); + memcpy(str + myLen, chrs, otherLen * sizeof(char16_t)); + str[myLen + otherLen] = 0; + mString = str; + return OK; } -status_t String16::insert(size_t pos, const char16_t* chrs) -{ +status_t String16::insert(size_t pos, const char16_t* chrs) { return insert(pos, chrs, strlen16(chrs)); } -status_t String16::insert(size_t pos, const char16_t* chrs, size_t len) -{ +status_t String16::insert(size_t pos, const char16_t* chrs, size_t otherLen) { const size_t myLen = size(); - if (myLen == 0) { - return setTo(chrs, len); - return OK; - } else if (len == 0) { - return OK; - } + + if (myLen == 0) return setTo(chrs, otherLen); + + if (otherLen == 0) return OK; if (pos > myLen) pos = myLen; - #if 0 - printf("Insert in to %s: pos=%d, len=%d, myLen=%d, chrs=%s\n", - String8(*this).string(), pos, - len, myLen, String8(chrs, len).string()); - #endif + size_t size = myLen; + if (__builtin_add_overflow(size, otherLen, &size) || + __builtin_add_overflow(size, 1, &size) || + __builtin_mul_overflow(size, sizeof(char16_t), &size)) return NO_MEMORY; - SharedBuffer* buf = - static_cast(editResize((myLen + len + 1) * sizeof(char16_t))); - if (buf) { - char16_t* str = (char16_t*)buf->data(); - if (pos < myLen) { - memmove(str+pos+len, str+pos, (myLen-pos)*sizeof(char16_t)); - } - memcpy(str+pos, chrs, len*sizeof(char16_t)); - str[myLen+len] = 0; - mString = str; - #if 0 - printf("Result (%d chrs): %s\n", size(), String8(*this).string()); - #endif - return OK; - } - return NO_MEMORY; + SharedBuffer* buf = static_cast(editResize(size)); + if (!buf) return NO_MEMORY; + + char16_t* str = static_cast(buf->data()); + if (pos < myLen) memmove(str + pos + otherLen, str + pos, (myLen - pos) * sizeof(char16_t)); + memcpy(str + pos, chrs, otherLen * sizeof(char16_t)); + str[myLen + otherLen] = 0; + mString = str; + return OK; } ssize_t String16::findFirst(char16_t c) const diff --git a/libutils/String16_test.cpp b/libutils/String16_test.cpp index 7d7230ef9..c6e6f746a 100644 --- a/libutils/String16_test.cpp +++ b/libutils/String16_test.cpp @@ -19,7 +19,7 @@ #include -namespace android { +using namespace android; ::testing::AssertionResult Char16_tStringEquals(const char16_t* a, const char16_t* b) { if (strcmp16(a, b) != 0) { @@ -224,4 +224,36 @@ TEST(String16Test, ValidUtf8Conversion) { EXPECT_STR16EQ(another, u"abcdef"); } -} // namespace android +TEST(String16Test, append) { + String16 s; + EXPECT_EQ(OK, s.append(String16(u"foo"))); + EXPECT_STR16EQ(u"foo", s); + EXPECT_EQ(OK, s.append(String16(u"bar"))); + EXPECT_STR16EQ(u"foobar", s); + EXPECT_EQ(OK, s.append(u"baz", 0)); + EXPECT_STR16EQ(u"foobar", s); + EXPECT_EQ(NO_MEMORY, s.append(u"baz", SIZE_MAX)); + EXPECT_STR16EQ(u"foobar", s); +} + +TEST(String16Test, insert) { + String16 s; + + // Inserting into the empty string inserts at the start. + EXPECT_EQ(OK, s.insert(123, u"foo")); + EXPECT_STR16EQ(u"foo", s); + + // Inserting zero characters at any position is okay, but won't expand the string. + EXPECT_EQ(OK, s.insert(123, u"foo", 0)); + EXPECT_STR16EQ(u"foo", s); + + // Inserting past the end of a non-empty string appends. + EXPECT_EQ(OK, s.insert(123, u"bar")); + EXPECT_STR16EQ(u"foobar", s); + + EXPECT_EQ(OK, s.insert(3, u"!")); + EXPECT_STR16EQ(u"foo!bar", s); + + EXPECT_EQ(NO_MEMORY, s.insert(3, u"", SIZE_MAX)); + EXPECT_STR16EQ(u"foo!bar", s); +} diff --git a/libutils/String8.cpp b/libutils/String8.cpp index 8511da9ce..419b2deed 100644 --- a/libutils/String8.cpp +++ b/libutils/String8.cpp @@ -313,8 +313,8 @@ status_t String8::appendFormatV(const char* fmt, va_list args) if (n > 0) { size_t oldLength = length(); - if ((size_t)n > SIZE_MAX - 1 || - oldLength > SIZE_MAX - (size_t)n - 1) { + if (n > std::numeric_limits::max() - 1 || + oldLength > std::numeric_limits::max() - n - 1) { return NO_MEMORY; } char* buf = lockBuffer(oldLength + n); @@ -327,21 +327,23 @@ status_t String8::appendFormatV(const char* fmt, va_list args) return result; } -status_t String8::real_append(const char* other, size_t otherLen) -{ +status_t String8::real_append(const char* other, size_t otherLen) { const size_t myLen = bytes(); - SharedBuffer* buf = SharedBuffer::bufferFromData(mString) - ->editResize(myLen+otherLen+1); - if (buf) { - char* str = (char*)buf->data(); - mString = str; - str += myLen; - memcpy(str, other, otherLen); - str[otherLen] = '\0'; - return OK; + SharedBuffer* buf; + size_t newLen; + if (__builtin_add_overflow(myLen, otherLen, &newLen) || + __builtin_add_overflow(newLen, 1, &newLen) || + (buf = SharedBuffer::bufferFromData(mString)->editResize(newLen)) == nullptr) { + return NO_MEMORY; } - return NO_MEMORY; + + char* str = (char*)buf->data(); + mString = str; + str += myLen; + memcpy(str, other, otherLen); + str[otherLen] = '\0'; + return OK; } char* String8::lockBuffer(size_t size) diff --git a/libutils/String8_test.cpp b/libutils/String8_test.cpp index 9efcc6fa4..1356cd08f 100644 --- a/libutils/String8_test.cpp +++ b/libutils/String8_test.cpp @@ -15,13 +15,14 @@ */ #define LOG_TAG "String8_test" + #include #include #include #include -namespace android { +using namespace android; class String8Test : public testing::Test { protected: @@ -101,4 +102,15 @@ TEST_F(String8Test, ValidUtf16Conversion) { String8 valid = String8(String16(tmp)); EXPECT_STREQ(valid, "abcdef"); } + +TEST_F(String8Test, append) { + String8 s; + EXPECT_EQ(OK, s.append("foo")); + EXPECT_STREQ("foo", s); + EXPECT_EQ(OK, s.append("bar")); + EXPECT_STREQ("foobar", s); + EXPECT_EQ(OK, s.append("baz", 0)); + EXPECT_STREQ("foobar", s); + EXPECT_EQ(NO_MEMORY, s.append("baz", SIZE_MAX)); + EXPECT_STREQ("foobar", s); }