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

Commit 4039051d authored by Josh Gao's avatar Josh Gao
Browse files

adb: clean up handle_host_request. 

Previously, we were returning the result of SendOkay/SendFail in a few
places after handling a host request, which is incorrect for two
reasons. First, the return type of SendOkay/SendFail is bool, and
handle_host_request was expected to return 0 on success. Second, we
don't care if the SendOkay fails; if we got to that point, we're done
with the request, regardless of whether we succeeded to report our
result. The result of this was a spurious failure result reported after
the initial result, which was ignored by the adb client.

Test: manually straced adb server
Test: python test_adb.py
Test: python test_device.py
Change-Id: I7d45ba527e1faccbbae5b15e7a0d1557b0a84858
parent 6b0ecdab

adb/adb.cpp

+36 −25
Original line number Diff line number Diff line
@@ -1044,7 +1044,7 @@ static int SendOkay(int fd, const std::string& s) { 
    return 0;

}



int handle_host_request(const char* service, TransportType type, const char* serial,

bool handle_host_request(const char* service, TransportType type, const char* serial,

                        TransportId transport_id, int reply_fd, asocket* s) {

    if (strcmp(service, "kill") == 0) {

        fprintf(stderr, "adb server killed by remote request\n");
@@ -1070,7 +1070,7 @@ int handle_host_request(const char* service, TransportType type, const char* ser 
            transport_id = strtoll(service, const_cast<char**>(&service), 10);

            if (*service != '\0') {

                SendFail(reply_fd, "invalid transport id");

                return 1;

                return true;

            }

        } else if (!strncmp(service, "transport-usb", strlen("transport-usb"))) {

            type = kTransportUsb;
@@ -1088,10 +1088,13 @@ int handle_host_request(const char* service, TransportType type, const char* ser 
        if (t != nullptr) {

            s->transport = t;

            SendOkay(reply_fd);



            // We succesfully handled the device selection, but there's another request coming.

            return false;

        } else {

            SendFail(reply_fd, error);

            return true;

        }

        return 1;

    }



    // return a list of all connected devices
@@ -1101,9 +1104,9 @@ int handle_host_request(const char* service, TransportType type, const char* ser 
            D("Getting device list...");

            std::string device_list = list_transports(long_listing);

            D("Sending device list...");

            return SendOkay(reply_fd, device_list);

            SendOkay(reply_fd, device_list);

        }

        return 1;

        return true;

    }



    if (!strcmp(service, "reconnect-offline")) {
@@ -1119,7 +1122,7 @@ int handle_host_request(const char* service, TransportType type, const char* ser 
            response.resize(response.size() - 1);

        }

        SendOkay(reply_fd, response);

        return 0;

        return true;

    }



    if (!strcmp(service, "features")) {
@@ -1130,7 +1133,7 @@ int handle_host_request(const char* service, TransportType type, const char* ser 
        } else {

            SendFail(reply_fd, error);

        }

        return 0;

        return true;

    }



    if (!strcmp(service, "host-features")) {
@@ -1141,7 +1144,7 @@ int handle_host_request(const char* service, TransportType type, const char* ser 
        }

        features.insert(kFeaturePushSync);

        SendOkay(reply_fd, FeatureSetToString(features));

        return 0;

        return true;

    }



    // remove TCP transport
@@ -1149,7 +1152,8 @@ int handle_host_request(const char* service, TransportType type, const char* ser 
        const std::string address(service + 11);

        if (address.empty()) {

            kick_all_tcp_devices();

            return SendOkay(reply_fd, "disconnected everything");

            SendOkay(reply_fd, "disconnected everything");

            return true;

        }



        std::string serial;
@@ -1157,21 +1161,24 @@ int handle_host_request(const char* service, TransportType type, const char* ser 
        int port = DEFAULT_ADB_LOCAL_TRANSPORT_PORT;

        std::string error;

        if (!android::base::ParseNetAddress(address, &host, &port, &serial, &error)) {

            return SendFail(reply_fd, android::base::StringPrintf("couldn't parse '%s': %s",

            SendFail(reply_fd, android::base::StringPrintf("couldn't parse '%s': %s",

                                                           address.c_str(), error.c_str()));

            return true;

        }

        atransport* t = find_transport(serial.c_str());

        if (t == nullptr) {

            return SendFail(reply_fd, android::base::StringPrintf("no such device '%s'",

                                                                  serial.c_str()));

            SendFail(reply_fd, android::base::StringPrintf("no such device '%s'", serial.c_str()));

            return true;

        }

        kick_transport(t);

        return SendOkay(reply_fd, android::base::StringPrintf("disconnected %s", address.c_str()));

        SendOkay(reply_fd, android::base::StringPrintf("disconnected %s", address.c_str()));

        return true;

    }



    // Returns our value for ADB_SERVER_VERSION.

    if (!strcmp(service, "version")) {

        return SendOkay(reply_fd, android::base::StringPrintf("%04x", ADB_SERVER_VERSION));

        SendOkay(reply_fd, android::base::StringPrintf("%04x", ADB_SERVER_VERSION));

        return true;

    }



    // These always report "unknown" rather than the actual error, for scripts.
@@ -1179,28 +1186,31 @@ int handle_host_request(const char* service, TransportType type, const char* ser 
        std::string error;

        atransport* t = acquire_one_transport(type, serial, transport_id, nullptr, &error);

        if (t) {

            return SendOkay(reply_fd, !t->serial.empty() ? t->serial : "unknown");

            SendOkay(reply_fd, !t->serial.empty() ? t->serial : "unknown");

        } else {

            return SendFail(reply_fd, error);

            SendFail(reply_fd, error);

        }

        return true;

    }

    if (!strcmp(service, "get-devpath")) {

        std::string error;

        atransport* t = acquire_one_transport(type, serial, transport_id, nullptr, &error);

        if (t) {

            return SendOkay(reply_fd, !t->devpath.empty() ? t->devpath : "unknown");

            SendOkay(reply_fd, !t->devpath.empty() ? t->devpath : "unknown");

        } else {

            return SendFail(reply_fd, error);

            SendFail(reply_fd, error);

        }

        return true;

    }

    if (!strcmp(service, "get-state")) {

        std::string error;

        atransport* t = acquire_one_transport(type, serial, transport_id, nullptr, &error);

        if (t) {

            return SendOkay(reply_fd, t->connection_state_name());

            SendOkay(reply_fd, t->connection_state_name());

        } else {

            return SendFail(reply_fd, error);

            SendFail(reply_fd, error);

        }

        return true;

    }



    // Indicates a new emulator instance has started.
@@ -1208,7 +1218,7 @@ int handle_host_request(const char* service, TransportType type, const char* ser 
        int  port = atoi(service+9);

        local_connect(port);

        /* we don't even need to send a reply */

        return 0;

        return true;

    }



    if (!strcmp(service, "reconnect")) {
@@ -1219,7 +1229,8 @@ int handle_host_request(const char* service, TransportType type, const char* ser 
            response =

                "reconnecting " + t->serial_name() + " [" + t->connection_state_name() + "]\n";

        }

        return SendOkay(reply_fd, response);

        SendOkay(reply_fd, response);

        return true;

    }



    if (handle_forward_request(service,
@@ -1228,10 +1239,10 @@ int handle_host_request(const char* service, TransportType type, const char* ser 
                                                                error);

                               },

                               reply_fd)) {

        return 0;

        return true;

    }



    return -1;

    return false;

}



static auto& init_mutex = *new std::mutex();

adb/adb.h

+2 −2
Original line number Diff line number Diff line
@@ -210,7 +210,7 @@ extern const char* adb_device_banner; 
#define USB_FFS_ADB_IN USB_FFS_ADB_EP(ep2)

#endif



int handle_host_request(const char* service, TransportType type, const char* serial,

bool handle_host_request(const char* service, TransportType type, const char* serial,

                         TransportId transport_id, int reply_fd, asocket* s);



void handle_online(atransport* t);

adb/sockets.cpp

+3 −7
Original line number Diff line number Diff line
@@ -685,13 +685,9 @@ static int smart_socket_enqueue(asocket* s, apacket::payload_type data) { 
    if (service) {

        asocket* s2;



        /* some requests are handled immediately -- in that

        ** case the handle_host_request() routine has sent

        ** the OKAY or FAIL message and all we have to do

        ** is clean up.

        */

        if (handle_host_request(service, type, serial, transport_id, s->peer->fd, s) == 0) {

            /* XXX fail message? */

        // Some requests are handled immediately -- in that case the handle_host_request() routine

        // has sent the OKAY or FAIL message and all we have to do is clean up.

        if (handle_host_request(service, type, serial, transport_id, s->peer->fd, s)) {

            D("SS(%d): handled host service '%s'", s->id, service);

            goto fail;

        }