From 8d8126a705dd3c5734a0894f88c2c758784bd469 Mon Sep 17 00:00:00 2001 From: Spencer Low Date: Tue, 21 Jul 2015 02:06:26 -0700 Subject: [PATCH] adb: logging: newlines, thread ids, error code overwriting Add missing \n to uses of legacy D() macro. This should make the legacy logging easier to read (and harder to miss important stuff). On POSIX, use gettid() from libcutils instead of pthread_self() so that the output shows a more reasonable number instead of a pointer value. This should be ok since libbase's logging already uses gettid(). Win32: Don't let the Win32 last error get overwritten by API calls after the original error'ing API. When encountering an unknown error, log the specific error code. Change-Id: Ib8f72754efa7ba895d2f1cd914251fec2a1d894c Signed-off-by: Spencer Low --- adb/adb.cpp | 2 +- adb/adb_auth_client.cpp | 2 +- adb/adb_auth_host.cpp | 12 ++++++------ adb/adb_client.cpp | 2 +- adb/fdevent.cpp | 2 +- adb/jdwp_service.cpp | 2 +- adb/services.cpp | 4 ++-- adb/sysdeps.h | 3 ++- adb/sysdeps_win32.cpp | 23 +++++++++++++---------- adb/transport.cpp | 6 +++--- adb/usb_linux_client.cpp | 4 ++-- 11 files changed, 33 insertions(+), 29 deletions(-) diff --git a/adb/adb.cpp b/adb/adb.cpp index 2e59634fd..a7a7e0698 100644 --- a/adb/adb.cpp +++ b/adb/adb.cpp @@ -174,7 +174,7 @@ static void setup_trace_mask() { for (const auto& elem : elements) { const auto& flag = trace_flags.find(elem); if (flag == trace_flags.end()) { - D("Unknown trace flag: %s", flag->first.c_str()); + D("Unknown trace flag: %s\n", flag->first.c_str()); continue; } diff --git a/adb/adb_auth_client.cpp b/adb/adb_auth_client.cpp index 8e7d38be1..884d5be20 100644 --- a/adb/adb_auth_client.cpp +++ b/adb/adb_auth_client.cpp @@ -212,7 +212,7 @@ void adb_auth_confirm_key(unsigned char *key, size_t len, atransport *t) ret = snprintf(msg, sizeof(msg), "PK%s", key); if (ret >= (signed)sizeof(msg)) { - D("Key too long. ret=%d", ret); + D("Key too long. ret=%d\n", ret); return; } D("Sending '%s'\n", msg); diff --git a/adb/adb_auth_host.cpp b/adb/adb_auth_host.cpp index e878f8b04..8085c1af8 100644 --- a/adb/adb_auth_host.cpp +++ b/adb/adb_auth_host.cpp @@ -183,7 +183,7 @@ static int write_public_keyfile(RSA *private_key, const char *private_key_path) #if defined(OPENSSL_IS_BORINGSSL) if (!EVP_EncodedLength(&encoded_length, sizeof(pkey))) { - D("Public key too large to base64 encode"); + D("Public key too large to base64 encode\n"); goto out; } #else @@ -194,7 +194,7 @@ static int write_public_keyfile(RSA *private_key, const char *private_key_path) encoded = new uint8_t[encoded_length]; if (encoded == nullptr) { - D("Allocation failure"); + D("Allocation failure\n"); goto out; } @@ -203,7 +203,7 @@ static int write_public_keyfile(RSA *private_key, const char *private_key_path) if (fwrite(encoded, encoded_length, 1, outfile) != 1 || fwrite(info, strlen(info), 1, outfile) != 1) { - D("Write error while writing public key"); + D("Write error while writing public key\n"); goto out; } @@ -323,7 +323,7 @@ static int get_user_keyfilepath(char *filename, size_t len) if (stat(android_dir, &buf)) { if (adb_mkdir(android_dir, 0750) < 0) { - D("Cannot mkdir '%s'", android_dir); + D("Cannot mkdir '%s'\n", android_dir); return -1; } } @@ -339,7 +339,7 @@ static int get_user_key(struct listnode *list) ret = get_user_keyfilepath(path, sizeof(path)); if (ret < 0 || ret >= (signed)sizeof(path)) { - D("Error getting user key filename"); + D("Error getting user key filename\n"); return 0; } @@ -414,7 +414,7 @@ int adb_auth_get_userkey(unsigned char *data, size_t len) char path[PATH_MAX]; int ret = get_user_keyfilepath(path, sizeof(path) - 4); if (ret < 0 || ret >= (signed)(sizeof(path) - 4)) { - D("Error getting user key filename"); + D("Error getting user key filename\n"); return 0; } strcat(path, ".pub"); diff --git a/adb/adb_client.cpp b/adb/adb_client.cpp index ef9a58697..e42d92879 100644 --- a/adb/adb_client.cpp +++ b/adb/adb_client.cpp @@ -243,7 +243,7 @@ int adb_connect(const std::string& service, std::string* error) { fd = _adb_connect(service, error); if (fd == -1) { - D("_adb_connect error: %s", error->c_str()); + D("_adb_connect error: %s\n", error->c_str()); } else if(fd == -2) { fprintf(stderr,"** daemon still not running\n"); } diff --git a/adb/fdevent.cpp b/adb/fdevent.cpp index 54976131b..5cd498831 100644 --- a/adb/fdevent.cpp +++ b/adb/fdevent.cpp @@ -578,7 +578,7 @@ void fdevent_subproc_setup() if(adb_socketpair(s)) { FATAL("cannot create shell-exit socket-pair\n"); } - D("socketpair: (%d,%d)", s[0], s[1]); + D("socketpair: (%d,%d)\n", s[0], s[1]); SHELL_EXIT_NOTIFY_FD = s[0]; fdevent *fde; diff --git a/adb/jdwp_service.cpp b/adb/jdwp_service.cpp index c0f7ec273..23af6c970 100644 --- a/adb/jdwp_service.cpp +++ b/adb/jdwp_service.cpp @@ -435,7 +435,7 @@ FoundIt: __FUNCTION__, strerror(errno)); return -1; } - D("socketpair: (%d,%d)", fds[0], fds[1]); + D("socketpair: (%d,%d)\n", fds[0], fds[1]); proc->out_fds[ proc->out_count ] = fds[1]; if (++proc->out_count == 1) diff --git a/adb/services.cpp b/adb/services.cpp index 227f22acc..e3c17f9da 100644 --- a/adb/services.cpp +++ b/adb/services.cpp @@ -205,7 +205,7 @@ static int create_service_thread(void (*func)(int, void *), void *cookie) printf("cannot create service socket pair\n"); return -1; } - D("socketpair: (%d,%d)", s[0], s[1]); + D("socketpair: (%d,%d)\n", s[0], s[1]); stinfo* sti = reinterpret_cast(malloc(sizeof(stinfo))); if (sti == nullptr) { @@ -317,7 +317,7 @@ static int create_subproc_raw(const char *cmd, const char *arg0, const char *arg printf("[ cannot create socket pair - %s ]\n", strerror(errno)); return -1; } - D("socketpair: (%d,%d)", sv[0], sv[1]); + D("socketpair: (%d,%d)\n", sv[0], sv[1]); *pid = fork(); if (*pid < 0) { diff --git a/adb/sysdeps.h b/adb/sysdeps.h index b7c16d983..5efdab5ba 100644 --- a/adb/sysdeps.h +++ b/adb/sysdeps.h @@ -276,6 +276,7 @@ static __inline__ int adb_is_absolute_host_path( const char* path ) #include "fdevent.h" #include #include +#include #include #include #include @@ -544,7 +545,7 @@ static __inline__ int adb_is_absolute_host_path( const char* path ) static __inline__ unsigned long adb_thread_id() { - return (unsigned long)pthread_self(); + return (unsigned long)gettid(); } #endif /* !_WIN32 */ diff --git a/adb/sysdeps_win32.cpp b/adb/sysdeps_win32.cpp index 50c99f1ad..bdc602775 100644 --- a/adb/sysdeps_win32.cpp +++ b/adb/sysdeps_win32.cpp @@ -361,9 +361,10 @@ int adb_open(const char* path, int options) 0, NULL ); if ( f->fh_handle == INVALID_HANDLE_VALUE ) { + const DWORD err = GetLastError(); _fh_close(f); - D( "adb_open: could not open '%s':", path ); - switch (GetLastError()) { + D( "adb_open: could not open '%s': ", path ); + switch (err) { case ERROR_FILE_NOT_FOUND: D( "file not found\n" ); errno = ENOENT; @@ -375,7 +376,7 @@ int adb_open(const char* path, int options) return -1; default: - D( "unknown error\n" ); + D( "unknown error: %ld\n", err ); errno = ENOENT; return -1; } @@ -402,9 +403,10 @@ int adb_creat(const char* path, int mode) NULL ); if ( f->fh_handle == INVALID_HANDLE_VALUE ) { + const DWORD err = GetLastError(); _fh_close(f); - D( "adb_creat: could not open '%s':", path ); - switch (GetLastError()) { + D( "adb_creat: could not open '%s': ", path ); + switch (err) { case ERROR_FILE_NOT_FOUND: D( "file not found\n" ); errno = ENOENT; @@ -416,7 +418,7 @@ int adb_creat(const char* path, int mode) return -1; default: - D( "unknown error\n" ); + D( "unknown error: %ld\n", err ); errno = ENOENT; return -1; } @@ -781,8 +783,9 @@ int adb_socket_accept(int serverfd, struct sockaddr* addr, socklen_t *addrle fh->fh_socket = accept( serverfh->fh_socket, addr, addrlen ); if (fh->fh_socket == INVALID_SOCKET) { + const DWORD err = WSAGetLastError(); _fh_close( fh ); - D( "adb_socket_accept: accept on fd %d return error %ld\n", serverfd, GetLastError() ); + D( "adb_socket_accept: accept on fd %d return error %ld\n", serverfd, err ); return -1; } @@ -1546,7 +1549,7 @@ _wait_for_all(HANDLE* handles, int handles_count) threads = (WaitForAllParam*)malloc((chunks + (remains ? 1 : 0)) * sizeof(WaitForAllParam)); if (threads == NULL) { - D("Unable to allocate thread array for %d handles.", handles_count); + D("Unable to allocate thread array for %d handles.\n", handles_count); return (int)WAIT_FAILED; } @@ -1554,7 +1557,7 @@ _wait_for_all(HANDLE* handles, int handles_count) * reset" event that will remain set once it was set. */ main_event = CreateEvent(NULL, TRUE, FALSE, NULL); if (main_event == NULL) { - D("Unable to create main event. Error: %d", (int)GetLastError()); + D("Unable to create main event. Error: %ld\n", GetLastError()); free(threads); return (int)WAIT_FAILED; } @@ -1587,7 +1590,7 @@ _wait_for_all(HANDLE* handles, int handles_count) &threads[chunk], 0, NULL); if (threads[chunk].thread == NULL) { /* Unable to create a waiter thread. Collapse. */ - D("Unable to create a waiting thread %d of %d. errno=%d", + D("Unable to create a waiting thread %d of %d. errno=%d\n", chunk, chunks, errno); chunks = chunk; SetEvent(main_event); diff --git a/adb/transport.cpp b/adb/transport.cpp index 1bb130f8e..87ca5accf 100644 --- a/adb/transport.cpp +++ b/adb/transport.cpp @@ -283,7 +283,7 @@ static void *output_thread(void *_t) p->msg.magic = A_SYNC ^ 0xffffffff; if(write_packet(t->fd, t->serial, &p)) { put_apacket(p); - D("%s: failed to write SYNC apacket to transport", t->serial); + D("%s: failed to write SYNC apacket to transport\n", t->serial); } oops: @@ -579,7 +579,7 @@ static void transport_registration_func(int _fd, unsigned ev, void *data) fatal_errno("cannot open transport socketpair"); } - D("transport: %s socketpair: (%d,%d) starting", t->serial, s[0], s[1]); + D("transport: %s socketpair: (%d,%d) starting\n", t->serial, s[0], s[1]); t->transport_socket = s[0]; t->fd = s[1]; @@ -617,7 +617,7 @@ void init_transport_registration(void) if(adb_socketpair(s)){ fatal_errno("cannot open transport registration socketpair"); } - D("socketpair: (%d,%d)", s[0], s[1]); + D("socketpair: (%d,%d)\n", s[0], s[1]); transport_registration_send = s[0]; transport_registration_recv = s[1]; diff --git a/adb/usb_linux_client.cpp b/adb/usb_linux_client.cpp index d34c45407..cfd95ddce 100644 --- a/adb/usb_linux_client.cpp +++ b/adb/usb_linux_client.cpp @@ -444,11 +444,11 @@ static void usb_ffs_kick(usb_handle *h) err = ioctl(h->bulk_in, FUNCTIONFS_CLEAR_HALT); if (err < 0) - D("[ kick: source (fd=%d) clear halt failed (%d) ]", h->bulk_in, errno); + D("[ kick: source (fd=%d) clear halt failed (%d) ]\n", h->bulk_in, errno); err = ioctl(h->bulk_out, FUNCTIONFS_CLEAR_HALT); if (err < 0) - D("[ kick: sink (fd=%d) clear halt failed (%d) ]", h->bulk_out, errno); + D("[ kick: sink (fd=%d) clear halt failed (%d) ]\n", h->bulk_out, errno); adb_mutex_lock(&h->lock);