Skip to content
This repository has been archived by the owner on Sep 2, 2020. It is now read-only.

Commit

Permalink
Always get a PTY even with newer adb
Browse files Browse the repository at this point in the history
New versions of ADB have improved the shell command so that it's
clean, propagates error codes, and allocates PTYs unconditionally,
much like ssh.  This last change breaks fb-adb, since we run the
inferior adb with pipes for stdio; ADB thinks it's being run
non-interactively, skips allocating a PTY on the device side.
The remote shell doesn't have a pty itself, so _it_ thinks it's being
run non-interactively, so it doesn't print a prompt.  Meanwhile,
fb-adb hangs because it's waiting forever for a prompt.

This change just gets us back to using a PTY unconditionally.
We could take advantage of adb fixes to get rid of some of our
encoding logic, but that's a separate project.
  • Loading branch information
dcolascione committed Oct 17, 2016
1 parent 8f532a9 commit a256027
Show file tree
Hide file tree
Showing 4 changed files with 162 additions and 15 deletions.
14 changes: 13 additions & 1 deletion chat.c
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,22 @@ chat_expect_maybe(struct chat* cc, char expected)
static void
chat_swallow_prompt(struct chat* cc)
{
SCOPED_RESLIST(rl);

/* 100% reliable prompt detection */
unsigned csi_arg = 0;
enum { S_NORMAL, S_AFTER_ESC, S_AFTER_CSI } state = S_NORMAL;
struct growable_string pre_prompt = {};

for (;;) {
char c = chat_getc(cc);
int char_read = getc(cc->from);
if (char_read == EOF) {
growable_string_trim_trailing_whitespace(&pre_prompt);
if (pre_prompt.strlen == 0)
chat_die();
die(ECOMM, "%s", growable_string_c_str(&pre_prompt));
}
char c = char_read;
if (c == '#' || c == '$')
break;

Expand All @@ -93,6 +103,8 @@ chat_swallow_prompt(struct chat* cc)
case S_NORMAL:
if (c == '\033')
state = S_AFTER_ESC;
else
growable_string_append_c(&pre_prompt, c);
break;

case S_AFTER_ESC:
Expand Down
122 changes: 108 additions & 14 deletions cmd_shex.c
Original file line number Diff line number Diff line change
Expand Up @@ -232,10 +232,10 @@ send_stub(const void* data,


static struct child*
try_adb_stub(const struct child_start_info* csi,
const char* adb_name,
struct child_hello* chello,
char** err)
try_adb_stub_2(const struct child_start_info* csi,
const char* adb_name,
struct child_hello* chello,
char** err)
{
SCOPED_RESLIST(rl);
struct child* child = child_start(csi);
Expand Down Expand Up @@ -301,6 +301,104 @@ try_adb_stub(const struct child_start_info* csi,
return NULL;
}

struct try_adb_stub_internal_info {
struct child* ret;
const struct child_start_info* csi;
const char* adb_name;
struct child_hello* chello;
char** err;
};

static void
try_adb_stub_1(void* arg)
{
struct try_adb_stub_internal_info* info = arg;
info->ret = try_adb_stub_2(
info->csi, info->adb_name, info->chello, info->err);
}

struct try_adb_stub_memory {
bool old_adb_detected;
};

static struct child*
try_adb_stub(struct try_adb_stub_memory* tasm,
const char* const* adb_args,
const char* adb_name,
struct child_hello* chello,
char** err)
{
const struct child_start_info csi = {
.io[STDIN_FILENO] = CHILD_IO_PIPE,
.io[STDOUT_FILENO] = CHILD_IO_PIPE,
.io[STDERR_FILENO] = CHILD_IO_RECORD,
.exename = "adb",
.argv = ARGV_CONCAT(ARGV("adb"),
adb_args,
tasm->old_adb_detected
? ARGV("shell")
: ARGV("shell", "-t", "-t")),
};

struct try_adb_stub_internal_info info = {
.csi = &csi,
.adb_name = adb_name,
.chello = chello,
.err = err,
};

struct errinfo ei = { .want_msg = true };
if (catch_error(try_adb_stub_1, &info, &ei)) {
if (ei.err == ECOMM &&
ei.msg &&
(string_ends_with_p(
ei.msg, "-t: unknown option" /* E1 */) ||
string_ends_with_p(
ei.msg,
"target doesn't support PTY args -Tt" /* E2 */ )))
{
/*
Here's what happens under various combinations of tool and
device versions when stdin is a pipe, not a tty.
Why doesn't "-t -t" just succeed when running a new ADB
against an older device, where we'll get a tty anyway?
Because the universe is malevolent. See the following
outcome table. E1 and E2 refer to the messages
just above.
Pre-N device | Post-N device
+ --------------+----------------
Pre-N ADB | E1 | E1
-t -t | |
-----------+---------------+----------------
Pre-N ADB | Get a TTY | Get a TTY
no options | |
-----------+---------------+----------------
Post-N ADB | E2 | Get a TTY
-t -t | |
-----------+---------------+----------------
Post-N ADB | Get a TTY | Get a pipe (and hang)
no options |
If we react to _either_ E1 or E2 by re-invoking without -t
-t, we get a TTY. The downside of this approach is that
we need a second "adb shell" invocation when we have a new
adb connecting to an old device, but hopefully, users will
just rely on the daemon and avoid having to talk to "adb
shell" frequently.
*/

tasm->old_adb_detected = true;
return try_adb_stub(tasm, adb_args, adb_name, chello, err);
}
die_rethrow(&ei);
}
return info.ret;
}

struct delete_device_tmpfile {
const char** adb_args;
char* device_filename;
Expand Down Expand Up @@ -351,17 +449,12 @@ static struct child*
start_stub_adb(const char* const* adb_args,
struct child_hello* chello)
{
const struct child_start_info csi = {
.io[STDIN_FILENO] = CHILD_IO_PIPE,
.io[STDOUT_FILENO] = CHILD_IO_PIPE,
.io[STDERR_FILENO] = CHILD_IO_RECORD,
.exename = "adb",
.argv = ARGV_CONCAT(ARGV("adb"), adb_args, ARGV("shell")),
};
struct try_adb_stub_memory tasm = { };

struct child* child = NULL;
char* err = NULL;
child = try_adb_stub(&csi, FB_ADB_REMOTE_FILENAME, chello, &err);
child = try_adb_stub(
&tasm, adb_args, FB_ADB_REMOTE_FILENAME, chello, &err);

if (child == NULL) {
char* tmp_adb = xaprintf(
Expand All @@ -378,7 +471,7 @@ start_stub_adb(const char* const* adb_args,
#endif
for (unsigned i = first_stub; i < ns && !child; ++i) {
send_stub(stubs[i].data, stubs[i].size, adb_args, tmp_adb);
child = try_adb_stub(&csi, tmp_adb, chello, &err);
child = try_adb_stub(&tasm, adb_args, tmp_adb, chello, &err);
}

if (!child)
Expand All @@ -393,7 +486,8 @@ start_stub_adb(const char* const* adb_args,
api_level,
ADB_RENAME_FALL_BACK_TO_CAT,
adb_args);
child = try_adb_stub(&csi, FB_ADB_REMOTE_FILENAME, chello, &err);
child = try_adb_stub(&tasm, adb_args,
FB_ADB_REMOTE_FILENAME, chello, &err);
if (!child)
die(ECOMM, "trouble starting adb stub: %s", err);
}
Expand Down
29 changes: 29 additions & 0 deletions util.c
Original file line number Diff line number Diff line change
Expand Up @@ -1681,6 +1681,35 @@ grow_buffer_dwim(struct growable_buffer* gb)
resize_buffer(gb, bufsz);
}

void
growable_string_append_c(struct growable_string* gs, char c)
{
assert(gs->strlen <= gs->gb.bufsz);
if (gs->strlen == gs->gb.bufsz)
grow_buffer_dwim(&gs->gb);
assert(gs->strlen < gs->gb.bufsz);
gs->gb.buf[gs->strlen++] = c;
}

void
growable_string_trim_trailing_whitespace(struct growable_string* gs)
{
while (gs->strlen > 0 && strchr(" \t\r\n\v", gs->gb.buf[gs->strlen - 1]))
gs->strlen--;
}

const char*
growable_string_c_str(struct growable_string* gs)
{
assert(gs->strlen <= gs->gb.bufsz);
if (gs->strlen == gs->gb.bufsz) {
grow_buffer_dwim(&gs->gb);
}
assert(gs->strlen < gs->gb.bufsz);
gs->gb.buf[gs->strlen] = '\0';
return (const char*) &gs->gb.buf[0];
}

static void
cleanup_regfree(void* data)
{
Expand Down
12 changes: 12 additions & 0 deletions util.h
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,18 @@ void resize_buffer(struct growable_buffer* gb, size_t new_size);
void grow_buffer(struct growable_buffer* gb, size_t min);
void grow_buffer_dwim(struct growable_buffer* gb);

// Like growable_buffer, but deals in characters and automatically
// NUL-terminates

struct growable_string {
struct growable_buffer gb;
size_t strlen;
};

void growable_string_append_c(struct growable_string* gs, char c);
const char* growable_string_c_str(struct growable_string* gs);
void growable_string_trim_trailing_whitespace(struct growable_string* gs);

regex_t* xregcomp(const char* regex, int cflags);
char* xregerror(int errcode, const regex_t* preg);

Expand Down

0 comments on commit a256027

Please sign in to comment.