From e9e8a16d359c900f114853eb0e407a8064e4350c Mon Sep 17 00:00:00 2001 Message-Id: From: Mark Wooding Date: Mon, 24 Mar 2008 21:32:56 +0000 Subject: [PATCH] Make the Disobedience login window a bit saner. There is now just a Login and a Cancel button. The former attempts a login synchronously (there not being any background activity to worry about blocking at this point); on success it (unconditionally) saves the config and pops down the login window, on error it gives you another go. Organization: Straylight/Edgeware From: Richard Kettlewell The problem described in defect 17 is eliminated in the login box by this change too since polling of the eclient is suppressed while the login box is up. However the underlying problem is still there and probably relates to multiple commands failing due to authentication errors. Those commands should probably be discarded silently. --- disobedience/Makefile.am | 2 +- disobedience/client.c | 3 +- disobedience/disobedience.h | 2 + disobedience/login.c | 91 +++++++++++++++---------------------- lib/client.c | 48 +++++++++++++++---- 5 files changed, 82 insertions(+), 64 deletions(-) diff --git a/disobedience/Makefile.am b/disobedience/Makefile.am index 6bba9f8..031d68d 100644 --- a/disobedience/Makefile.am +++ b/disobedience/Makefile.am @@ -30,7 +30,7 @@ disobedience_SOURCES=disobedience.h disobedience.c client.c queue.c \ log.c progress.c login.c rtp.c help.c \ ../lib/memgc.c settings.c disobedience_LDADD=../lib/libdisorder.a $(LIBPCRE) $(LIBGC) $(LIBGCRYPT) \ - $(LIBASOUND) $(COREAUDIO) + $(LIBASOUND) $(COREAUDIO) $(LIBDB) disobedience_LDFLAGS=$(GTK_LIBS) install-exec-hook: diff --git a/disobedience/client.c b/disobedience/client.c index 12dfcb2..35c1ce7 100644 --- a/disobedience/client.c +++ b/disobedience/client.c @@ -68,7 +68,8 @@ static gboolean gtkclient_dispatch(GSource *source, if(revents & (G_IO_OUT|G_IO_HUP|G_IO_ERR)) mode |= DISORDER_POLL_WRITE; time(&esource->last_poll); - disorder_eclient_polled(esource->client, mode); + if(!login_window) + disorder_eclient_polled(esource->client, mode); return TRUE; /* ??? not documented */ } diff --git a/disobedience/disobedience.h b/disobedience/disobedience.h index 0e16474..a9c801c 100644 --- a/disobedience/disobedience.h +++ b/disobedience/disobedience.h @@ -240,6 +240,8 @@ void choose_update(void); void login_box(void); +GtkWidget *login_window; + /* Help */ void popup_help(void); diff --git a/disobedience/login.c b/disobedience/login.c index 8dc8f0a..edefe29 100644 --- a/disobedience/login.c +++ b/disobedience/login.c @@ -19,11 +19,20 @@ */ /** @file disobedience/login.c * @brief Login box for Disobedience + * + * As of 2.1 we have only two buttons: Login and Cancel. + * + * If you hit Login then a login is attempted. If it works the window + * disappears and the settings are saved, otherwise they are NOT saved and the + * window remains. + * + * It you hit Cancel then the window disappears without saving anything. */ #include "disobedience.h" #include "split.h" #include "filepart.h" +#include "client.h" #include #include #include @@ -51,7 +60,7 @@ struct login_window_item { #define LWI_HIDDEN 0x0001 /** @brief Current login window */ -static GtkWidget *login_window; +GtkWidget *login_window; /** @brief Set connection defaults */ static void default_connect(void) { @@ -84,59 +93,18 @@ static const struct login_window_item lwis[] = { static GtkWidget *lwi_entry[NLWIS]; -static void update_config(void) { +static void login_update_config(void) { size_t n; for(n = 0; n < NLWIS; ++n) lwis[n].set(xstrdup(gtk_entry_get_text(GTK_ENTRY(lwi_entry[n])))); } -#if 0 -static int modified_config(void) { - size_t n; - - for(n = 0; n < NLWIS; ++n) { - const char *entered = gtk_entry_get_text(GTK_ENTRY(lwi_entry[n])); - const char *current = lwis[n].get(); - if(strcmp(entered, current)) - return 1; - } - return 0; -} -#endif - -static void login_ok(GtkButton attribute((unused)) *button, - gpointer attribute((unused)) userdata) { - update_config(); - reset(); -} - -static void login_save(GtkButton attribute((unused)) *button, - gpointer attribute((unused)) userdata) { +/** @brief Save current login details */ +static void login_save_config(void) { char *path = config_userconf(0, 0), *tmp; FILE *fp; - GtkWidget *yorn = 0; - update_config(); - /* See if the file already exists */ - if(access(path, F_OK) == 0) { - yorn = gtk_message_dialog_new - (GTK_WINDOW(login_window), - GTK_DIALOG_MODAL|GTK_DIALOG_DESTROY_WITH_PARENT, - GTK_MESSAGE_QUESTION, - GTK_BUTTONS_NONE, - "File %s already exists.", path); - gtk_window_set_title(GTK_WINDOW(yorn), - "Configuration file already exists"); - gtk_dialog_add_buttons(GTK_DIALOG(yorn), - "Overwrite it", GTK_RESPONSE_ACCEPT, - "Don't save after all", GTK_RESPONSE_REJECT, - (char *)0); - if(gtk_dialog_run(GTK_DIALOG(yorn)) != GTK_RESPONSE_ACCEPT) - goto done; - gtk_widget_destroy(yorn); - yorn = 0; - } byte_xasprintf(&tmp, "%s.tmp", path); /* Make sure the directory exists; don't care if it already exists. */ mkdir(d_dirname(tmp), 02700); @@ -169,13 +137,33 @@ static void login_save(GtkButton attribute((unused)) *button, tmp, strerror(errno)); goto done; } - fpopup_msg(GTK_MESSAGE_INFO, "Saved login configuration to %s", path); - gtk_widget_destroy(login_window); done: - if(yorn) - gtk_widget_destroy(yorn); + ; } +/** @brief User pressed OK in login window */ +static void login_ok(GtkButton attribute((unused)) *button, + gpointer attribute((unused)) userdata) { + disorder_client *c; + + /* Copy the new config into @ref config */ + login_update_config(); + /* Attempt a login with the new details */ + c = disorder_new(0); + if(!disorder_connect(c)) { + /* Success; save the config and start using it */ + login_save_config(); + reset(); + /* Pop down login window */ + gtk_widget_destroy(login_window); + } else { + /* Failed to connect - report the error */ + popup_msg(GTK_MESSAGE_ERROR, disorder_last(c)); + } + disorder_close(c); /* no use for this any more */ +} + +/** @brief User pressed cancel in the login window */ static void login_cancel(GtkButton attribute((unused)) *button, gpointer attribute((unused)) userdata) { gtk_widget_destroy(login_window); @@ -188,11 +176,6 @@ static const struct button buttons[] = { login_ok, "(Re-)connect using these settings", }, - { - GTK_STOCK_SAVE, - login_save, - "Save these settings and close window", - }, { GTK_STOCK_CLOSE, login_cancel, diff --git a/lib/client.c b/lib/client.c index c06bdf1..ccbdf6d 100644 --- a/lib/client.c +++ b/lib/client.c @@ -72,7 +72,7 @@ struct disorder_client { /** @brief Report errors to @c stderr */ int verbose; /** @brief Last error string */ - char *last; + const char *last; }; /** @brief Create a new client @@ -98,8 +98,10 @@ disorder_client *disorder_new(int verbose) { static int response(disorder_client *c, char **rp) { char *r; - if(inputline(c->ident, c->fpin, &r, '\n')) + if(inputline(c->ident, c->fpin, &r, '\n')) { + byte_xasprintf((char **)&c->last, "input error: %s", strerror(errno)); return -1; + } D(("response: %s", r)); if(rp) *rp = r; @@ -110,6 +112,7 @@ static int response(disorder_client *c, char **rp) { c->last = r + 4; return (r[0] * 10 + r[1]) * 10 + r[2] - 111 * '0'; } else { + c->last = "invalid reply format"; error(0, "invalid reply format from %s", c->ident); return -1; } @@ -174,6 +177,7 @@ static int disorder_simple_v(disorder_client *c, struct dynstr d; if(!c->fpout) { + c->last = "not connected"; error(0, "not connected to server"); return -1; } @@ -188,6 +192,7 @@ static int disorder_simple_v(disorder_client *c, dynstr_terminate(&d); D(("command: %s", d.vec)); if(fputs(d.vec, c->fpout) < 0 || fflush(c->fpout)) { + byte_xasprintf((char **)&c->last, "write error: %s", strerror(errno)); error(errno, "error writing to %s", c->ident); return -1; } @@ -273,23 +278,28 @@ static int disorder_connect_generic(disorder_client *c, return -1; c->fpin = c->fpout = 0; if((fd = socket(sa->sa_family, SOCK_STREAM, 0)) < 0) { + byte_xasprintf((char **)&c->last, "socket: %s", strerror(errno)); error(errno, "error calling socket"); return -1; } if(connect(fd, sa, salen) < 0) { + byte_xasprintf((char **)&c->last, "connect: %s", strerror(errno)); error(errno, "error calling connect"); goto error; } if((fd2 = dup(fd)) < 0) { + byte_xasprintf((char **)&c->last, "dup: %s", strerror(errno)); error(errno, "error calling dup"); goto error; } if(!(c->fpin = fdopen(fd, "rb"))) { + byte_xasprintf((char **)&c->last, "fdopen: %s", strerror(errno)); error(errno, "error calling fdopen"); goto error; } fd = -1; if(!(c->fpout = fdopen(fd2, "wb"))) { + byte_xasprintf((char **)&c->last, "fdopen: %s", strerror(errno)); error(errno, "error calling fdopen"); goto error; } @@ -299,11 +309,13 @@ static int disorder_connect_generic(disorder_client *c, if(!(rvec = split(r, &nrvec, SPLIT_QUOTES, 0, 0))) goto error; if(nrvec != 3) { + c->last = "cannot parse server greeting"; error(0, "cannot parse server greeting %s", r); goto error; } protocol = *rvec++; if(strcmp(protocol, "2")) { + c->last = "unknown protocol version"; error(0, "unknown protocol version: %s", protocol); goto error; } @@ -316,11 +328,15 @@ static int disorder_connect_generic(disorder_client *c, &c->user)) return 0; /* success */ if(!username) { + c->last = "cookie failed and no username"; error(0, "cookie did not work and no username available"); goto error; } } - if(!(res = authhash(nonce, nl, password, algorithm))) goto error; + if(!(res = authhash(nonce, nl, password, algorithm))) { + c->last = "error computing authorization hash"; + goto error; + } if((rc = disorder_simple(c, 0, "user", username, res, (char *)0))) goto error_rc; c->user = xstrdup(username); @@ -368,6 +384,7 @@ int disorder_connect(disorder_client *c) { const char *username, *password; if(!(username = config->username)) { + c->last = "no username"; error(0, "no username configured"); return -1; } @@ -382,6 +399,7 @@ int disorder_connect(disorder_client *c) { } if(!password) { /* Oh well */ + c->last = "no password"; error(0, "no password configured"); return -1; } @@ -420,6 +438,7 @@ int disorder_close(disorder_client *c) { if(c->fpin) { if(fclose(c->fpin) < 0) { + byte_xasprintf((char **)&c->last, "fclose: %s", strerror(errno)); error(errno, "error calling fclose"); ret = -1; } @@ -427,6 +446,7 @@ int disorder_close(disorder_client *c) { } if(c->fpout) { if(fclose(c->fpout) < 0) { + byte_xasprintf((char **)&c->last, "fclose: %s", strerror(errno)); error(errno, "error calling fclose"); ret = -1; } @@ -576,10 +596,13 @@ static int disorder_somequeue(disorder_client *c, qt = &q->next; } } - if(ferror(c->fpin)) + if(ferror(c->fpin)) { + byte_xasprintf((char **)&c->last, "input error: %s", strerror(errno)); error(errno, "error reading %s", c->ident); - else + } else { + c->last = "input error: unexpxected EOF"; error(0, "error reading %s: unexpected EOF", c->ident); + } return -1; } @@ -628,10 +651,13 @@ static int readlist(disorder_client *c, char ***vecp, int *nvecp) { } vector_append(&v, l + (*l == '.')); } - if(ferror(c->fpin)) + if(ferror(c->fpin)) { + byte_xasprintf((char **)&c->last, "input error: %s", strerror(errno)); error(errno, "error reading %s", c->ident); - else + } else { + c->last = "input error: unexpxected EOF"; error(0, "error reading %s: unexpected EOF", c->ident); + } return -1; } @@ -923,6 +949,7 @@ int disorder_get_volume(disorder_client *c, int *left, int *right) { if((rc = disorder_simple(c, &r, "volume", (char *)0))) return rc; if(sscanf(r, "%d %d", left, right) != 2) { + c->last = "malformed volume response"; error(0, "error parsing response to 'volume': '%s'", r); return -1; } @@ -942,7 +969,11 @@ int disorder_log(disorder_client *c, struct sink *s) { return rc; while(inputline(c->ident, c->fpin, &l, '\n') >= 0 && strcmp(l, ".")) if(sink_printf(s, "%s\n", l) < 0) return -1; - if(ferror(c->fpin) || feof(c->fpin)) return -1; + if(ferror(c->fpin) || feof(c->fpin)) { + byte_xasprintf((char **)&c->last, "input error: %s", + ferror(c->fpin) ? strerror(errno) : "unexpxected EOF"); + return -1; + } return 0; } @@ -1071,6 +1102,7 @@ int disorder_rtp_address(disorder_client *c, char **addressp, char **portp) { return rc; vec = split(r, &n, SPLIT_QUOTES, 0, 0); if(n != 2) { + c->last = "malformed RTP address"; error(0, "malformed rtp-address reply"); return -1; } -- [mdw]