cleanup error message handling

The top-level funtion in a thread should print errors using
priv_get_err(), while lower-level functions should set error messages
using priv_set_err() except that error mesesages should be printed
immediately, e.g., under walk_src_path().

6 files changed, 58 insertions, 58 deletions

diff --git a/src/main.c b/src/main.c
index 30e6550..fae5e0d 100644
--- a/src/main.c
+++ b/src/main.c
@@ -186,8 +186,10 @@ struct target *validate_targets(char **arg, int len)
 	for (n = 0; n < len; n++) {
 		t[n].copy =
 			split_user_host_path(arg[n], &t[n].user, &t[n].host, &t[n].path);
-		if (!t[n].copy)
+		if (!t[n].copy) {
+			pr_err("failed to parse '%s'", arg[n]);
 			goto free_target_out;
+		}
 	}
 
 	/* check all user@host are identical. t[len - 1] is destination,
@@ -421,8 +423,6 @@ int main(int argc, char **argv)
 		pr_err("mscp_start: %s", priv_get_err());
 
 	ret = mscp_join(m);
-	if (ret != 0)
-		pr_err("mscp_join: %s", priv_get_err());
 
 	pthread_cancel(tid_stat);
 	pthread_join(tid_stat, NULL);
diff --git a/src/mscp.c b/src/mscp.c
index c3a088c..ececb83 100644
--- a/src/mscp.c
+++ b/src/mscp.c
@@ -100,7 +100,7 @@ static int expand_coremask(const char *coremask, int **cores, int *nr_cores)
 
 	core_list = realloc(NULL, sizeof(int) * 64);
 	if (!core_list) {
-		priv_set_errv("failed to realloc: %s", strerrno());
+		priv_set_errv("realloc: %s", strerrno());
 		return -1;
 	}
 
@@ -232,7 +232,7 @@ struct mscp *mscp_init(const char *remote_host, int direction, struct mscp_opts
 
 	m = malloc(sizeof(*m));
 	if (!m) {
-		priv_set_errv("failed to allocate memory: %s", strerrno());
+		priv_set_errv("malloc: %s", strerrno());
 		return NULL;
 	}
 
@@ -251,7 +251,7 @@ struct mscp *mscp_init(const char *remote_host, int direction, struct mscp_opts
 
 	m->remote = strdup(remote_host);
 	if (!m->remote) {
-		priv_set_errv("failed to allocate memory: %s", strerrno());
+		priv_set_errv("strdup: %s", strerrno());
 		goto free_out;
 	}
 	m->direction = direction;
@@ -260,6 +260,7 @@ struct mscp *mscp_init(const char *remote_host, int direction, struct mscp_opts
 		if (expand_coremask(o->coremask, &m->cores, &m->nr_cores) < 0)
 			goto free_out;
 		char b[512], c[8];
+		memset(b, 0, sizeof(b));
 		for (n = 0; n < m->nr_cores; n++) {
 			memset(c, 0, sizeof(c));
 			snprintf(c, sizeof(c) - 1, " %d", m->cores[n]);
@@ -293,14 +294,14 @@ int mscp_add_src_path(struct mscp *m, const char *src_path)
 
 	s = malloc(sizeof(*s));
 	if (!s) {
-		priv_set_errv("failed to allocate memory: %s", strerrno());
+		priv_set_errv("malloc: %s", strerrno());
 		return -1;
 	}
 
 	memset(s, 0, sizeof(*s));
 	s->path = strdup(src_path);
 	if (!s->path) {
-		priv_set_errv("failed to allocate memory: %s", strerrno());
+		priv_set_errv("malloc: %s", strerrno());
 		free(s);
 		return -1;
 	}
@@ -386,7 +387,7 @@ void *mscp_scan_thread(void *arg)
 		dst_sftp = NULL;
 		break;
 	default:
-		priv_set_errv("invalid copy direction: %d", m->direction);
+		pr_err("invalid copy direction: %d", m->direction);
 		goto err_out;
 	}
 
@@ -414,13 +415,13 @@ void *mscp_scan_thread(void *arg)
 	list_for_each_entry(s, &m->src_list, list) {
 		memset(&pglob, 0, sizeof(pglob));
 		if (mscp_glob(s->path, GLOB_NOCHECK, &pglob, src_sftp) < 0) {
-			priv_set_errv("mscp_glob: %s", strerrno());
+			pr_err("mscp_glob: %s", strerrno());
 			goto err_out;
 		}
 
 		for (n = 0; n < pglob.gl_pathc; n++) {
 			if (mscp_stat(pglob.gl_pathv[n], &ss, src_sftp) < 0) {
-				priv_set_errv("stat: %s %s", s->path, strerrno());
+				pr_err("stat: %s %s", s->path, strerrno());
 				goto err_out;
 			}
 
@@ -456,7 +457,7 @@ int mscp_scan(struct mscp *m)
 {
 	int ret = pthread_create(&m->tid_scan, NULL, mscp_scan_thread, m);
 	if (ret < 0) {
-		priv_set_errv("pthread_create_error: %d", ret);
+		priv_set_err("pthread_create: %d", ret);
 		m->tid_scan = 0;
 		mscp_stop(m);
 		return -1;
@@ -507,7 +508,7 @@ static struct mscp_thread *mscp_copy_thread_spawn(struct mscp *m, int id)
 
 	ret = pthread_create(&t->tid, NULL, mscp_copy_thread, t);
 	if (ret < 0) {
-		priv_set_errv("pthread_create error: %d", ret);
+		priv_set_errv("pthread_create: %d", ret);
 		free(t);
 		return NULL;
 	}
@@ -521,18 +522,15 @@ int mscp_start(struct mscp *m)
 	int n, ret = 0;
 
 	if ((n = chunk_pool_size(&m->cp)) < m->opts->nr_threads) {
-		pr_notice("we have only %d chunk(s). "
-			  "set number of connections to %d",
-			  n, n);
+		pr_notice("we have %d chunk(s), set number of connections to %d", n, n);
 		m->opts->nr_threads = n;
 	}
 
 	for (n = 0; n < m->opts->nr_threads; n++) {
 		t = mscp_copy_thread_spawn(m, n);
-		if (!t) {
-			pr_err("failed to spawn copy thread");
+		if (!t)
 			break;
-		}
+
 		RWLOCK_WRITE_ACQUIRE(&m->thread_rwlock);
 		list_add_tail(&t->list, &m->thread_list);
 		RWLOCK_RELEASE();
@@ -556,7 +554,7 @@ int mscp_join(struct mscp *m)
 	list_for_each_entry(t, &m->thread_list, list) {
 		pthread_join(t->tid, NULL);
 		done += t->done;
-		if (t->ret < 0)
+		if (t->ret != 0)
 			ret = t->ret;
 		if (t->sftp) {
 			ssh_sftp_close(t->sftp);
@@ -615,11 +613,17 @@ void *mscp_copy_thread(void *arg)
 	struct chunk *c;
 	bool nomore;
 
+	/* when error occurs, each thread prints error messages
+	 * immediately with pr_* functions. */
+
 	pthread_cleanup_push(mscp_copy_thread_cleanup, t);
 
 	if (t->cpu > -1) {
-		if (set_thread_affinity(pthread_self(), t->cpu) < 0)
+		if (set_thread_affinity(pthread_self(), t->cpu) < 0) {
+			pr_err("set_thread_affinity: %s", priv_get_err());
 			goto err_out;
+		}
+		pr_notice("thread[%d]: pin to cpu core %d", t->id, t->cpu);
 	}
 
 	if (sem_wait(m->sem) < 0) {
@@ -630,7 +634,7 @@ void *mscp_copy_thread(void *arg)
 	if (!(nomore = chunk_pool_is_empty(&m->cp))) {
 		if (m->opts->interval > 0)
 			wait_for_interval(m->opts->interval);
-		pr_notice("thread:%d connecting to %s", t->id, m->remote);
+		pr_notice("thread[%d]: connecting to %s", t->id, m->remote);
 		t->sftp = ssh_init_sftp_session(m->remote, m->ssh_opts);
 	}
 
@@ -640,12 +644,12 @@ void *mscp_copy_thread(void *arg)
 	}
 
 	if (nomore) {
-		pr_notice("thread:%d no more connections needed", t->id);
+		pr_notice("thread[%d]: no more connections needed", t->id);
 		goto out;
 	}
 
 	if (!t->sftp) {
-		pr_err("thread:%d: %s", t->id, priv_get_err());
+		pr_err("thread[%d]: %s", t->id, priv_get_err());
 		goto err_out;
 	}
 
@@ -659,7 +663,8 @@ void *mscp_copy_thread(void *arg)
 		dst_sftp = NULL;
 		break;
 	default:
-		return NULL; /* not reached */
+		assert(false);
+		goto err_out; /* not reached */
 	}
 
 	while (1) {
@@ -680,9 +685,11 @@ void *mscp_copy_thread(void *arg)
 
 	pthread_cleanup_pop(1);
 
-	if (t->ret < 0)
-		pr_err("thread:%d copy failed: %s 0x%010lx-0x%010lx", t->id, c->p->path,
-		       c->off, c->off + c->len);
+	if (t->ret < 0) {
+		pr_err("thread[%d]: copy failed: %s -> %s, 0x%010lx-0x%010lx, %s", t->id,
+		       c->p->path, c->p->dst_path, c->off, c->off + c->len,
+		       priv_get_err());
+	}
 
 	return NULL;
 
diff --git a/src/path.c b/src/path.c
index f2e759c..a340126 100644
--- a/src/path.c
+++ b/src/path.c
@@ -104,7 +104,7 @@ static char *resolve_dst_path(const char *src_file_path, struct path_resolve_arg
 	strncpy(copy, a->src_path, PATH_MAX);
 	prefix = dirname(copy);
 	if (!prefix) {
-		priv_set_errv("dirname: %s", strerrno());
+		pr_err("dirname: %s", strerrno());
 		return NULL;
 	}
 
@@ -162,7 +162,7 @@ static struct chunk *alloc_chunk(struct path *p)
 	struct chunk *c;
 
 	if (!(c = malloc(sizeof(*c)))) {
-		priv_set_errv("malloc %s", strerrno());
+		pr_err("malloc %s", strerrno());
 		return NULL;
 	}
 	memset(c, 0, sizeof(*c));
@@ -231,8 +231,10 @@ static int append_path(sftp_session sftp, const char *path, struct stat st,
 	memset(p, 0, sizeof(*p));
 	INIT_LIST_HEAD(&p->list);
 	p->path = strndup(path, PATH_MAX);
-	if (!p->path)
+	if (!p->path) {
+		pr_err("strndup: %s", strerrno());
 		goto free_out;
+	}
 	p->size = st.st_size;
 	p->mode = st.st_mode;
 	p->state = FILE_STATE_INIT;
@@ -306,7 +308,11 @@ static int walk_path_recursive(sftp_session sftp, const char *path,
 
 		walk_path_recursive(sftp, next_path, path_list, a);
 		/* do not stop even when walk_path_recursive returns
-		 * -1 due to an unreadable file. go to a next file. */
+		 * -1 due to an unreadable file. go to a next
+		 * file. Thus, do not pass error messages via
+		 * priv_set_err() under walk_path_recursive.  Print
+		 * the error with pr_err immediately.
+		 */
 	}
 
 	mscp_closedir(d);
@@ -320,16 +326,6 @@ int walk_src_path(sftp_session src_sftp, const char *src_path,
 	return walk_path_recursive(src_sftp, src_path, path_list, a);
 }
 
-void path_dump(struct list_head *path_list)
-{
-	struct path *p;
-
-	list_for_each_entry(p, path_list, list) {
-		printf("src: %s %lu-byte\n", p->path, p->size);
-		printf("dst: %s\n", p->dst_path);
-	}
-}
-
 /* based on
  * https://stackoverflow.com/questions/2336242/recursive-mkdir-system-call-on-unix */
 static int touch_dst_path(struct path *p, sftp_session sftp)
@@ -390,7 +386,6 @@ static int prepare_dst_path(struct path *p, sftp_session dst_sftp)
 	if (p->state == FILE_STATE_INIT) {
 		if (touch_dst_path(p, dst_sftp) < 0) {
 			ret = -1;
-			pr_err("failed to prepare dst path: %s", priv_get_err());
 			goto out;
 		}
 		p->state = FILE_STATE_OPENED;
@@ -608,11 +603,11 @@ int copy_chunk(struct chunk *c, sftp_session src_sftp, sftp_session dst_sftp,
 
 		/* sync stat */
 		if (mscp_stat(c->p->path, &st, src_sftp) < 0) {
-			pr_err("mscp_stat: %s: %s", c->p->path, strerrno());
+			priv_set_errv("mscp_stat: %s: %s", c->p->path, strerrno());
 			return -1;
 		}
 		if (mscp_setstat(c->p->dst_path, &st, preserve_ts, dst_sftp) < 0) {
-			pr_err("mscp_setstat: %s: %s", c->p->path, strerrno());
+			priv_set_errv("mscp_setstat: %s: %s", c->p->path, strerrno());
 			return -1;
 		}
 		pr_info("copy done: %s", c->p->path);
diff --git a/src/ssh.c b/src/ssh.c
index 7f003b4..d9a6a2d 100644
--- a/src/ssh.c
+++ b/src/ssh.c
@@ -186,6 +186,7 @@ static ssh_session ssh_init_session(const char *sshdst, struct mscp_ssh_opts *op
 	}
 
 	if (!opts->no_hostkey_check && ssh_verify_known_hosts(ssh) != 0) {
+		priv_set_errv("ssh_veriy_known_hosts failed");
 		goto disconnect_out;
 	}
 
@@ -203,9 +204,8 @@ sftp_session ssh_init_sftp_session(const char *sshdst, struct mscp_ssh_opts *opt
 	sftp_session sftp;
 	ssh_session ssh = ssh_init_session(sshdst, opts);
 
-	if (!ssh) {
+	if (!ssh)
 		return NULL;
-	}
 
 	sftp = sftp_new(ssh);
 	if (!sftp) {
@@ -304,7 +304,7 @@ static int ssh_verify_known_hosts(ssh_session session)
 
 		break;
 	case SSH_KNOWN_HOSTS_ERROR:
-		priv_set_errv("known hosts error: %s", ssh_get_error(session));
+		fprintf(stderr, "known hosts error: %s", ssh_get_error(session));
 		ssh_clean_pubkey_hash(&hash);
 		return -1;
 	}
diff --git a/src/strerrno.c b/src/strerrno.c
index f610c8f..68d35bc 100644
--- a/src/strerrno.c
+++ b/src/strerrno.c
@@ -18,20 +18,15 @@ const char *strerrno(void)
 }
 
 #define PRIV_ERR_BUFSIZ (1 << 12)
-static char priv_err_buf[PRIV_ERR_BUFSIZ], internal[PRIV_ERR_BUFSIZ];
+__thread char priv_err_buf[PRIV_ERR_BUFSIZ], internal[PRIV_ERR_BUFSIZ];
 
 void priv_set_err(const char *fmt, ...)
 {
 	va_list va;
-
-	/* arguments may contains priv_err_buf. Thus, we build the
-	 * string in a internal buffer, and then copy it to
-	 * priv_err_buf. */
 	memset(internal, 0, sizeof(internal));
 	va_start(va, fmt);
 	vsnprintf(internal, sizeof(internal), fmt, va);
 	va_end(va);
-
 	snprintf(priv_err_buf, sizeof(priv_err_buf), "%s", internal);
 }
 
diff --git a/src/strerrno.h b/src/strerrno.h
index 2abdd41..c7d935d 100644
--- a/src/strerrno.h
+++ b/src/strerrno.h
@@ -11,9 +11,12 @@
 const char *strerrno(void);
 
 /**
- * priv_set_err() sets an error message into a private buffer. This
- * error message set by priv_set_err() can be accessed via
- * priv_get_err(). priv_*_err functions are not thread safe.
+ * priv_set_err() sets an error message into a thread-local private
+ * buffer. This error message can be accessed via priv_get_err().
+ *
+ * The top-level function in a thread should print errors using
+ * priv_get_err(), while lower-level functions should set error
+ * messages using priv_set_err().
  */
 void priv_set_err(const char *fmt, ...);
 
@@ -26,7 +29,7 @@ void priv_set_err(const char *fmt, ...);
 		     ##__VA_ARGS__)
 
 /**
- * priv_get_err() gets the error message sotred in a private buffer.
+ * priv_get_err() gets the error message sotred in the thread-local private buffer.
  */
 const char *priv_get_err();