From d2bed4990db9bbc1b850021cd357a91fa063d2fc Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Sun, 10 Apr 2011 15:32:25 +0200 Subject: [PATCH] unify error reporting - introduce sys_error() and use it instead of perror() and error(strerror()) in all expected error conditions - perror() is used only for "something's really wrong with the system" kind of errors - file names, etc. are quoted if they are not validated yet, so e.g. an empty string becomes immediately obvious - improve and unify language - add missing newlines --- TODO | 2 - src/compat/config.c | 2 +- src/compat/convert.c | 20 ++++----- src/compat/isync.h | 1 + src/compat/main.c | 9 ++-- src/compat/util.c | 13 ++++++ src/config.c | 3 +- src/drv_imap.c | 7 ++-- src/drv_maildir.c | 99 +++++++++++++++++++++++++------------------- src/isync.h | 3 ++ src/mdconvert.c | 46 +++++++++++++++----- src/socket.c | 49 ++++++++++++---------- src/sync.c | 10 ++--- src/util.c | 17 ++++++++ 14 files changed, 174 insertions(+), 107 deletions(-) diff --git a/TODO b/TODO index b7dd5b2..c3fdb35 100644 --- a/TODO +++ b/TODO @@ -15,8 +15,6 @@ quotas are weird, they make close() fail. clarify error cases of transactions. -cleanup/improve error messages in the drivers. - sync.c does not consider UID 0 valid, which is wrong. fixing this requires an incompatible change or a remapping hack in sync state files. diff --git a/src/compat/config.c b/src/compat/config.c index 85a641d..02115b9 100644 --- a/src/compat/config.c +++ b/src/compat/config.c @@ -97,7 +97,7 @@ load_config( const char *path, config_t ***stor ) if (!(fp = fopen( path, "r" ))) { if (errno != ENOENT) - perror( "fopen" ); + sys_error( "Cannot read config file '%s'", path ); return; } if (!Quiet && !Debug && !Verbose) diff --git a/src/compat/convert.c b/src/compat/convert.c index 0bd0f15..3bd8879 100644 --- a/src/compat/convert.c +++ b/src/compat/convert.c @@ -28,7 +28,6 @@ #include #include #include -#include #include #include @@ -97,10 +96,9 @@ convert( config_t *box ) for (i = 0; i < 3; i++) { nfsnprintf( buf, sizeof(buf), "%s/%s", mboxdir, subdirs[i] ); if (stat( buf, &sb )) { - fprintf( stderr, "ERROR: stat %s: %s (errno %d)\n", buf, - strerror(errno), errno ); + sys_error( "ERROR: cannot access %s", buf ); fprintf( stderr, - "ERROR: %s does not appear to be a valid maildir style mailbox\n", + "ERROR: '%s' does not appear to be a valid maildir style mailbox\n", mboxdir ); goto err1; } @@ -111,7 +109,7 @@ convert( config_t *box ) nfsnprintf( sname, sizeof(sname), "%s/.mbsyncstate", mboxdir ); if ((fd = open( ilname, O_WRONLY|O_CREAT, 0600 )) < 0) { - perror( ilname ); + sys_error( "Cannot create %s", ilname ); goto err1; } #if SEEK_SET != 0 @@ -121,20 +119,20 @@ convert( config_t *box ) lck.l_type = F_WRLCK; #endif if (fcntl( fd, F_SETLKW, &lck )) { - perror( ilname ); + sys_error( "Cannot lock %s", ilname ); err2: close( fd ); goto err1; } if (!(fp = fopen( iuvname, "r" ))) { - perror( iuvname ); + sys_error( "Cannot open %s", iuvname ); goto err2; } fscanf( fp, "%d", &uidval ); fclose( fp ); if (!(fp = fopen( imuname, "r" ))) { - perror( imuname ); + sys_error( "Cannot open %s", imuname ); goto err2; } fscanf( fp, "%d", &maxuid ); @@ -162,7 +160,7 @@ convert( config_t *box ) for (i = 0; i < 2; i++) { nfsnprintf( buf, sizeof(buf), "%s/%s/", mboxdir, subdirs[i] ); if (!(d = opendir( buf ))) { - perror( "opendir" ); + sys_error( "Cannot list %s", buf ); err4: free( msgs ); if (db) @@ -199,7 +197,7 @@ convert( config_t *box ) qsort( msgs, nmsgs, sizeof(msg_t), compare_uids ); if (!(fp = fopen( sname, "w" ))) { - perror( sname ); + sys_error( "Cannot create %s", sname ); goto err4; } if (box->max_messages) { @@ -238,7 +236,7 @@ convert( config_t *box ) rename( iumname, diumname ); } else { if (!(fp = fopen( uvname, "w" ))) { - perror( uvname ); + sys_error( "Cannot create %s", uvname ); goto err4; } fprintf( fp, "%d\n%d\n", uidval, maxuid ); diff --git a/src/compat/isync.h b/src/compat/isync.h index 86dddef..09ec283 100644 --- a/src/compat/isync.h +++ b/src/compat/isync.h @@ -98,4 +98,5 @@ char *nfstrdup( const char *str ); int nfvasprintf( char **str, const char *fmt, va_list va ); int nfasprintf( char **str, const char *fmt, ... ); int nfsnprintf( char *buf, int blen, const char *fmt, ... ); +void sys_error( const char *, ... ); void ATTR_NORETURN oob( void ); diff --git a/src/compat/main.c b/src/compat/main.c index 1220e5a..10ade7f 100644 --- a/src/compat/main.c +++ b/src/compat/main.c @@ -29,7 +29,6 @@ #include #include #include -#include #include #include #include @@ -336,7 +335,7 @@ main( int argc, char **argv ) struct dirent *de; if (!(dir = opendir( xmaildir ))) { - fprintf( stderr, "%s: %s\n", xmaildir, strerror(errno) ); + sys_error( "Cannot list '%s'", xmaildir ); return 1; } while ((de = readdir( dir ))) { @@ -379,13 +378,13 @@ main( int argc, char **argv ) outconfig = path2; } if ((fd = creat( outconfig, 0666 )) < 0) { - fprintf( stderr, "Error: cannot write new config %s: %s\n", outconfig, strerror(errno) ); + sys_error( "Error: cannot create config file '%s'", outconfig ); return 1; } } else { strcpy( path2, "/tmp/mbsyncrcXXXXXX" ); if ((fd = mkstemp( path2 )) < 0) { - fprintf( stderr, "Can't create temp file\n" ); + sys_error( "Error: cannot create temporary config file" ); return 1; } } @@ -432,6 +431,6 @@ main( int argc, char **argv ) add_arg( &args, find_box( argv[optind] )->channel_name ); } execvp( args[0], args ); - perror( args[0] ); + sys_error( "Cannot execute %s", args[0] ); return 1; } diff --git a/src/compat/util.c b/src/compat/util.c index 7ac085b..1cb0f79 100644 --- a/src/compat/util.c +++ b/src/compat/util.c @@ -27,6 +27,19 @@ #include #include +void +sys_error( const char *msg, ... ) +{ + va_list va; + char buf[1024]; + + va_start( va, msg ); + if ((unsigned)vsnprintf( buf, sizeof(buf), msg, va ) >= sizeof(buf)) + oob(); + va_end( va ); + perror( buf ); +} + char * next_arg( char **s ) { diff --git a/src/config.c b/src/config.c index ef7641a..ed0466f 100644 --- a/src/config.c +++ b/src/config.c @@ -25,7 +25,6 @@ #include #include -#include #include #include #include @@ -272,7 +271,7 @@ load_config( const char *where, int pseudo ) info( "Reading configuration file %s\n", cfile.file ); if (!(cfile.fp = fopen( cfile.file, "r" ))) { - perror( "Cannot open config file" ); + sys_error( "Cannot open config file '%s'", cfile.file ); return 1; } buf[sizeof(buf) - 1] = 0; diff --git a/src/drv_imap.c b/src/drv_imap.c index 574e3a4..a22c4c6 100644 --- a/src/drv_imap.c +++ b/src/drv_imap.c @@ -30,7 +30,6 @@ #include #include #include -#include #include #include @@ -841,7 +840,7 @@ imap_socket_read( void *aux ) if (*arg == '*') { arg = next_arg( &cmd ); if (!arg) { - error( "IMAP error: unable to parse untagged response\n" ); + error( "IMAP error: malformed untagged response\n" ); break; } @@ -1838,8 +1837,8 @@ imap_parse_store( conffile_t *cfg, store_conf_t **storep, int *err ) else if (!strcasecmp( "CertificateFile", cfg->cmd )) { server->sconf.cert_file = expand_strdup( cfg->val ); if (access( server->sconf.cert_file, R_OK )) { - error( "%s:%d: CertificateFile '%s': %s\n", - cfg->file, cfg->line, server->sconf.cert_file, strerror( errno ) ); + sys_error( "%s:%d: CertificateFile '%s'", + cfg->file, cfg->line, server->sconf.cert_file ); *err = 1; } } else if (!strcasecmp( "RequireSSL", cfg->cmd )) diff --git a/src/drv_maildir.c b/src/drv_maildir.c index 1fe3c51..2173e60 100644 --- a/src/drv_maildir.c +++ b/src/drv_maildir.c @@ -101,7 +101,7 @@ maildir_open_store( store_conf_t *conf, struct stat st; if (stat( conf->path, &st ) || !S_ISDIR(st.st_mode)) { - error( "Maildir error: cannot open store %s\n", conf->path ); + error( "Maildir error: cannot open store '%s'\n", conf->path ); cb( 0, aux ); return; } @@ -173,7 +173,7 @@ maildir_list( store_t *gctx, struct dirent *de; if (!(dir = opendir( gctx->conf->path ))) { - error( "%s: %s\n", gctx->conf->path, strerror(errno) ); + sys_error( "Maildir error: cannot list %s", gctx->conf->path ); maildir_invoke_bad_callback( gctx ); cb( DRV_CANCELED, aux ); return; @@ -240,16 +240,14 @@ maildir_validate( const char *prefix, const char *box, int create, maildir_store if (errno == ENOENT) { if (create) { if (mkdir( buf, 0700 )) { - error( "Maildir error: mkdir %s: %s (errno %d)\n", - buf, strerror(errno), errno ); + sys_error( "Maildir error: cannot create mailbox '%s'", buf ); maildir_invoke_bad_callback( &ctx->gen ); return DRV_CANCELED; } for (i = 0; i < 3; i++) { memcpy( buf + bl, subdirs[i], 4 ); if (mkdir( buf, 0700 )) { - error( "Maildir error: mkdir %s: %s (errno %d)\n", - buf, strerror(errno), errno ); + sys_error( "Maildir error: cannot create directory %s", buf ); return DRV_BOX_BAD; } } @@ -258,8 +256,7 @@ maildir_validate( const char *prefix, const char *box, int create, maildir_store return DRV_BOX_BAD; } } else { - error( "Maildir error: stat %s: %s (errno %d)\n", - buf, strerror(errno), errno ); + sys_error( "Maildir error: cannot access mailbox '%s'", buf ); return DRV_BOX_BAD; } } else { @@ -273,8 +270,7 @@ maildir_validate( const char *prefix, const char *box, int create, maildir_store memcpy( buf + bl, "tmp/", 5 ); bl += 4; if (!(dirp = opendir( buf ))) { - error( "Maildir error: opendir: %s: %s (errno %d)\n", - buf, strerror(errno), errno ); + sys_error( "Maildir error: cannot list %s", buf ); return DRV_BOX_BAD; } time( &now ); @@ -282,16 +278,14 @@ maildir_validate( const char *prefix, const char *box, int create, maildir_store nfsnprintf( buf + bl, sizeof(buf) - bl, "%s", entry->d_name ); if (stat( buf, &st )) { if (errno != ENOENT) - error( "Maildir error: stat: %s: %s (errno %d)\n", - buf, strerror(errno), errno ); + sys_error( "Maildir error: cannot access %s", buf ); } else if (S_ISREG(st.st_mode) && now - st.st_ctime >= _24_HOURS) { /* this should happen infrequently enough that it won't be * bothersome to the user to display when it occurs. */ info( "Maildir notice: removing stale file %s\n", buf ); if (unlink( buf ) && errno != ENOENT) - error( "Maildir error: unlink: %s: %s (errno %d)\n", - buf, strerror(errno), errno ); + sys_error( "Maildir error: cannot remove %s", buf ); } } closedir( dirp ); @@ -360,9 +354,8 @@ maildir_store_uid( maildir_store_t *ctx ) } static int -maildir_init_uid( maildir_store_t *ctx, const char *msg ) +maildir_init_uid( maildir_store_t *ctx ) { - info( "Maildir notice: %s.\n", msg ? msg : "cannot read UIDVALIDITY, creating new" ); ctx->gen.uidvalidity = time( 0 ); ctx->nuid = 0; ctx->uvok = 0; @@ -376,6 +369,13 @@ maildir_init_uid( maildir_store_t *ctx, const char *msg ) return maildir_store_uid( ctx ); } +static int +maildir_init_uid_new( maildir_store_t *ctx ) +{ + info( "Maildir notice: no UIDVALIDITY, creating new.\n" ); + return maildir_init_uid( ctx ); +} + static int maildir_uidval_lock( maildir_store_t *ctx ) { @@ -402,7 +402,7 @@ maildir_uidval_lock( maildir_store_t *ctx ) lseek( ctx->uvfd, 0, SEEK_SET ); if ((n = read( ctx->uvfd, buf, sizeof(buf) )) <= 0 || (buf[n] = 0, sscanf( buf, "%d\n%d", &ctx->gen.uidvalidity, &ctx->nuid ) != 2)) { - return maildir_init_uid( ctx, 0 ); + return maildir_init_uid_new( ctx ); } else ctx->uvok = 1; return DRV_OK; @@ -532,7 +532,7 @@ maildir_scan( maildir_store_t *ctx, msglist_t *msglist ) for (i = 0; i < 2; i++) { memcpy( buf + bl, subdirs[i], 4 ); if (!(d = opendir( buf ))) { - perror( buf ); + sys_error( "Maildir error: cannot list %s", buf ); #ifdef USE_DB if (!ctx->db) #endif /* USE_DB */ @@ -633,7 +633,8 @@ maildir_scan( maildir_store_t *ctx, msglist_t *msglist ) entry = &msglist->ents[i]; if (entry->uid != INT_MAX) { if (uid == entry->uid) { - if ((ret = maildir_init_uid( ctx, "duplicate UID; changing UIDVALIDITY" )) != DRV_OK) { + info( "Maildir notice: duplicate UID; changing UIDVALIDITY.\n"); + if ((ret = maildir_init_uid( ctx )) != DRV_OK) { maildir_free_scan( msglist ); return ret; } @@ -670,13 +671,14 @@ maildir_scan( maildir_store_t *ctx, msglist_t *msglist ) memcpy( nbuf, buf, bl + 4 ); nfsnprintf( nbuf + bl + 4, sizeof(nbuf) - bl - 4, "%s", entry->base ); if (rename( nbuf, buf )) { - notok: if (errno != ENOENT) { - perror( buf ); + sys_error( "Maildir error: cannot rename %s to %s", nbuf, buf ); + fail: maildir_uidval_unlock( ctx ); maildir_free_scan( msglist ); return DRV_BOX_BAD; } + retry: maildir_free_scan( msglist ); goto again; } @@ -685,13 +687,23 @@ maildir_scan( maildir_store_t *ctx, msglist_t *msglist ) memcpy( entry->base, buf + bl + 4, fnl ); } if (ctx->gen.opts & OPEN_SIZE) { - if (stat( buf, &st )) - goto notok; + if (stat( buf, &st )) { + if (errno != ENOENT) { + sys_error( "Maildir error: cannot stat %s", buf ); + goto fail; + } + goto retry; + } entry->size = st.st_size; } if (ctx->gen.opts & OPEN_FIND) { - if (!(f = fopen( buf, "r" ))) - goto notok; + if (!(f = fopen( buf, "r" ))) { + if (errno != ENOENT) { + sys_error( "Maildir error: cannot open %s", buf ); + goto fail; + } + goto retry; + } while (fgets( nbuf, sizeof(nbuf), f )) { if (!nbuf[0] || nbuf[0] == '\n') break; @@ -767,7 +779,7 @@ maildir_select( store_t *gctx, int create, nfsnprintf( uvpath, sizeof(uvpath), "%s/.uidvalidity", gctx->path ); #ifndef USE_DB if ((ctx->uvfd = open( uvpath, O_RDWR|O_CREAT, 0600 )) < 0) { - perror( uvpath ); + sys_error( "Maildir error: cannot write %s", uvpath ); cb( DRV_BOX_BAD, aux ); return; } @@ -783,7 +795,7 @@ maildir_select( store_t *gctx, int create, if ((ctx->uvfd = open( uvpath, O_RDWR|O_CREAT, 0600 )) >= 0) goto fnok; } - perror( uvpath ); + sys_error( "Maildir error: cannot write %s", uvpath ); cb( DRV_BOX_BAD, aux ); return; } @@ -793,7 +805,7 @@ maildir_select( store_t *gctx, int create, #endif lck.l_type = F_WRLCK; if (fcntl( ctx->uvfd, F_SETLKW, &lck )) { - perror( uvpath ); + sys_error( "Maildir error: cannot lock %s", uvpath ); bork: close( ctx->uvfd ); ctx->uvfd = -1; @@ -817,7 +829,7 @@ maildir_select( store_t *gctx, int create, ctx->db->err( ctx->db, ret, "Maildir error: db->get()" ); goto dbork; } - if (maildir_init_uid( ctx, 0 ) != DRV_OK) + if (maildir_init_uid_new( ctx ) != DRV_OK) goto dbork; } else { ctx->gen.uidvalidity = ((int *)value.data)[0]; @@ -926,12 +938,13 @@ maildir_rescan( maildir_store_t *ctx ) } static int -maildir_again( maildir_store_t *ctx, maildir_message_t *msg, const char *fn ) +maildir_again( maildir_store_t *ctx, maildir_message_t *msg, + const char *err, const char *fn, const char *fn2 ) { int ret; if (errno != ENOENT) { - perror( fn ); + sys_error( err, fn, fn2 ); return DRV_BOX_BAD; } if ((ret = maildir_rescan( ctx )) != DRV_OK) @@ -953,7 +966,7 @@ maildir_fetch_msg( store_t *gctx, message_t *gmsg, msg_data_t *data, nfsnprintf( buf, sizeof(buf), "%s/%s/%s", gctx->path, subdirs[gmsg->status & M_RECENT], msg->base ); if ((fd = open( buf, O_RDONLY )) >= 0) break; - if ((ret = maildir_again( ctx, msg, buf )) != DRV_OK) { + if ((ret = maildir_again( ctx, msg, "Cannot open %s", buf, 0 )) != DRV_OK) { cb( ret, aux ); return; } @@ -962,7 +975,7 @@ maildir_fetch_msg( store_t *gctx, message_t *gmsg, msg_data_t *data, data->len = st.st_size; data->data = nfmalloc( data->len ); if (read( fd, data->data, data->len ) != data->len) { - perror( buf ); + sys_error( "Maildir error: cannot read %s", buf ); close( fd ); cb( DRV_MSG_BAD, aux ); return; @@ -1028,7 +1041,7 @@ maildir_store_msg( store_t *gctx, msg_data_t *data, int to_trash, nfsnprintf( buf, sizeof(buf), "%s%s/tmp/%s%s", prefix, box, base, fbuf ); if ((fd = open( buf, O_WRONLY|O_CREAT|O_EXCL, 0600 )) < 0) { if (errno != ENOENT) { - perror( buf ); + sys_error( "Maildir error: cannot create %s", buf ); free( data->data ); cb( DRV_BOX_BAD, 0, aux ); return; @@ -1039,7 +1052,7 @@ maildir_store_msg( store_t *gctx, msg_data_t *data, int to_trash, return; } if ((fd = open( buf, O_WRONLY|O_CREAT|O_EXCL, 0600 )) < 0) { - perror( buf ); + sys_error( "Maildir error: cannot create %s", buf ); free( data->data ); cb( DRV_BOX_BAD, 0, aux ); return; @@ -1049,23 +1062,23 @@ maildir_store_msg( store_t *gctx, msg_data_t *data, int to_trash, free( data->data ); if (ret != data->len) { if (ret < 0) - perror( buf ); + sys_error( "Maildir error: cannot write %s", buf ); else - error( "Maildir error: %s: partial write\n", buf ); + error( "Maildir error: cannot write %s. Disk full?\n", buf ); close( fd ); cb( DRV_BOX_BAD, 0, aux ); return; } if (close( fd ) < 0) { /* Quota exceeded may cause this. */ - perror( buf ); + sys_error( "Maildir error: cannot write %s", buf ); cb( DRV_BOX_BAD, 0, aux ); return; } /* Moving seen messages to cur/ is strictly speaking incorrect, but makes mutt happy. */ nfsnprintf( nbuf, sizeof(nbuf), "%s%s/%s/%s%s", prefix, box, subdirs[!(data->flags & F_SEEN)], base, fbuf ); if (rename( buf, nbuf )) { - perror( nbuf ); + sys_error( "Maildir error: cannot rename %s to %s", buf, nbuf ); cb( DRV_BOX_BAD, 0, aux ); return; } @@ -1131,7 +1144,7 @@ maildir_set_flags( store_t *gctx, message_t *gmsg, int uid, int add, int del, } if (!rename( buf, nbuf )) break; - if ((ret = maildir_again( ctx, msg, buf )) != DRV_OK) { + if ((ret = maildir_again( ctx, msg, "Maildir error: cannot rename %s to %s", buf, nbuf )) != DRV_OK) { cb( ret, aux ); return; } @@ -1189,12 +1202,12 @@ maildir_trash_msg( store_t *gctx, message_t *gmsg, if (!rename( buf, nbuf )) break; if (errno != ENOENT) { - perror( nbuf ); + sys_error( "Maildir error: cannot move %s to %s", buf, nbuf ); cb( DRV_BOX_BAD, aux ); return; } } - if ((ret = maildir_again( ctx, msg, buf )) != DRV_OK) { + if ((ret = maildir_again( ctx, msg, "Maildir error: cannot move %s to %s", buf, nbuf )) != DRV_OK) { cb( ret, aux ); return; } @@ -1232,7 +1245,7 @@ maildir_close( store_t *gctx, if (errno == ENOENT) retry = 1; else - perror( buf ); + sys_error( "Maildir error: cannot remove %s", buf ); } else { msg->status |= M_DEAD; gctx->count--; diff --git a/src/isync.h b/src/isync.h index e72c627..c9b5798 100644 --- a/src/isync.h +++ b/src/isync.h @@ -83,6 +83,7 @@ typedef struct { int fd; int state; const server_conf_t *conf; /* needed during connect */ + char *name; #ifdef HAVE_LIBSSL SSL *ssl; #endif @@ -366,6 +367,7 @@ static INLINE void socket_init( conn_t *conn, conn->write_callback = write_callback; conn->callback_aux = aux; conn->fd = -1; + conn->name = 0; conn->write_buf_append = &conn->write_buf; } void socket_connect( conn_t *conn, void (*cb)( int ok, void *aux ) ); @@ -397,6 +399,7 @@ void info( const char *, ... ); void infon( const char *, ... ); void warn( const char *, ... ); void error( const char *, ... ); +void sys_error( const char *, ... ); char *next_arg( char ** ); diff --git a/src/mdconvert.c b/src/mdconvert.c index c2fbf25..bd4b7cb 100644 --- a/src/mdconvert.c +++ b/src/mdconvert.c @@ -35,6 +35,32 @@ #define EXE "mdconvert" +#if __GNUC__ > 2 || (__GNUC__ == 2 && __GNUC_MINOR__ > 4) +# define ATTR_NORETURN __attribute__((noreturn)) +#else +# define ATTR_NORETURN +#endif + +static void ATTR_NORETURN +oob( void ) +{ + fputs( "Fatal: buffer too small. Please report a bug.\n", stderr ); + abort(); +} + +static void +sys_error( const char *msg, ... ) +{ + va_list va; + char buf[1024]; + + va_start( va, msg ); + if ((unsigned)vsnprintf( buf, sizeof(buf), msg, va ) >= sizeof(buf)) + oob(); + va_end( va ); + perror( buf ); +} + static int nfsnprintf( char *buf, int blen, const char *fmt, ... ) { @@ -42,10 +68,8 @@ nfsnprintf( char *buf, int blen, const char *fmt, ... ) va_list va; va_start( va, fmt ); - if (blen <= 0 || (unsigned)(ret = vsnprintf( buf, blen, fmt, va )) >= (unsigned)blen) { - fputs( "Fatal: buffer too small. Please report a bug.\n", stderr ); - abort(); - } + if (blen <= 0 || (unsigned)(ret = vsnprintf( buf, blen, fmt, va )) >= (unsigned)blen) + oob(); va_end( va ); return ret; } @@ -81,15 +105,15 @@ convert( const char *box, int altmap ) nfsnprintf( tdpath, sizeof(tdpath), "%s.tmp", dpath ); if ((sfd = open( spath, O_RDWR )) < 0) { if (errno != ENOENT) - perror( spath ); + sys_error( "Cannot open %s", spath ); return 1; } if (fcntl( sfd, F_SETLKW, &lck )) { - perror( spath ); + sys_error( "Cannot lock %s", spath ); goto sbork; } if ((dfd = open( tdpath, O_RDWR|O_CREAT, 0600 )) < 0) { - perror( tdpath ); + sys_error( "Cannot create %s", tdpath ); goto sbork; } if (db_create( &db, 0, 0 )) { @@ -138,7 +162,7 @@ convert( const char *box, int altmap ) for (i = 0; i < 2; i++) { bl = nfsnprintf( buf, sizeof(buf), "%s/%s/", box, subdirs[i] ); if (!(d = opendir( buf ))) { - perror( "opendir" ); + sys_error( "Cannot list %s", buf ); goto dbork; } while ((e = readdir( d ))) { @@ -185,7 +209,7 @@ convert( const char *box, int altmap ) if (rename( buf, buf2 )) { if (errno == ENOENT) goto again; - perror( buf ); + sys_error( "Cannot rename %s to %s", buf, buf2 ); ebork: closedir( d ); goto dbork; @@ -198,11 +222,11 @@ convert( const char *box, int altmap ) db->close( db, 0 ); close( dfd ); if (rename( tdpath, dpath )) { - perror( dpath ); + sys_error( "Cannot rename %s to %s", tdpath, dpath ); return 1; } if (unlink( spath )) - perror( spath ); + sys_error( "Cannot remove %s", spath ); close( sfd ); return 0; } diff --git a/src/socket.c b/src/socket.c index bf8a837..ca44bb3 100644 --- a/src/socket.c +++ b/src/socket.c @@ -79,15 +79,15 @@ ssl_return( const char *func, conn_t *conn, int ret ) case SSL_ERROR_SSL: if (!(err = ERR_get_error())) { if (ret == 0) - error( "SSL_%s: unexpected EOF\n", func ); + error( "Socket error: secure %s %s: unexpected EOF\n", func, conn->name ); else - error( "SSL_%s: %s\n", func, strerror( errno ) ); + sys_error( "Socket error: secure %s %s", func, conn->name ); } else { - error( "SSL_%s: %s\n", func, ERR_error_string( err, 0 ) ); + error( "Socket error: secure %s %s: %s\n", func, conn->name, ERR_error_string( err, 0 ) ); } break; default: - error( "SSL_%s: unhandled SSL error %d\n", func, err ); + error( "Socket error: secure %s %s: unhandled SSL error %d\n", func, conn->name, err ); break; } if (conn->state == SCK_STARTTLS) @@ -148,11 +148,11 @@ verify_cert( const server_conf_t *conf, conn_t *sock ) while (conf->cert_file) { /* while() instead of if() so break works */ if (X509_cmp_current_time( X509_get_notBefore( cert )) >= 0) { - error( "Server certificate is not yet valid" ); + error( "Server certificate is not yet valid\n" ); break; } if (X509_cmp_current_time( X509_get_notAfter( cert )) <= 0) { - error( "Server certificate has expired" ); + error( "Server certificate has expired\n" ); break; } if (!X509_digest( cert, EVP_sha1(), md, &n )) { @@ -160,8 +160,7 @@ verify_cert( const server_conf_t *conf, conn_t *sock ) break; } if (!(fp = fopen( conf->cert_file, "rt" ))) { - error( "Unable to load CertificateFile '%s': %s\n", - conf->cert_file, strerror( errno ) ); + sys_error( "Unable to load CertificateFile '%s'", conf->cert_file ); return -1; } err = -1; @@ -197,7 +196,7 @@ verify_cert( const server_conf_t *conf, conn_t *sock ) X509_STORE_CTX_cleanup( &xsc ); if (!err) return 0; - error( "Error, can't verify certificate: %s (%d)\n", + error( "Error, cannot verify certificate: %s (%d)\n", X509_verify_cert_error_string( err ), err ); X509_NAME_oneline( X509_get_subject_name( cert ), buf, sizeof(buf) ); @@ -286,7 +285,7 @@ socket_start_tls( conn_t *conn, void (*cb)( int ok, void *aux ) ) static void start_tls_p2( conn_t *conn ) { - switch (ssl_return( "connect", conn, SSL_connect( conn->ssl ) )) { + switch (ssl_return( "connect to", conn, SSL_connect( conn->ssl ) )) { case -1: start_tls_p3( conn, 0 ); break; @@ -337,7 +336,8 @@ socket_connect( conn_t *sock, void (*cb)( int ok, void *aux ) ) /* open connection to IMAP server */ if (conf->tunnel) { - infon( "Starting tunnel '%s'... ", conf->tunnel ); + nfasprintf( &sock->name, "tunnel '%s'", conf->tunnel ); + infon( "Starting %s... ", sock->name ); if (socketpair( PF_UNIX, SOCK_STREAM, 0, a )) { perror( "socketpair" ); @@ -387,11 +387,12 @@ socket_connect( conn_t *sock, void (*cb)( int ok, void *aux ) ) fcntl( s, F_SETFL, O_NONBLOCK ); add_fd( s, socket_fd_cb, sock ); - infon( "Connecting to %s (%s:%hu) ... ", - conf->host, inet_ntoa( addr.sin_addr ), ntohs( addr.sin_port ) ); + nfasprintf( &sock->name, "%s (%s:%hu)", + conf->host, inet_ntoa( addr.sin_addr ), ntohs( addr.sin_port ) ); + infon( "Connecting to %s... ", sock->name ); if (connect( s, (struct sockaddr *)&addr, sizeof(addr) )) { if (errno != EINPROGRESS) { - perror( "connect" ); + sys_error( "Cannot connect to %s", sock->name ); socket_close_internal( sock ); goto bail; } @@ -416,19 +417,17 @@ socket_connected( conn_t *conn ) int soerr; socklen_t selen = sizeof(soerr); - infon( "Connecting to %s: ", conn->conf->host ); if (getsockopt( conn->fd, SOL_SOCKET, SO_ERROR, &soerr, &selen )) { perror( "getsockopt" ); exit( 1 ); } if (soerr) { errno = soerr; - perror( "connect" ); + sys_error( "Cannot connect to %s", conn->name ); socket_close_internal( conn ); socket_connect_bail( conn ); return; } - info( "ok\n" ); socket_connected2( conn ); } @@ -443,6 +442,8 @@ socket_connected2( conn_t *conn ) static void socket_connect_bail( conn_t *conn ) { + free( conn->name ); + conn->name = 0; conn->callbacks.connect( 0, conn->callback_aux ); } @@ -453,6 +454,8 @@ socket_close( conn_t *sock ) { if (sock->fd >= 0) socket_close_internal( sock ); + free( sock->name ); + sock->name = 0; #ifdef HAVE_LIBSSL if (sock->ssl) { SSL_free( sock->ssl ); @@ -478,7 +481,7 @@ socket_fill( conn_t *sock ) buf = sock->buf + n; #ifdef HAVE_LIBSSL if (sock->ssl) { - if ((n = ssl_return( "read", sock, SSL_read( sock->ssl, buf, len ) )) <= 0) + if ((n = ssl_return( "read from", sock, SSL_read( sock->ssl, buf, len ) )) <= 0) return; if (n == len && SSL_pending( sock->ssl )) fake_fd( sock->fd, POLLIN ); @@ -486,11 +489,11 @@ socket_fill( conn_t *sock ) #endif { if ((n = read( sock->fd, buf, len )) < 0) { - perror( "read" ); + sys_error( "Socket error: read from %s", sock->name ); socket_fail( sock ); return; } else if (!n) { - error( "read: unexpected EOF\n" ); + error( "Socket error: read from %s: unexpected EOF\n", sock->name ); socket_fail( sock ); return; } @@ -549,12 +552,12 @@ do_write( conn_t *sock, char *buf, int len ) assert( sock->fd >= 0 ); #ifdef HAVE_LIBSSL if (sock->ssl) - return ssl_return( "write", sock, SSL_write( sock->ssl, buf, len ) ); + return ssl_return( "write to", sock, SSL_write( sock->ssl, buf, len ) ); #endif n = write( sock->fd, buf, len ); if (n < 0) { if (errno != EAGAIN && errno != EWOULDBLOCK) { - perror( "write" ); + sys_error( "Socket error: write to %s", sock->name ); socket_fail( sock ); } else { n = 0; @@ -646,7 +649,7 @@ socket_fd_cb( int events, void *aux ) conn_t *conn = (conn_t *)aux; if (events & POLLERR) { - error( "Unidentified socket error.\n" ); + error( "Unidentified socket error from %s.\n", conn->name ); socket_fail( conn ); return; } diff --git a/src/sync.c b/src/sync.c index 9db8228..d72ae62 100644 --- a/src/sync.c +++ b/src/sync.c @@ -40,7 +40,7 @@ void Fclose( FILE *f ) { if (fclose( f ) == EOF) { - perror( "cannot close file" ); + sys_error( "Error: cannot close file. Disk full?" ); exit( 1 ); } } @@ -55,7 +55,7 @@ Fprintf( FILE *f, const char *msg, ... ) r = vfprintf( f, msg, va ); va_end( va ); if (r < 0) { - perror( "cannot write file" ); + sys_error( "Error: cannot write file. Disk full?" ); exit( 1 ); } } @@ -624,12 +624,12 @@ box_selected( int sts, void *aux ) } free( csname ); if (!(s = strrchr( svars->dname, '/' ))) { - error( "Error: invalid SyncState '%s'\n", svars->dname ); + error( "Error: invalid SyncState location '%s'\n", svars->dname ); goto sbail; } *s = 0; if (mkdir( svars->dname, 0700 ) && errno != EEXIST) { - error( "Error: cannot create SyncState directory '%s': %s\n", svars->dname, strerror(errno) ); + sys_error( "Error: cannot create SyncState directory '%s'", svars->dname ); goto sbail; } *s = '/'; @@ -645,7 +645,7 @@ box_selected( int sts, void *aux ) lck.l_type = F_WRLCK; #endif if ((svars->lfd = open( svars->lname, O_WRONLY|O_CREAT, 0666 )) < 0) { - error( "Error: cannot create lock file %s: %s\n", svars->lname, strerror(errno) ); + sys_error( "Error: cannot create lock file %s", svars->lname ); svars->ret = SYNC_FAIL; sync_bail2( svars ); return; diff --git a/src/util.c b/src/util.c index 2c4179c..11ce4fc 100644 --- a/src/util.c +++ b/src/util.c @@ -120,6 +120,23 @@ error( const char *msg, ... ) va_end( va ); } +void +sys_error( const char *msg, ... ) +{ + va_list va; + char buf[1024]; + + if (need_nl) { + putchar( '\n' ); + need_nl = 0; + } + va_start( va, msg ); + if ((unsigned)vsnprintf( buf, sizeof(buf), msg, va ) >= sizeof(buf)) + oob(); + va_end( va ); + perror( buf ); +} + char * next_arg( char **s ) {