From 06bfbba4a281a9e615869b857cc0b775225accb3 Mon Sep 17 00:00:00 2001 Message-Id: <06bfbba4a281a9e615869b857cc0b775225accb3.1714837262.git.mdw@distorted.org.uk> From: Mark Wooding Date: Sat, 7 Jun 2008 22:05:15 +0100 Subject: [PATCH 1/1] eclient string response calls now get errors reported to the completed callback rather than the protocol_error callback. Organization: Straylight/Edgeware From: Richard Kettlewell --- clients/Makefile.am | 8 +- clients/test-eclient.c | 184 -------------------------------------- disobedience/choose.c | 44 ++++----- disobedience/menu.c | 18 +++- disobedience/properties.c | 13 +-- disobedience/queue.c | 20 ++--- disobedience/users.c | 20 ++++- lib/eclient.c | 25 ++++-- lib/eclient.h | 14 ++- 9 files changed, 99 insertions(+), 247 deletions(-) delete mode 100644 clients/test-eclient.c diff --git a/clients/Makefile.am b/clients/Makefile.am index 58316b4..0ff3477 100644 --- a/clients/Makefile.am +++ b/clients/Makefile.am @@ -19,7 +19,7 @@ # bin_PROGRAMS=disorder disorderfm disorder-playrtp -noinst_PROGRAMS=test-eclient filename-bytes +noinst_PROGRAMS=filename-bytes noinst_SCRIPTS=dump2wav AM_CPPFLAGS=-I${top_srcdir}/lib -I../lib @@ -44,12 +44,6 @@ disorder_playrtp_DEPENDENCIES=$(LIBOBJS) ../lib/libdisorder.a filename_bytes_SOURCES=filename-bytes.c -test_eclient_SOURCES=test-eclient.c \ - ../lib/memgc.c -test_eclient_LDADD=../lib/libdisorder.a \ - $(LIBGC) $(LIBGCRYPT) $(LIBPCRE) -test_eclient_DEPENDENCIES=../lib/libdisorder.a - install-exec-hook: $(LIBTOOL) --mode=finish $(DESTDIR)$(libdir) diff --git a/clients/test-eclient.c b/clients/test-eclient.c deleted file mode 100644 index b5742a3..0000000 --- a/clients/test-eclient.c +++ /dev/null @@ -1,184 +0,0 @@ -/* - * This file is part of DisOrder. - * Copyright (C) 2006 Richard Kettlewell - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 - * USA - */ - -#include "common.h" - -#include -#include -#include - -#include "queue.h" -#include "mem.h" -#include "log.h" -#include "eclient.h" -#include "configuration.h" -#include "syscalls.h" -#include "wstat.h" -#include "charset.h" - -/* TODO: a more comprehensive test */ - -static fd_set rfd, wfd; -static int maxfd; -static disorder_eclient *clients[1024]; -static char **tracks; -static disorder_eclient *c; -static char u_value; -static int quit; - -static const char *modes[] = { "none", "read", "write", "read write" }; - -static void cb_comms_error(void *u, const char *msg) { - assert(u == &u_value); - fprintf(stderr, "! comms error: %s\n", msg); -} - -static void cb_protocol_error(void *u, - void attribute((unused)) *v, - int attribute((unused)) code, - const char *msg) { - assert(u == &u_value); - fprintf(stderr, "! protocol error: %s\n", msg); -} - -static void cb_poll(void *u, disorder_eclient *c_, int fd, unsigned mode) { - assert(u == &u_value); - assert(fd >= 0); - assert(fd < 1024); /* bodge */ - fprintf(stderr, " poll callback %d %s\n", fd, modes[mode]); - if(mode & DISORDER_POLL_READ) - FD_SET(fd, &rfd); - else - FD_CLR(fd, &rfd); - if(mode & DISORDER_POLL_WRITE) - FD_SET(fd, &wfd); - else - FD_CLR(fd, &wfd); - clients[fd] = mode ? c_ : 0; - if(fd > maxfd) maxfd = fd; -} - -static void cb_report(void attribute((unused)) *u, - const char attribute((unused)) *msg) { -} - -static const disorder_eclient_callbacks callbacks = { - cb_comms_error, - cb_protocol_error, - cb_poll, - cb_report -}; - -/* cheap plastic event loop */ -static void loop(void) { - int n; - - while(!quit) { - fd_set r = rfd, w = wfd; - n = select(maxfd + 1, &r, &w, 0, 0); - if(n < 0) { - if(errno == EINTR) continue; - fatal(errno, "select"); - } - for(n = 0; n <= maxfd; ++n) - if(clients[n] && (FD_ISSET(n, &r) || FD_ISSET(n, &w))) - disorder_eclient_polled(clients[n], - ((FD_ISSET(n, &r) ? DISORDER_POLL_READ : 0) - |(FD_ISSET(n, &w) ? DISORDER_POLL_WRITE : 0))); - } - printf(". quit\n"); -} - -static void done(void) { - printf(". done\n"); - disorder_eclient_close(c); - quit = 1; -} - -static void play_completed(void *v) { - assert(v == tracks); - printf("* played: %s\n", *tracks); - ++tracks; - if(*tracks) { - if(disorder_eclient_play(c, *tracks, play_completed, tracks)) - exit(1); - } else - done(); -} - -static void version_completed(void *v, const char *value) { - printf("* version: %s\n", value); - if(v) { - if(*tracks) { - if(disorder_eclient_play(c, *tracks, play_completed, tracks)) - exit(1); - } else - done(); - } -} - -/* TODO: de-dupe with disorder.c */ -static void print_queue_entry(const struct queue_entry *q) { - if(q->track) xprintf("track %s\n", nullcheck(utf82mb(q->track))); - if(q->id) xprintf(" id %s\n", nullcheck(utf82mb(q->id))); - if(q->submitter) xprintf(" submitted by %s at %s", - nullcheck(utf82mb(q->submitter)), ctime(&q->when)); - if(q->played) xprintf(" played at %s", ctime(&q->played)); - if(q->state == playing_started - || q->state == playing_paused) xprintf(" %lds so far", q->sofar); - else if(q->expected) xprintf(" might start at %s", ctime(&q->expected)); - if(q->scratched) xprintf(" scratched by %s\n", - nullcheck(utf82mb(q->scratched))); - else xprintf(" %s\n", playing_states[q->state]); - if(q->wstat) xprintf(" %s\n", wstat(q->wstat)); -} - -static void recent_completed(void *v, struct queue_entry *q) { - assert(v == 0); - for(; q; q = q->next) - print_queue_entry(q); - if(disorder_eclient_version(c, version_completed, (void *)"")) exit(1); -} - -int main(int argc, char **argv) { - assert(argc > 0); - mem_init(); - debugging = 0; /* turn on for even more verbosity */ - if(config_read(0)) fatal(0, "config_read failed"); - tracks = &argv[1]; - c = disorder_eclient_new(&callbacks, &u_value); - assert(c != 0); - /* stack up several version commands to test pipelining */ - if(disorder_eclient_version(c, version_completed, 0)) exit(1); - if(disorder_eclient_version(c, version_completed, 0)) exit(1); - if(disorder_eclient_version(c, version_completed, 0)) exit(1); - if(disorder_eclient_version(c, version_completed, 0)) exit(1); - if(disorder_eclient_version(c, version_completed, 0)) exit(1); - if(disorder_eclient_recent(c, recent_completed, 0)) exit(1); - loop(); - exit(0); -} - -/* -Local Variables: -c-basic-offset:2 -comment-column:40 -End: -*/ diff --git a/disobedience/choose.c b/disobedience/choose.c index 6658a45..2348267 100644 --- a/disobedience/choose.c +++ b/disobedience/choose.c @@ -215,7 +215,7 @@ static struct choosenode *newnode(struct choosenode *parent, static void fill_root_node(struct choosenode *cn); static void fill_directory_node(struct choosenode *cn); static void got_files(void *v, int nvec, char **vec); -static void got_resolved_file(void *v, const char *track); +static void got_resolved_file(void *v, const char *error, const char *track); static void got_dirs(void *v, int nvec, char **vec); static void expand_node(struct choosenode *cn, int contingent); @@ -429,14 +429,13 @@ static void got_files(void *v, int nvec, char **vec) { int n; D(("got_files %d files for %s %s", nvec, cn->path, cnflags(cn))); - /* Complicated by the need to resolve aliases. We can save a bit of effort - * by re-using cbd though. */ + /* Complicated by the need to resolve aliases. */ cn->flags &= ~CN_GETTING_FILES; --gets_in_flight; if((cn->pending = nvec)) { cn->flags |= CN_RESOLVING_FILES; for(n = 0; n < nvec; ++n) { - disorder_eclient_resolve(client, got_resolved_file, vec[n], cbd); + disorder_eclient_resolve(client, got_resolved_file, vec[n], cn); ++gets_in_flight; } } @@ -449,23 +448,26 @@ static void got_files(void *v, int nvec, char **vec) { } /** @brief Called with an alias resolved filename */ -static void got_resolved_file(void *v, const char *track) { - struct callbackdata *cbd = v; - struct choosenode *cn = cbd->u.choosenode, *file_cn; - - D(("resolved %s %s %d left", cn->path, cnflags(cn), cn->pending - 1)); - /* TODO as below */ - file_cn = newnode(cn, track, - trackname_transform("track", track, "display"), - trackname_transform("track", track, "sort"), - 0/*flags*/, 0/*fill*/); - --gets_in_flight; - /* Only bother updating when we've got the lot */ - if(--cn->pending == 0) { - cn->flags &= ~CN_RESOLVING_FILES; - updated_node(cn, gets_in_flight == 0, "got_resolved_file"); - if(!(cn->flags & CN_GETTING_ANY)) - filled(cn); +static void got_resolved_file(void *v, const char *error, const char *track) { + struct choosenode *const cn = v, *file_cn; + + if(error) { + popup_protocol_error(0, error); + } else { + D(("resolved %s %s %d left", cn->path, cnflags(cn), cn->pending - 1)); + /* TODO as below */ + file_cn = newnode(cn, track, + trackname_transform("track", track, "display"), + trackname_transform("track", track, "sort"), + 0/*flags*/, 0/*fill*/); + --gets_in_flight; + /* Only bother updating when we've got the lot */ + if(--cn->pending == 0) { + cn->flags &= ~CN_RESOLVING_FILES; + updated_node(cn, gets_in_flight == 0, "got_resolved_file"); + if(!(cn->flags & CN_GETTING_ANY)) + filled(cn); + } } } diff --git a/disobedience/menu.c b/disobedience/menu.c index b80537e..209438c 100644 --- a/disobedience/menu.c +++ b/disobedience/menu.c @@ -30,7 +30,9 @@ static GtkWidget *properties_widget; /** @brief Main menu widgets */ GtkItemFactory *mainmenufactory; -static void about_popup_got_version(void *v, const char *value); +static void about_popup_got_version(void *v, + const char *error, + const char *value); /** @brief Called when the quit option is activated * @@ -156,12 +158,15 @@ static void manual_popup(gpointer attribute((unused)) callback_data, /** @brief Called when version arrives, displays about... popup */ static void about_popup_got_version(void attribute((unused)) *v, + const char attribute((unused)) *error, const char *value) { GtkWidget *w; char *server_version_string; char *short_version_string; GtkWidget *hbox, *vbox, *title; + if(!value) + value = "[error]"; byte_xasprintf(&server_version_string, "Server version %s", value); byte_xasprintf(&short_version_string, "Disobedience %s", disorder_short_version_string); @@ -224,11 +229,18 @@ void users_set_sensitive(int sensitive) { } /** @brief Called with current user's rights string */ -static void menu_got_rights(void attribute((unused)) *v, const char *value) { +static void menu_got_rights(void attribute((unused)) *v, + const char *error, + const char *value) { rights_type r; - if(parse_rights(value, &r, 0)) + if(error) { + popup_protocol_error(0, error); r = 0; + } else { + if(parse_rights(value, &r, 0)) + r = 0; + } users_set_sensitive(!!(r & RIGHT_ADMIN)); } diff --git a/disobedience/properties.c b/disobedience/properties.c index 96d5c48..ec7b46a 100644 --- a/disobedience/properties.c +++ b/disobedience/properties.c @@ -43,7 +43,7 @@ static const char *get_edited_boolean(struct prefdata *f); static void set_edited_boolean(struct prefdata *f, const char *value); static void set_boolean(struct prefdata *f, const char *value); -static void prefdata_completed(void *v, const char *value); +static void prefdata_completed(void *v, const char *error, const char *value); static void prefdata_onerror(struct callbackdata *cbd, int code, const char *msg); @@ -434,10 +434,13 @@ static void prefdata_onerror(struct callbackdata *cbd, } /* Got the value of a pref */ -static void prefdata_completed(void *v, const char *value) { - struct callbackdata *cbd = v; - - prefdata_completed_common(cbd->u.f, value); +static void prefdata_completed(void *v, const char *error, const char *value) { + if(error) { + } else { + struct callbackdata *cbd = v; + + prefdata_completed_common(cbd->u.f, value); + } } static void prefdata_completed_common(struct prefdata *f, diff --git a/disobedience/queue.c b/disobedience/queue.c index 6008e4e..291fab9 100644 --- a/disobedience/queue.c +++ b/disobedience/queue.c @@ -273,12 +273,13 @@ static void namepart_completed_or_failed(void) { } /** @brief Called when A namepart lookup has completed */ -static void namepart_completed(void *v, const char *value) { - struct callbackdata *cbd = v; - - D(("namepart_completed")); - cache_put(&cachetype_string, cbd->u.key, value); - ++namepart_completions_deferred; +static void namepart_completed(void *v, const char *error, const char *value) { + if(error) { + gtk_label_set_text(GTK_LABEL(report_label), error); + } else { + cache_put(&cachetype_string, v, value); + ++namepart_completions_deferred; + } namepart_completed_or_failed(); } @@ -310,14 +311,9 @@ static void namepart_fill(const char *track, const char *context, const char *part, const char *key) { - struct callbackdata *cbd; - ++namepart_lookups_outstanding; - cbd = xmalloc(sizeof *cbd); - cbd->onerror = namepart_protocol_error; - cbd->u.key = key; disorder_eclient_namepart(client, namepart_completed, - track, context, part, cbd); + track, context, part, (void *)key); } /** @brief Look up a namepart diff --git a/disobedience/users.c b/disobedience/users.c index fe90433..cb44be5 100644 --- a/disobedience/users.c +++ b/disobedience/users.c @@ -520,15 +520,29 @@ static void users_delete(GtkButton attribute((unused)) *button, } } -static void users_got_email(void attribute((unused)) *v, const char *value) { +static void users_got_email(void attribute((unused)) *v, + const char *error, + const char *value) { + if(error) + popup_protocol_error(0, error); users_email = value; } -static void users_got_rights(void attribute((unused)) *v, const char *value) { +static void users_got_rights(void attribute((unused)) *v, + const char *error, + const char *value) { + if(error) + popup_protocol_error(0, error); users_rights = value; } -static void users_got_password(void attribute((unused)) *v, const char *value) { +static void users_got_password(void attribute((unused)) *v, + const char *error, + const char *value) { + if(error) + popup_protocol_error(0, error); + /* TODO if an error occurred gathering user info, we should react in some + * different way */ users_password = value; users_makedetails(users_selected, users_email, diff --git a/lib/eclient.c b/lib/eclient.c index d0e398a..96df2c0 100644 --- a/lib/eclient.c +++ b/lib/eclient.c @@ -834,29 +834,38 @@ static void stash_command(disorder_eclient *c, /* Command support ***********************************************************/ +static const char *errorstring(disorder_eclient *c) { + char *s; + + byte_xasprintf(&s, "%s: %s: %d", c->ident, c->line, c->rc); + return s; +} + /* for commands with a quoted string response */ static void string_response_opcallback(disorder_eclient *c, struct operation *op) { + disorder_eclient_string_response *completed + = (disorder_eclient_string_response *)op->completed; + D(("string_response_callback")); if(c->rc / 100 == 2 || c->rc == 555) { if(op->completed) { if(c->rc == 555) - ((disorder_eclient_string_response *)op->completed)(op->v, NULL); + completed(op->v, NULL, NULL); else if(c->protocol >= 2) { char **rr = split(c->line + 4, 0, SPLIT_QUOTES, 0, 0); if(rr && *rr) - ((disorder_eclient_string_response *)op->completed)(op->v, *rr); + completed(op->v, NULL, *rr); else - /* TODO don't use protocol_error here */ - protocol_error(c, op, c->rc, "%s: %s", c->ident, c->line); + /* TODO error message a is bit lame but generally indicates a server + * bug rather than anything the user can address */ + completed(op->v, "error parsing response", NULL); } else - ((disorder_eclient_string_response *)op->completed)(op->v, - c->line + 4); + completed(op->v, NULL, c->line + 4); } } else - /* TODO don't use protocol_error here */ - protocol_error(c, op, c->rc, "%s: %s", c->ident, c->line); + completed(op->v, errorstring(c), NULL); } /* for commands with a simple integer response */ diff --git a/lib/eclient.h b/lib/eclient.h index 1b79cc0..e459619 100644 --- a/lib/eclient.h +++ b/lib/eclient.h @@ -152,12 +152,18 @@ typedef void disorder_eclient_no_response(void *v); /** @brief String result completion callback * @param v User data - * @param value or NULL + * @param error Error string or NULL on succes + * @param value or NULL if not found * - * @p value can be NULL for disorder_eclient_get(), - * disorder_eclient_get_global() and disorder_eclient_userinfo(). + * @p error will be NULL on success. In this case @p value will be the result + * (which might be NULL for disorder_eclient_get(), + * disorder_eclient_get_global() and disorder_eclient_userinfo()). + * + * @p error will be non-NULL on failure. In this case @p value is always NULL. */ -typedef void disorder_eclient_string_response(void *v, const char *value); +typedef void disorder_eclient_string_response(void *v, + const char *error, + const char *value); typedef void disorder_eclient_integer_response(void *v, long value); /* completion callback with a integer result */ -- [mdw]