Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 2cf04119 authored by Elliott Hughes's avatar Elliott Hughes
Browse files

Remove String16::remove. 

This function, ironically, is being removed.

Even more amusing, it was never "remove" anyway --- it literally did
the opposite, and removed everything *except* the range you passed to
it, and should probably have been called "keep"!

I'm looking at reimplementing much of libutils, but first I'm improving
test coverage, and literally every test I wrote for this failed. And
then when I fixed the "obvious bugs" in the implementation, I found
there actually were a couple of existing unit tests --- that mostly
served to demonstrate just how counter-intuitive this function was.

Bug: http://b/156999009
Test: treehugger
Change-Id: I41fd85f7c0988070f4039f607d2e57523d862ed9
parent 8371ec25

libutils/String16.cpp

+0 −32
Original line number Diff line number Diff line
@@ -411,36 +411,4 @@ status_t String16::replaceAll(char16_t replaceThis, char16_t withThis) 
    return OK;

}



status_t String16::remove(size_t len, size_t begin)

{

    const size_t N = size();

    if (begin >= N) {

        release();

        mString = getEmptyString();

        return OK;

    }

    if (len > N || len > N - begin) len = N - begin;

    if (begin == 0 && len == N) {

        return OK;

    }



    if (begin > 0) {

        SharedBuffer* buf = static_cast<SharedBuffer*>(editResize((N + 1) * sizeof(char16_t)));

        if (!buf) {

            return NO_MEMORY;

        }

        char16_t* str = (char16_t*)buf->data();

        memmove(str, str+begin, (N-begin+1)*sizeof(char16_t));

        mString = str;

    }

    SharedBuffer* buf = static_cast<SharedBuffer*>(editResize((len + 1) * sizeof(char16_t)));

    if (buf) {

        char16_t* str = (char16_t*)buf->data();

        str[len] = 0;

        mString = str;

        return OK;

    }

    return NO_MEMORY;

}



}; // namespace android

libutils/String16_fuzz.cpp

+0 −8
Original line number Diff line number Diff line
@@ -72,12 +72,6 @@ std::vector<std::function<void(FuzzedDataProvider&, android::String16, android:: 
                    char16_t replaceChar = dataProvider.ConsumeIntegral<char16_t>();

                    str1.replaceAll(findChar, replaceChar);

                }),

                ([](FuzzedDataProvider& dataProvider, android::String16 str1,

                    android::String16) -> void {

                    size_t len = dataProvider.ConsumeIntegral<size_t>();

                    size_t begin = dataProvider.ConsumeIntegral<size_t>();

                    str1.remove(len, begin);

                }),

};



void callFunc(uint8_t index, FuzzedDataProvider& dataProvider, android::String16 str1,
@@ -111,7 +105,5 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { 
        callFunc(op, dataProvider, str_one_utf16, str_two_utf16);

    }



    str_one_utf16.remove(0, str_one_utf16.size());

    str_two_utf16.remove(0, str_two_utf16.size());

    return 0;

}

libutils/String16_test.cpp

+0 −15
Original line number Diff line number Diff line
@@ -90,13 +90,6 @@ TEST(String16Test, Insert) { 
    EXPECT_STR16EQ(u"VerifyInsert me", tmp);

}



TEST(String16Test, Remove) {

    String16 tmp("Verify me");

    tmp.remove(2, 6);

    EXPECT_EQ(2U, tmp.size());

    EXPECT_STR16EQ(u" m", tmp);

}



TEST(String16Test, ReplaceAll) {

    String16 tmp("Verify verify Verify");

    tmp.replaceAll(u'r', u'!');
@@ -161,14 +154,6 @@ TEST(String16Test, StaticStringInsert) { 
    EXPECT_FALSE(tmp.isStaticString());

}



TEST(String16Test, StaticStringRemove) {

    StaticString16 tmp(u"Verify me");

    tmp.remove(2, 6);

    EXPECT_EQ(2U, tmp.size());

    EXPECT_STR16EQ(u" m", tmp);

    EXPECT_FALSE(tmp.isStaticString());

}



TEST(String16Test, StaticStringReplaceAll) {

    StaticString16 tmp(u"Verify verify Verify");

    tmp.replaceAll(u'r', u'!');

libutils/include/utils/String16.h

+0 −2
Original line number Diff line number Diff line
@@ -88,8 +88,6 @@ public: 
            status_t            replaceAll(char16_t replaceThis,

                                           char16_t withThis);



            status_t            remove(size_t len, size_t begin=0);



    inline  int                 compare(const String16& other) const;



    inline  bool                operator<(const String16& other) const;