From d3a70a285d03224fde9e6ef36eba9de21b773f39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bert=20M=C3=BCnnich?= Date: Wed, 28 Oct 2015 23:03:37 +0100 Subject: Revised error handling - Functions warn() and die() replaced by GNU-like error(3) function - Register cleanup() with atexit(3) - Functions called by cleanup() are marked with CLEANUP and are not allowed to call exit(3) --- commands.c | 2 -- image.c | 11 ++++--- image.h | 2 +- main.c | 101 +++++++++++++++++++++++++++---------------------------------- options.c | 33 ++++++++------------ thumbs.c | 15 ++++----- thumbs.h | 2 +- types.h | 2 ++ util.c | 55 +++++++++++++-------------------- util.h | 5 +-- window.c | 16 +++++----- window.h | 2 +- 12 files changed, 108 insertions(+), 138 deletions(-) diff --git a/commands.c b/commands.c index b1f73ed..ab1e805 100644 --- a/commands.c +++ b/commands.c @@ -30,7 +30,6 @@ #define _IMAGE_CONFIG #include "config.h" -void cleanup(void); void remove_file(int, bool); void load_image(int); void open_info(void); @@ -64,7 +63,6 @@ bool cg_quit(arg_t _) printf("%s\n", files[i].name); } } - cleanup(); exit(EXIT_SUCCESS); } diff --git a/image.c b/image.c index 494ff07..e067c71 100644 --- a/image.c +++ b/image.c @@ -16,6 +16,7 @@ * along with sxiv. If not, see . */ +#include #include #include #include @@ -149,7 +150,7 @@ bool img_load_gif(img_t *img, const fileinfo_t *file) gif = DGifOpenFileName(file->path); #endif if (gif == NULL) { - warn("could not open gif file: %s", file->name); + error(0, 0, "%s: Error opening gif image", file->name); return false; } bg = gif->SBackGroundColor; @@ -274,7 +275,7 @@ bool img_load_gif(img_t *img, const fileinfo_t *file) #endif if (err && (file->flags & FF_WARN)) - warn("corrupted gif file: %s", file->name); + error(0, 0, "%s: Corrupted gif file", file->name); if (img->multi.cnt > 1) { imlib_context_set_image(img->im); @@ -300,7 +301,7 @@ bool img_load(img_t *img, const fileinfo_t *file) (img->im = imlib_load_image(file->path)) == NULL) { if (file->flags & FF_WARN) - warn("could not open image: %s", file->name); + error(0, 0, "%s: Error opening image", file->name); return false; } @@ -325,7 +326,7 @@ bool img_load(img_t *img, const fileinfo_t *file) return true; } -void img_close(img_t *img, bool decache) +CLEANUP void img_close(img_t *img, bool decache) { int i; @@ -464,7 +465,7 @@ void img_render(img_t *img) if (imlib_image_has_alpha()) { if ((bg = imlib_create_image(dw, dh)) == NULL) - die("could not allocate memory"); + error(EXIT_FAILURE, ENOMEM, NULL); imlib_context_set_image(bg); imlib_image_set_has_alpha(0); diff --git a/image.h b/image.h index 35c0d71..8932fdf 100644 --- a/image.h +++ b/image.h @@ -69,7 +69,7 @@ typedef struct { void img_init(img_t*, win_t*); bool img_load(img_t*, const fileinfo_t*); -void img_close(img_t*, bool); +CLEANUP void img_close(img_t*, bool); void img_render(img_t*); diff --git a/main.c b/main.c index b18dc36..00a3ffc 100644 --- a/main.c +++ b/main.c @@ -42,11 +42,6 @@ #define _MAPPINGS_CONFIG #include "config.h" -typedef struct { - const char *name; - char *cmd; -} exec_t; - typedef struct { struct timeval when; bool active; @@ -75,15 +70,20 @@ bool extprefix; bool resized = false; +typedef struct { + int err; + char *cmd; +} extcmd_t; + struct { - char *cmd; + extcmd_t f; int fd; unsigned int i, lastsep; bool open; } info; struct { - char *cmd; + extcmd_t f; bool warned; } keyhandler; @@ -97,26 +97,24 @@ timeout_t timeouts[] = { void cleanup(void) { - static bool in = false; - - if (!in) { - in = true; - img_close(&img, false); - tns_free(&tns); - win_close(&win); - } + img_close(&img, false); + tns_free(&tns); + win_close(&win); } void check_add_file(char *filename, bool given) { + char *path; const char *bn; if (*filename == '\0') return; - if (access(filename, R_OK) < 0) { + if (access(filename, R_OK) < 0 || + (path = realpath(filename, NULL)) == NULL) + { if (given) - warn("could not open file: %s", filename); + error(0, errno, "%s", filename); return; } @@ -126,12 +124,8 @@ void check_add_file(char *filename, bool given) memset(&files[filecnt/2], 0, filecnt/2 * sizeof(*files)); } - if ((files[fileidx].path = realpath(filename, NULL)) == NULL) { - warn("could not get real path of file: %s\n", filename); - return; - } - files[fileidx].name = estrdup(filename); + files[fileidx].path = path; if ((bn = strrchr(files[fileidx].name , '/')) != NULL && bn[1] != '\0') files[fileidx].base = ++bn; else @@ -149,7 +143,6 @@ void remove_file(int n, bool manual) if (filecnt == 1) { if (!manual) fprintf(stderr, "sxiv: no more files to display, aborting\n"); - cleanup(); exit(manual ? EXIT_SUCCESS : EXIT_FAILURE); } if (files[n].flags & FF_MARK) @@ -232,7 +225,7 @@ void open_info(void) static pid_t pid; int pfd[2]; - if (info.cmd == NULL || info.open || win.bar.h == 0) + if (info.f.err != 0 || info.open || win.bar.h == 0) return; if (info.fd != -1) { close(info.fd); @@ -246,9 +239,8 @@ void open_info(void) if ((pid = fork()) == 0) { close(pfd[0]); dup2(pfd[1], 1); - execl(info.cmd, info.cmd, files[fileidx].name, NULL); - warn("could not exec: %s", info.cmd); - exit(EXIT_FAILURE); + execl(info.f.cmd, info.f.cmd, files[fileidx].name, NULL); + error(EXIT_FAILURE, errno, "exec: %s", info.f.cmd); } close(pfd[1]); if (pid < 0) { @@ -386,7 +378,7 @@ void update_info(void) bar_put(r, "%0*d/%d | ", fn, img.multi.sel + 1, img.multi.cnt); } bar_put(r, "%0*d/%d", fw, fileidx + 1, filecnt); - ow_info = info.cmd == NULL; + ow_info = info.f.err != 0; } if (ow_info) { fn = strlen(files[fileidx].name); @@ -469,14 +461,14 @@ void run_key_handler(const char *key, unsigned int mask) FILE *pfs; bool marked = mode == MODE_THUMB && markcnt > 0; bool changed = false; - int f, i, pfd[2], retval, status; + int f, i, pfd[2], status; int fcnt = marked ? markcnt : 1; char kstr[32]; struct stat *oldst, st; - if (keyhandler.cmd == NULL) { + if (keyhandler.f.err != 0) { if (!keyhandler.warned) { - warn("key handler not installed"); + error(0, keyhandler.f.err, "%s", keyhandler.f.cmd); keyhandler.warned = true; } return; @@ -485,12 +477,12 @@ void run_key_handler(const char *key, unsigned int mask) return; if (pipe(pfd) < 0) { - warn("could not create pipe for key handler"); + error(0, errno, "pipe"); return; } if ((pfs = fdopen(pfd[1], "w")) == NULL) { + error(0, errno, "open pipe"); close(pfd[0]), close(pfd[1]); - warn("could not open pipe for key handler"); return; } oldst = emalloc(fcnt * sizeof(*oldst)); @@ -507,14 +499,13 @@ void run_key_handler(const char *key, unsigned int mask) if ((pid = fork()) == 0) { close(pfd[1]); dup2(pfd[0], 0); - execl(keyhandler.cmd, keyhandler.cmd, kstr, NULL); - warn("could not exec key handler"); - exit(EXIT_FAILURE); + execl(keyhandler.f.cmd, keyhandler.f.cmd, kstr, NULL); + error(EXIT_FAILURE, errno, "exec: %s", keyhandler.f.cmd); } close(pfd[0]); if (pid < 0) { + error(0, errno, "fork"); fclose(pfs); - warn("could not fork key handler"); goto end; } @@ -527,9 +518,8 @@ void run_key_handler(const char *key, unsigned int mask) } fclose(pfs); waitpid(pid, &status, 0); - retval = WEXITSTATUS(status); - if (WIFEXITED(status) == 0 || retval != 0) - warn("key handler exited with non-zero return value: %d", retval); + if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) + error(0, 0, "%s: Exited abnormally", keyhandler.f.cmd); for (f = i = 0; f < fcnt; i++) { if ((marked && (files[i].flags & FF_MARK)) || (!marked && i == fileidx)) { @@ -550,7 +540,7 @@ end: if (changed) { img_close(&img, true); load_image(fileidx); - } else if (info.cmd != NULL) { + } else if (info.f.err == 0) { info.open = false; open_info(); } @@ -819,18 +809,18 @@ int main(int argc, char **argv) filename = options->filenames[i]; if (stat(filename, &fstats) < 0) { - warn("could not stat file: %s", filename); + error(0, errno, "%s", filename); continue; } if (!S_ISDIR(fstats.st_mode)) { check_add_file(filename, true); } else { if (!options->recursive) { - warn("ignoring directory: %s", filename); + error(0, 0, "%s: Is a directory", filename); continue; } if (r_opendir(&dir, filename) < 0) { - warn("could not open directory: %s", filename); + error(0, errno, "%s", filename); continue; } start = fileidx; @@ -844,10 +834,8 @@ int main(int argc, char **argv) } } - if (fileidx == 0) { - fprintf(stderr, "sxiv: no valid image file given, aborting\n"); - exit(EXIT_FAILURE); - } + if (fileidx == 0) + error(EXIT_FAILURE, 0, "No valid image file given, aborting"); filecnt = fileidx; fileidx = options->startnum < filecnt ? options->startnum : 0; @@ -860,20 +848,18 @@ int main(int argc, char **argv) dsuffix = "/.config"; } if (homedir != NULL) { - char **cmd[] = { &info.cmd, &keyhandler.cmd }; + extcmd_t *cmd[] = { &info.f, &keyhandler.f }; const char *name[] = { "image-info", "key-handler" }; for (i = 0; i < ARRLEN(cmd); i++) { n = strlen(homedir) + strlen(dsuffix) + strlen(name[i]) + 12; - *cmd[i] = (char*) emalloc(n); - snprintf(*cmd[i], n, "%s%s/sxiv/exec/%s", homedir, dsuffix, name[i]); - if (access(*cmd[i], X_OK) != 0) { - free(*cmd[i]); - *cmd[i] = NULL; - } + cmd[i]->cmd = (char*) emalloc(n); + snprintf(cmd[i]->cmd, n, "%s%s/sxiv/exec/%s", homedir, dsuffix, name[i]); + if (access(cmd[i]->cmd, X_OK) != 0) + cmd[i]->err = errno; } } else { - warn("could not locate exec directory"); + error(0, 0, "Exec directory not found"); } info.fd = -1; @@ -890,10 +876,11 @@ int main(int argc, char **argv) win_open(&win); win_set_cursor(&win, CURSOR_WATCH); + atexit(cleanup); + set_timeout(redraw, 25, false); run(); - cleanup(); return 0; } diff --git a/options.c b/options.c index 882ac49..f7aac23 100644 --- a/options.c +++ b/options.c @@ -47,6 +47,9 @@ void parse_options(int argc, char **argv) char *end, *s; const char *scalemodes = "dfwh"; + progname = strrchr(argv[0], '/'); + progname = progname ? progname + 1 : argv[0]; + _options.from_stdin = false; _options.to_stdout = false; _options.recursive = false; @@ -86,10 +89,8 @@ void parse_options(int argc, char **argv) break; case 'G': n = strtol(optarg, &end, 0); - if (*end != '\0') { - fprintf(stderr, "sxiv: invalid argument for option -G: %s\n", optarg); - exit(EXIT_FAILURE); - } + if (*end != '\0') + error(EXIT_FAILURE, 0, "Invalid argument for option -G: %s", optarg); _options.gamma = n; break; case 'g': @@ -103,10 +104,8 @@ void parse_options(int argc, char **argv) break; case 'n': n = strtol(optarg, &end, 0); - if (*end != '\0' || n <= 0) { - fprintf(stderr, "sxiv: invalid argument for option -n: %s\n", optarg); - exit(EXIT_FAILURE); - } + if (*end != '\0' || n <= 0) + error(EXIT_FAILURE, 0, "Invalid argument for option -n: %s", optarg); _options.startnum = n - 1; break; case 'N': @@ -123,18 +122,14 @@ void parse_options(int argc, char **argv) break; case 'S': n = strtol(optarg, &end, 0); - if (*end != '\0' || n <= 0) { - fprintf(stderr, "sxiv: invalid argument for option -S: %s\n", optarg); - exit(EXIT_FAILURE); - } + if (*end != '\0' || n <= 0) + error(EXIT_FAILURE, 0, "Invalid argument for option -S: %s", optarg); _options.slideshow = n; break; case 's': s = strchr(scalemodes, optarg[0]); - if (s == NULL || *s == '\0' || strlen(optarg) != 1) { - fprintf(stderr, "sxiv: invalid argument for option -s: %s\n", optarg); - exit(EXIT_FAILURE); - } + if (s == NULL || *s == '\0' || strlen(optarg) != 1) + error(EXIT_FAILURE, 0, "Invalid argument for option -s: %s", optarg); _options.scalemode = s - scalemodes; break; case 't': @@ -149,10 +144,8 @@ void parse_options(int argc, char **argv) break; case 'z': n = strtol(optarg, &end, 0); - if (*end != '\0' || n <= 0) { - fprintf(stderr, "sxiv: invalid argument for option -z: %s\n", optarg); - exit(EXIT_FAILURE); - } + if (*end != '\0' || n <= 0) + error(EXIT_FAILURE, 0, "Invalid argument for option -z: %s", optarg); _options.scalemode = SCALE_ZOOM; _options.zoom = (float) n / 100.0; break; diff --git a/thumbs.c b/thumbs.c index 4790ed9..f10a1aa 100644 --- a/thumbs.c +++ b/thumbs.c @@ -16,6 +16,7 @@ * along with sxiv. If not, see . */ +#include #include #include #include @@ -122,7 +123,7 @@ void tns_clean_cache(tns_t *tns) r_dir_t dir; if (r_opendir(&dir, cache_dir) < 0) { - warn("could not open thumbnail cache directory: %s", cache_dir); + error(0, errno, "%s", cache_dir); return; } @@ -140,7 +141,7 @@ void tns_clean_cache(tns_t *tns) } if (delete) { if (unlink(cfile) < 0) - warn("could not delete cache file: %s", cfile); + error(0, errno, "%s", cfile); } free(cfile); } @@ -181,11 +182,11 @@ void tns_init(tns_t *tns, fileinfo_t *files, const int *cnt, int *sel, cache_dir = (char*) emalloc(len); snprintf(cache_dir, len, "%s%s/sxiv", homedir, dsuffix); } else { - warn("could not locate thumbnail cache directory"); + error(0, 0, "Cache directory not found"); } } -void tns_free(tns_t *tns) +CLEANUP void tns_free(tns_t *tns) { int i; @@ -222,7 +223,7 @@ Imlib_Image tns_scale_down(Imlib_Image im, int dim) im = imlib_create_cropped_scaled_image(0, 0, w, h, MAX(z * w, 1), MAX(z * h, 1)); if (im == NULL) - die("could not allocate memory"); + error(EXIT_FAILURE, ENOMEM, NULL); imlib_free_image_and_decache(); } return im; @@ -316,7 +317,7 @@ bool tns_load(tns_t *tns, int n, bool force, bool cache_only) } if (w >= maxwh || h >= maxwh) { if ((im = imlib_create_cropped_image(x, y, w, h)) == NULL) - die("could not allocate memory"); + error(EXIT_FAILURE, ENOMEM, NULL); } imlib_free_image_and_decache(); } @@ -332,7 +333,7 @@ bool tns_load(tns_t *tns, int n, bool force, bool cache_only) (im = imlib_load_image(file->path)) == NULL)) { if (file->flags & FF_WARN) - warn("could not open image: %s", file->name); + error(0, 0, "%s: Error opening image", file->name); return false; } imlib_context_set_image(im); diff --git a/thumbs.h b/thumbs.h index cda2777..7b9987e 100644 --- a/thumbs.h +++ b/thumbs.h @@ -58,7 +58,7 @@ typedef struct { void tns_clean_cache(tns_t*); void tns_init(tns_t*, fileinfo_t*, const int*, int*, win_t*); -void tns_free(tns_t*); +CLEANUP void tns_free(tns_t*); bool tns_load(tns_t*, int, bool, bool); void tns_unload(tns_t*, int); diff --git a/types.h b/types.h index 30475a2..47d115c 100644 --- a/types.h +++ b/types.h @@ -21,6 +21,8 @@ #include +#define CLEANUP + typedef enum { BO_BIG_ENDIAN, BO_LITTLE_ENDIAN diff --git a/util.c b/util.c index 4569489..620c8b6 100644 --- a/util.c +++ b/util.c @@ -26,7 +26,7 @@ #include "options.h" #include "util.h" -void cleanup(void); +const char *progname; void* emalloc(size_t size) { @@ -34,7 +34,7 @@ void* emalloc(size_t size) ptr = malloc(size); if (ptr == NULL) - die("could not allocate memory"); + error(EXIT_FAILURE, errno, NULL); return ptr; } @@ -42,7 +42,7 @@ void* erealloc(void *ptr, size_t size) { ptr = realloc(ptr, size); if (ptr == NULL) - die("could not allocate memory"); + error(EXIT_FAILURE, errno, NULL); return ptr; } @@ -53,40 +53,27 @@ char* estrdup(const char *s) d = malloc(n); if (d == NULL) - die("could not allocate memory"); + error(EXIT_FAILURE, errno, NULL); memcpy(d, s, n); return d; } -void warn(const char* fmt, ...) +void error(int eval, int err, const char* fmt, ...) { - va_list args; - - if (fmt == NULL || options->quiet) - return; - - va_start(args, fmt); - fprintf(stderr, "sxiv: warning: "); - vfprintf(stderr, fmt, args); - fprintf(stderr, "\n"); - va_end(args); -} - -void die(const char* fmt, ...) -{ - va_list args; - - if (fmt == NULL) - return; - - va_start(args, fmt); - fprintf(stderr, "sxiv: error: "); - vfprintf(stderr, fmt, args); - fprintf(stderr, "\n"); - va_end(args); - - cleanup(); - exit(EXIT_FAILURE); + va_list ap; + + fflush(stdout); + fprintf(stderr, "%s: ", progname); + va_start(ap, fmt); + if (fmt != NULL) + vfprintf(stderr, fmt, ap); + va_end(ap); + if (err != 0) + fprintf(stderr, "%s%s", fmt != NULL ? ": " : "", strerror(err)); + fputc('\n', stderr); + + if (eval != 0) + exit(eval); } void size_readable(float *size, const char **unit) @@ -185,7 +172,7 @@ char* r_readdir(r_dir_t *rdir) rdir->name = rdir->stack[--rdir->stlen]; rdir->d = 1; if ((rdir->dir = opendir(rdir->name)) == NULL) - warn("could not open directory: %s", rdir->name); + error(0, errno, "%s", rdir->name); continue; } /* no more entries */ @@ -215,7 +202,7 @@ int r_mkdir(const char *path) *d = '\0'; if (access(dir, F_OK) < 0 && errno == ENOENT) { if (mkdir(dir, 0755) < 0) { - warn("could not create directory: %s", dir); + error(0, errno, "%s", dir); err = -1; } } else if (stat(dir, &stats) < 0 || !S_ISDIR(stats.st_mode)) { diff --git a/util.h b/util.h index 86abea8..8359dc8 100644 --- a/util.h +++ b/util.h @@ -61,12 +61,13 @@ typedef struct { int stlen; } r_dir_t; +extern const char *progname; + void* emalloc(size_t); void* erealloc(void*, size_t); char* estrdup(const char*); -void warn(const char*, ...); -void die(const char*, ...); +void error(int, int, const char*, ...); void size_readable(float*, const char**); diff --git a/window.c b/window.c index 39d648b..696c2ea 100644 --- a/window.c +++ b/window.c @@ -80,7 +80,7 @@ void win_init_font(Display *dpy, const char *fontstr) if ((font.xfont = XLoadQueryFont(dpy, fontstr)) == NULL && (font.xfont = XLoadQueryFont(dpy, "fixed")) == NULL) { - die("could not load font: %s", fontstr); + error(EXIT_FAILURE, 0, "Error loading font '%s'", fontstr); } font.ascent = font.xfont->ascent; font.descent = font.xfont->descent; @@ -97,7 +97,7 @@ unsigned long win_alloc_color(win_t *win, const char *name) DefaultColormap(win->env.dpy, win->env.scr), name, &col, &col) == 0) { - die("could not allocate color: %s", name); + error(EXIT_FAILURE, 0, "Error allocating color '%s'", name); } return col.pixel; } @@ -143,7 +143,7 @@ void win_init(win_t *win) e = &win->env; if ((e->dpy = XOpenDisplay(NULL)) == NULL) - die("could not open display"); + error(EXIT_FAILURE, 0, "Error opening X display"); e->scr = DefaultScreen(e->dpy); e->scrw = DisplayWidth(e->dpy, e->scr); @@ -153,7 +153,7 @@ void win_init(win_t *win) e->depth = DefaultDepth(e->dpy, e->scr); if (setlocale(LC_CTYPE, "") == NULL || XSupportsLocale() == 0) - warn("no locale support"); + error(0, 0, "No locale support"); win_init_font(e->dpy, BAR_FONT); @@ -236,7 +236,7 @@ void win_open(win_t *win) win->x, win->y, win->w, win->h, 0, e->depth, InputOutput, e->vis, 0, NULL); if (win->xwin == None) - die("could not create window"); + error(EXIT_FAILURE, 0, "Error creating X window"); XSelectInput(e->dpy, win->xwin, ButtonReleaseMask | ButtonPressMask | KeyPressMask | @@ -249,7 +249,7 @@ void win_open(win_t *win) if (XAllocNamedColor(e->dpy, DefaultColormap(e->dpy, e->scr), "black", &col, &col) == 0) { - die("could not allocate color: black"); + error(EXIT_FAILURE, 0, "Error allocating color 'black'"); } none = XCreateBitmapFromData(e->dpy, win->xwin, none_data, 8, 8); cnone = XCreatePixmapCursor(e->dpy, none, none, &col, &col, 0, 0); @@ -306,7 +306,7 @@ void win_open(win_t *win) win_toggle_fullscreen(win); } -void win_close(win_t *win) +CLEANUP void win_close(win_t *win) { XFreeCursor(win->env.dpy, carrow); XFreeCursor(win->env.dpy, cnone); @@ -341,7 +341,7 @@ void win_toggle_fullscreen(win_t *win) if (!fs_support) { if (!fs_warned) { - warn("window manager does not support fullscreen"); + error(0, 0, "No fullscreen support"); fs_warned = True; } return; diff --git a/window.h b/window.h index 8761120..31bd898 100644 --- a/window.h +++ b/window.h @@ -90,7 +90,7 @@ extern Atom atoms[ATOM_COUNT]; void win_init(win_t*); void win_open(win_t*); -void win_close(win_t*); +CLEANUP void win_close(win_t*); bool win_configure(win_t*, XConfigureEvent*); -- cgit v1.2.3