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

Commit cc65c3b9 authored by Elliott Hughes's avatar Elliott Hughes
Browse files

Fix "adb sync" (and "adb push") error reporting. 

This patch ensures that we read any error response from the server if the
server closes the connection. Unfortunately, that's not sufficient to ensure
that we always see the server's error message --- sometimes the data just
gets thrown away because we keep writing without reading. Setting SO_LINGER
avoids this.

Bug: http://b/25230872
Change-Id: I96c019cc72bd139198de79bf29e6536cc462c20f
parent ddf41f1a

adb/adb_listeners.cpp

+8 −12
Original line number Diff line number Diff line
@@ -33,20 +33,16 @@ static alistener listener_list = { 
};



static void ss_listener_event_func(int _fd, unsigned ev, void *_l) {

    asocket *s;



    if (ev & FDE_READ) {

        struct sockaddr addr;

        socklen_t alen;

        int fd;



        alen = sizeof(addr);

        fd = adb_socket_accept(_fd, &addr, &alen);

        socklen_t alen = sizeof(addr);

        int fd = adb_socket_accept(_fd, &addr, &alen);

        if (fd < 0) return;



        adb_socket_setbufsize(fd, CHUNK_SIZE);

        int rcv_buf_size = CHUNK_SIZE;

        adb_setsockopt(fd, SOL_SOCKET, SO_RCVBUF, &rcv_buf_size, sizeof(rcv_buf_size));



        s = create_local_socket(fd);

        asocket* s = create_local_socket(fd);

        if (s) {

            connect_to_smartsocket(s);

            return;

adb/file_sync_client.cpp

+40 −26
Original line number Diff line number Diff line
@@ -103,7 +103,7 @@ class SyncConnection { 
    // Sending header, payload, and footer in a single write makes a huge

    // difference to "adb sync" performance.

    bool SendSmallFile(const char* path_and_mode,

                       const char* rpath,

                       const char* lpath, const char* rpath,

                       unsigned mtime,

                       const char* data, size_t data_length) {

        Print(rpath);
@@ -139,8 +139,7 @@ class SyncConnection { 
        req_done->path_length = mtime;

        p += sizeof(SyncRequest);



        if (!WriteFdExactly(fd, &buf[0], (p - &buf[0]))) return false;



        WriteOrDie(lpath, rpath, &buf[0], (p - &buf[0]));

        total_bytes += data_length;

        return true;

    }
@@ -164,31 +163,27 @@ class SyncConnection { 


        int lfd = adb_open(lpath, O_RDONLY);

        if (lfd < 0) {

            Error("cannot open '%s': %s", lpath, strerror(errno));

            Error("opening '%s' locally failed: %s", lpath, strerror(errno));

            return false;

        }



        syncsendbuf sbuf;

        sbuf.id = ID_DATA;

        while (true) {

            int ret = adb_read(lfd, sbuf.data, max);

            if (ret <= 0) {

                if (ret < 0) {

                    Error("cannot read '%s': %s", lpath, strerror(errno));

            int bytes_read = adb_read(lfd, sbuf.data, max);

            if (bytes_read == -1) {

                Error("reading '%s' locally failed: %s", lpath, strerror(errno));

                adb_close(lfd);

                return false;

                }

            } else if (bytes_read == 0) {

                break;

            }



            sbuf.size = ret;

            if (!WriteFdExactly(fd, &sbuf, sizeof(unsigned) * 2 + ret)) {

                adb_close(lfd);

                return false;

            }

            total_bytes += ret;

            sbuf.size = bytes_read;

            WriteOrDie(lpath, rpath, &sbuf, sizeof(SyncRequest) + bytes_read);



            bytes_copied += ret;

            total_bytes += bytes_read;

            bytes_copied += bytes_read;



            int percentage = static_cast<int>(bytes_copied * 100 / total_size);

            Printf("%s: %d%%", rpath, percentage);
@@ -199,12 +194,7 @@ class SyncConnection { 
        syncmsg msg;

        msg.data.id = ID_DONE;

        msg.data.size = mtime;

        if (!WriteFdExactly(fd, &msg.data, sizeof(msg.data))) {

            Error("failed to send ID_DONE message for '%s': %s", rpath, strerror(errno));

            return false;

        }



        return true;

        return WriteOrDie(lpath, rpath, &msg.data, sizeof(msg.data));

    }



    bool CopyDone(const char* from, const char* to) {
@@ -297,6 +287,27 @@ class SyncConnection { 
        return SendRequest(ID_QUIT, ""); // TODO: add a SendResponse?

    }



    bool WriteOrDie(const char* from, const char* to, const void* data, size_t data_length) {

        if (!WriteFdExactly(fd, data, data_length)) {

            if (errno == ECONNRESET) {

                // Assume adbd told us why it was closing the connection, and

                // try to read failure reason from adbd.

                syncmsg msg;

                if (!ReadFdExactly(fd, &msg.status, sizeof(msg.status))) {

                    Error("failed to copy '%s' to '%s': no response: %s", from, to, strerror(errno));

                } else if (msg.status.id != ID_FAIL) {

                    Error("failed to copy '%s' to '%s': not ID_FAIL: %d", from, to, msg.status.id);

                } else {

                    ReportCopyFailure(from, to, msg);

                }

            } else {

                Error("%zu-byte write failed: %s", data_length, strerror(errno));

            }

            _exit(1);

        }

        return true;

    }



    static uint64_t CurrentTimeMs() {

        struct timeval tv;

        gettimeofday(&tv, 0); // (Not clock_gettime because of Mac/Windows.)
@@ -362,7 +373,9 @@ static bool sync_send(SyncConnection& sc, const char* lpath, const char* rpath, 
        }

        buf[data_length++] = '\0';



        if (!sc.SendSmallFile(path_and_mode.c_str(), rpath, mtime, buf, data_length)) return false;

        if (!sc.SendSmallFile(path_and_mode.c_str(), lpath, rpath, mtime, buf, data_length)) {

            return false;

        }

        return sc.CopyDone(lpath, rpath);

#endif

    }
@@ -383,7 +396,8 @@ static bool sync_send(SyncConnection& sc, const char* lpath, const char* rpath, 
            sc.Error("failed to read all of '%s': %s", lpath, strerror(errno));

            return false;

        }

        if (!sc.SendSmallFile(path_and_mode.c_str(), rpath, mtime, data.data(), data.size())) {

        if (!sc.SendSmallFile(path_and_mode.c_str(), lpath, rpath, mtime,

                              data.data(), data.size())) {

            return false;

        }

    } else {

adb/file_sync_service.cpp

+12 −0
Original line number Diff line number Diff line
@@ -394,6 +394,18 @@ static bool handle_sync_command(int fd, std::vector<char>& buffer) { 
void file_sync_service(int fd, void* cookie) {

    std::vector<char> buffer(SYNC_DATA_MAX);



    // If there's a problem on the device, we'll send an ID_FAIL message and

    // close the socket. Unfortunately the kernel will sometimes throw that

    // data away if the other end keeps writing without reading (which is

    // the normal case with our protocol --- they won't read until the end).

    // So set SO_LINGER to give the client 20s to get around to reading our

    // failure response. Without this, the other side's ability to report

    // useful errors is reduced.

    struct linger l;

    l.l_onoff = 1;

    l.l_linger = 20;

    adb_setsockopt(fd, SOL_SOCKET, SO_LINGER, &l, sizeof(l));



    while (handle_sync_command(fd, buffer)) {

    }

adb/sysdeps.h

+5 −24
Original line number Diff line number Diff line
@@ -242,18 +242,6 @@ extern int adb_setsockopt(int fd, int level, int optname, const void* optva 
#undef   setsockopt

#define  setsockopt  ___xxx_setsockopt



static __inline__  int  adb_socket_setbufsize( int   fd, int  bufsize )

{

    int opt = bufsize;

    return adb_setsockopt(fd, SOL_SOCKET, SO_RCVBUF, (const void*)&opt, sizeof(opt));

}



static __inline__ void  disable_tcp_nagle( int  fd )

{

    int  on = 1;

    adb_setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, (const void*)&on, sizeof(on));

}



extern int  adb_socketpair( int  sv[2] );



static __inline__ int adb_is_absolute_host_path(const char* path) {
@@ -670,18 +658,6 @@ static __inline__ int adb_thread_setname(const std::string& name) { 
#endif

}



static __inline__  int  adb_socket_setbufsize(int fd, int  bufsize )

{

    int opt = bufsize;

    return setsockopt(fd, SOL_SOCKET, SO_RCVBUF, &opt, sizeof(opt));

}



static __inline__ void  disable_tcp_nagle(int fd)

{

    int  on = 1;

    setsockopt( fd, IPPROTO_TCP, TCP_NODELAY, (void*)&on, sizeof(on) );

}



static __inline__ int  adb_setsockopt( int  fd, int  level, int  optname, const void*  optval, socklen_t  optlen )

{

    return setsockopt( fd, level, optname, optval, optlen );
@@ -739,4 +715,9 @@ static __inline__ unsigned long adb_thread_id() 


#endif /* !_WIN32 */



static inline void disable_tcp_nagle(int fd) {

    int off = 1;

    adb_setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, &off, sizeof(off));

}



#endif /* _ADB_SYSDEPS_H */