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

Commit dec9caf1 authored by Dmitrii Merkurev's avatar Dmitrii Merkurev Committed by Automerger Merge Worker
Browse files

Merge "fastboot: get rid of manual transport memory management" into main am: f667b6d8 

Original change: https://android-review.googlesource.com/c/platform/system/core/+/2736776



Change-Id: Icf0624a7ee50d493c4a2e5a7417d9918fae411ef
Signed-off-by: default avatarAutomerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
parents baced3df f667b6d8

fastboot/fastboot.cpp

+27 −31
Original line number Diff line number Diff line
@@ -350,23 +350,22 @@ Result<NetworkSerial, FastbootError> ParseNetworkSerial(const std::string& seria 
//

// The returned Transport is a singleton, so multiple calls to this function will return the same

// object, and the caller should not attempt to delete the returned Transport.

static Transport* open_device(const char* local_serial, bool wait_for_device = true,

static std::unique_ptr<Transport> open_device(const char* local_serial,

                                              bool wait_for_device = true,

                                              bool announce = true) {

    const Result<NetworkSerial, FastbootError> network_serial = ParseNetworkSerial(local_serial);



    Transport* transport = nullptr;

    std::unique_ptr<Transport> transport;

    while (true) {

        if (network_serial.ok()) {

            std::string error;

            if (network_serial->protocol == Socket::Protocol::kTcp) {

                transport = tcp::Connect(network_serial->address, network_serial->port, &error)

                                    .release();

                transport = tcp::Connect(network_serial->address, network_serial->port, &error);

            } else if (network_serial->protocol == Socket::Protocol::kUdp) {

                transport = udp::Connect(network_serial->address, network_serial->port, &error)

                                    .release();

                transport = udp::Connect(network_serial->address, network_serial->port, &error);

            }



            if (transport == nullptr && announce) {

            if (!transport && announce) {

                LOG(ERROR) << "error: " << error;

            }

        } else if (network_serial.error().code() ==
@@ -378,12 +377,12 @@ static Transport* open_device(const char* local_serial, bool wait_for_device = t 
            Expect(network_serial);

        }



        if (transport != nullptr) {

        if (transport) {

            return transport;

        }



        if (!wait_for_device) {

            return nullptr;

            return transport;

        }



        if (announce) {
@@ -394,9 +393,9 @@ static Transport* open_device(const char* local_serial, bool wait_for_device = t 
    }

}



static Transport* NetworkDeviceConnected(bool print = false) {

    Transport* transport = nullptr;

    Transport* result = nullptr;

static std::unique_ptr<Transport> NetworkDeviceConnected(bool print = false) {

    std::unique_ptr<Transport> transport;

    std::unique_ptr<Transport> result;



    ConnectedDevicesStorage storage;

    std::set<std::string> devices;
@@ -409,11 +408,11 @@ static Transport* NetworkDeviceConnected(bool print = false) { 
        transport = open_device(device.c_str(), false, false);



        if (print) {

            PrintDevice(device.c_str(), transport == nullptr ? "offline" : "fastboot");

            PrintDevice(device.c_str(), transport ? "offline" : "fastboot");

        }



        if (transport != nullptr) {

            result = transport;

        if (transport) {

            result = std::move(transport);

        }

    }
@@ -431,21 +430,21 @@ static Transport* NetworkDeviceConnected(bool print = false) { 
//

// The returned Transport is a singleton, so multiple calls to this function will return the same

// object, and the caller should not attempt to delete the returned Transport.

static Transport* open_device() {

static std::unique_ptr<Transport> open_device() {

    if (serial != nullptr) {

        return open_device(serial);

    }



    bool announce = true;

    Transport* transport = nullptr;

    std::unique_ptr<Transport> transport;

    while (true) {

        transport = usb_open(match_fastboot(nullptr));

        if (transport != nullptr) {

        if (transport) {

            return transport;

        }



        transport = NetworkDeviceConnected();

        if (transport != nullptr) {

        if (transport) {

            return transport;

        }
@@ -455,6 +454,8 @@ static Transport* open_device() { 
        }

        std::this_thread::sleep_for(std::chrono::seconds(1));

    }



    return transport;

}



static int Connect(int argc, char* argv[]) {
@@ -466,8 +467,7 @@ static int Connect(int argc, char* argv[]) { 
    const char* local_serial = *argv;

    Expect(ParseNetworkSerial(local_serial));



    const Transport* transport = open_device(local_serial, false);

    if (transport == nullptr) {

    if (!open_device(local_serial, false)) {

        return 1;

    }
@@ -531,6 +531,7 @@ static void list_devices() { 
    usb_open(list_devices_callback);

    NetworkDeviceConnected(/* print */ true);

}



void syntax_error(const char* fmt, ...) {

    fprintf(stderr, "fastboot: usage: ");
@@ -1547,9 +1548,7 @@ bool is_userspace_fastboot() { 


void reboot_to_userspace_fastboot() {

    fb->RebootTo("fastboot");



    auto* old_transport = fb->set_transport(nullptr);

    delete old_transport;

    fb->set_transport(nullptr);



    // Give the current connection time to close.

    std::this_thread::sleep_for(std::chrono::seconds(1));
@@ -2372,8 +2371,8 @@ int FastBootTool::Main(int argc, char* argv[]) { 
        return show_help();

    }



    Transport* transport = open_device();

    if (transport == nullptr) {

    std::unique_ptr<Transport> transport = open_device();

    if (!transport) {

        return 1;

    }

    fastboot::DriverCallbacks driver_callbacks = {
@@ -2383,7 +2382,7 @@ int FastBootTool::Main(int argc, char* argv[]) { 
            .text = TextMessage,

    };



    fastboot::FastBootDriver fastboot_driver(transport, driver_callbacks, false);

    fastboot::FastBootDriver fastboot_driver(std::move(transport), driver_callbacks, false);

    fb = &fastboot_driver;

    fp->fb = &fastboot_driver;
@@ -2628,9 +2627,6 @@ int FastBootTool::Main(int argc, char* argv[]) { 
    }

    fprintf(stderr, "Finished. Total time: %.3fs\n", (now() - start));



    auto* old_transport = fb->set_transport(nullptr);

    delete old_transport;



    return 0;

}

fastboot/fastboot_driver.cpp

+5 −5
Original line number Diff line number Diff line
@@ -58,9 +58,10 @@ using namespace android::storage_literals; 
namespace fastboot {



/*************************** PUBLIC *******************************/

FastBootDriver::FastBootDriver(Transport* transport, DriverCallbacks driver_callbacks,

FastBootDriver::FastBootDriver(std::unique_ptr<Transport> transport,

                               DriverCallbacks driver_callbacks,

                               bool no_checks)

    : transport_(transport),

    : transport_(std::move(transport)),

      prolog_(std::move(driver_callbacks.prolog)),

      epilog_(std::move(driver_callbacks.epilog)),

      info_(std::move(driver_callbacks.info)),
@@ -627,9 +628,8 @@ int FastBootDriver::SparseWriteCallback(std::vector<char>& tpbuf, const char* da 
    return 0;

}



Transport* FastBootDriver::set_transport(Transport* transport) {

    std::swap(transport_, transport);

    return transport;

void FastBootDriver::set_transport(std::unique_ptr<Transport> transport) {

    transport_ = std::move(transport);

}



}  // End namespace fastboot

fastboot/fastboot_driver.h

+4 −5
Original line number Diff line number Diff line
@@ -30,6 +30,7 @@ 
#include <deque>

#include <functional>

#include <limits>

#include <memory>

#include <string>

#include <vector>
@@ -63,7 +64,7 @@ class FastBootDriver : public IFastBootDriver { 
    static constexpr uint32_t MAX_DOWNLOAD_SIZE = std::numeric_limits<uint32_t>::max();

    static constexpr size_t TRANSPORT_CHUNK_SIZE = 1024;



    FastBootDriver(Transport* transport, DriverCallbacks driver_callbacks = {},

    FastBootDriver(std::unique_ptr<Transport> transport, DriverCallbacks driver_callbacks = {},

                   bool no_checks = false);

    ~FastBootDriver();
@@ -124,9 +125,7 @@ class FastBootDriver : public IFastBootDriver { 
    std::string Error();

    RetCode WaitForDisconnect() override;



    // Note: set_transport will return the previous transport.

    Transport* set_transport(Transport* transport);

    Transport* transport() const { return transport_; }

    void set_transport(std::unique_ptr<Transport> transport);



    RetCode RawCommand(const std::string& cmd, const std::string& message,

                       std::string* response = nullptr, std::vector<std::string>* info = nullptr,
@@ -143,7 +142,7 @@ class FastBootDriver : public IFastBootDriver { 


    std::string ErrnoStr(const std::string& msg);



    Transport* transport_;

    std::unique_ptr<Transport> transport_;



  private:

    RetCode SendBuffer(android::base::borrowed_fd fd, size_t size);

fastboot/fastboot_driver_test.cpp

+21 −17
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@ 


#include "fastboot_driver.h"



#include <memory>

#include <optional>



#include <gtest/gtest.h>
@@ -30,13 +31,14 @@ class DriverTest : public ::testing::Test { 
};



TEST_F(DriverTest, GetVar) {

    MockTransport transport;

    FastBootDriver driver(&transport);

    std::unique_ptr<MockTransport> transport_pointer = std::make_unique<MockTransport>();

    MockTransport* transport = transport_pointer.get();

    FastBootDriver driver(std::move(transport_pointer));



    EXPECT_CALL(transport, Write(_, _))

    EXPECT_CALL(*transport, Write(_, _))

            .With(AllArgs(RawData("getvar:version")))

            .WillOnce(ReturnArg<1>());

    EXPECT_CALL(transport, Read(_, _)).WillOnce(Invoke(CopyData("OKAY0.4")));

    EXPECT_CALL(*transport, Read(_, _)).WillOnce(Invoke(CopyData("OKAY0.4")));



    std::string output;

    ASSERT_EQ(driver.GetVar("version", &output), SUCCESS) << driver.Error();
@@ -44,14 +46,15 @@ TEST_F(DriverTest, GetVar) { 
}



TEST_F(DriverTest, InfoMessage) {

    MockTransport transport;

    FastBootDriver driver(&transport);

    std::unique_ptr<MockTransport> transport_pointer = std::make_unique<MockTransport>();

    MockTransport* transport = transport_pointer.get();

    FastBootDriver driver(std::move(transport_pointer));



    EXPECT_CALL(transport, Write(_, _))

    EXPECT_CALL(*transport, Write(_, _))

            .With(AllArgs(RawData("oem dmesg")))

            .WillOnce(ReturnArg<1>());

    EXPECT_CALL(transport, Read(_, _)).WillOnce(Invoke(CopyData("INFOthis is an info line")));

    EXPECT_CALL(transport, Read(_, _)).WillOnce(Invoke(CopyData("OKAY")));

    EXPECT_CALL(*transport, Read(_, _)).WillOnce(Invoke(CopyData("INFOthis is an info line")));

    EXPECT_CALL(*transport, Read(_, _)).WillOnce(Invoke(CopyData("OKAY")));



    std::vector<std::string> info;

    ASSERT_EQ(driver.RawCommand("oem dmesg", "", nullptr, &info), SUCCESS) << driver.Error();
@@ -60,28 +63,29 @@ TEST_F(DriverTest, InfoMessage) { 
}



TEST_F(DriverTest, TextMessage) {

    MockTransport transport;

    std::string text;

    std::unique_ptr<MockTransport> transport_pointer = std::make_unique<MockTransport>();

    MockTransport* transport = transport_pointer.get();



    DriverCallbacks callbacks{[](const std::string&) {}, [](int) {}, [](const std::string&) {},

                              [&text](const std::string& extra_text) { text += extra_text; }};



    FastBootDriver driver(&transport, callbacks);

    FastBootDriver driver(std::move(transport_pointer), callbacks);



    EXPECT_CALL(transport, Write(_, _))

    EXPECT_CALL(*transport, Write(_, _))

            .With(AllArgs(RawData("oem trusty runtest trusty.hwaes.bench")))

            .WillOnce(ReturnArg<1>());

    EXPECT_CALL(transport, Read(_, _)).WillOnce(Invoke(CopyData("TEXTthis is a text line")));

    EXPECT_CALL(transport, Read(_, _))

    EXPECT_CALL(*transport, Read(_, _)).WillOnce(Invoke(CopyData("TEXTthis is a text line")));

    EXPECT_CALL(*transport, Read(_, _))

            .WillOnce(Invoke(

                    CopyData("TEXT, albeit very long and split over multiple TEXT messages.")));

    EXPECT_CALL(transport, Read(_, _))

    EXPECT_CALL(*transport, Read(_, _))

            .WillOnce(Invoke(CopyData("TEXT Indeed we can do that now with a TEXT message whenever "

                                      "we feel like it.")));

    EXPECT_CALL(transport, Read(_, _))

    EXPECT_CALL(*transport, Read(_, _))

            .WillOnce(Invoke(CopyData("TEXT Isn't that truly super cool?")));



    EXPECT_CALL(transport, Read(_, _)).WillOnce(Invoke(CopyData("OKAY")));

    EXPECT_CALL(*transport, Read(_, _)).WillOnce(Invoke(CopyData("OKAY")));



    std::vector<std::string> info;

    ASSERT_EQ(driver.RawCommand("oem trusty runtest trusty.hwaes.bench", "", nullptr, &info),

fastboot/fuzzy_fastboot/fixtures.cpp

+5 −5
Original line number Diff line number Diff line
@@ -128,7 +128,7 @@ void FastBootTest::SetUp() { 
            return MatchFastboot(info, device_serial);

        };

        for (int i = 0; i < MAX_USB_TRIES && !transport; i++) {

            std::unique_ptr<UsbTransport> usb(usb_open(matcher, USB_TIMEOUT));

            std::unique_ptr<UsbTransport> usb = usb_open(matcher, USB_TIMEOUT);

            if (usb)

                transport = std::unique_ptr<TransportSniffer>(

                        new TransportSniffer(std::move(usb), serial_port));
@@ -143,7 +143,7 @@ void FastBootTest::SetUp() { 
    } else {

        ASSERT_EQ(device_path, cb_scratch);  // The path can not change

    }

    fb = std::unique_ptr<FastBootDriver>(new FastBootDriver(transport.get(), {}, true));

    fb = std::unique_ptr<FastBootDriver>(new FastBootDriver(std::move(transport), {}, true));

    // No error checking since non-A/B devices may not support the command

    fb->GetVar("current-slot", &initial_slot);

}
@@ -200,7 +200,7 @@ void FastBootTest::ReconnectFastbootDevice() { 
    if (IsFastbootOverTcp()) {

        ConnectTcpFastbootDevice();

        device_path = cb_scratch;

        fb = std::unique_ptr<FastBootDriver>(new FastBootDriver(transport.get(), {}, true));

        fb = std::unique_ptr<FastBootDriver>(new FastBootDriver(std::move(transport), {}, true));

        return;

    }
@@ -212,7 +212,7 @@ void FastBootTest::ReconnectFastbootDevice() { 
        return MatchFastboot(info, device_serial);

    };

    while (!transport) {

        std::unique_ptr<UsbTransport> usb(usb_open(matcher, USB_TIMEOUT));

        std::unique_ptr<UsbTransport> usb = usb_open(matcher, USB_TIMEOUT);

        if (usb) {

            transport = std::unique_ptr<TransportSniffer>(

                    new TransportSniffer(std::move(usb), serial_port));
@@ -220,7 +220,7 @@ void FastBootTest::ReconnectFastbootDevice() { 
        std::this_thread::sleep_for(1s);

    }

    device_path = cb_scratch;

    fb = std::unique_ptr<FastBootDriver>(new FastBootDriver(transport.get(), {}, true));

    fb = std::unique_ptr<FastBootDriver>(new FastBootDriver(std::move(transport), {}, true));

}



void FastBootTest::SetLockState(bool unlock, bool assert_change) {