Currently our rsync does not follow the exit codes from rsync. Also the
error handling is complex because ERR() and ERRX() are not terminating the
process.
This diff tries to start cleaning up the mess a bit. Introduce some exit
codes to use and apply them in places where it is obvious.
The rsync server process should normally communicate errors back via the
rsync socket but there are some errors where this will most probably fail
as well. A good example are memory failures.
I used ERR_IPC as a kind of catchall for any system error that pops up
(fork, socketpair but also pledge, unveil).
The goal is to reduce the amount of ERR(), ERRX() and ERRX1() from rsync.
This should simplify the code base.
I also synced the mkpath() call with the one from mkdir and handle the
error now outside of the call. Again I think the result is cleaner.
OK?
--
:wq Claudio
Index: client.c
===================================================================
RCS file: /cvs/src/usr.bin/rsync/client.c,v
retrieving revision 1.15
diff -u -p -r1.15 client.c
--- client.c 8 May 2019 20:00:25 -0000 1.15
+++ client.c 7 May 2021 08:29:10 -0000
@@ -43,7 +43,7 @@ rsync_client(const struct opts *opts, in
if (pledge("stdio unix rpath wpath cpath dpath fattr chown getpw
unveil",
NULL) == -1)
- err(1, "pledge");
+ err(ERR_IPC, "pledge");
memset(&sess, 0, sizeof(struct sess));
sess.opts = opts;
Index: extern.h
===================================================================
RCS file: /cvs/src/usr.bin/rsync/extern.h,v
retrieving revision 1.36
diff -u -p -r1.36 extern.h
--- extern.h 31 Mar 2021 19:45:16 -0000 1.36
+++ extern.h 7 May 2021 08:22:32 -0000
@@ -42,6 +42,19 @@
#define CSUM_LENGTH_PHASE2 (16)
/*
+ * Rsync error codes.
+ */
+#define ERR_SYNTAX 1
+#define ERR_PROTOCOL 2
+#define ERR_SOCK_IO 10
+#define ERR_FILE_IO 11
+#define ERR_WIREPROTO 12
+#define ERR_IPC 14 /* catchall for any kind of syscall
error */
+#define ERR_TERMIMATED 16
+#define ERR_WAITPID 21
+#define ERR_NOMEM 22
+
+/*
* Use this for --timeout.
* All poll events will use it and catch time-outs.
*/
Index: fargs.c
===================================================================
RCS file: /cvs/src/usr.bin/rsync/fargs.c,v
retrieving revision 1.17
diff -u -p -r1.17 fargs.c
--- fargs.c 8 May 2019 20:00:25 -0000 1.17
+++ fargs.c 7 May 2021 08:19:29 -0000
@@ -17,6 +17,7 @@
#include <sys/stat.h>
#include <assert.h>
+#include <err.h>
#include <stdint.h>
#include <stdlib.h>
#include <string.h>
@@ -51,7 +52,7 @@ fargs_cmdline(struct sess *sess, const s
if (sess->opts->ssh_prog) {
ap = strdup(sess->opts->ssh_prog);
if (ap == NULL)
- goto out;
+ err(ERR_NOMEM, NULL);
while ((arg = strsep(&ap, " \t")) != NULL) {
if (arg[0] == '\0') {
@@ -127,8 +128,4 @@ fargs_cmdline(struct sess *sess, const s
addargs(&args, "%s", f->sink);
return args.list;
-out:
- freeargs(&args);
- ERR("calloc");
- return NULL;
}
Index: main.c
===================================================================
RCS file: /cvs/src/usr.bin/rsync/main.c,v
retrieving revision 1.53
diff -u -p -r1.53 main.c
--- main.c 31 Mar 2021 19:45:16 -0000 1.53
+++ main.c 7 May 2021 08:30:18 -0000
@@ -83,18 +83,18 @@ fargs_parse(size_t argc, char *argv[], s
/* Allocations. */
if ((f = calloc(1, sizeof(struct fargs))) == NULL)
- err(1, "calloc");
+ err(ERR_NOMEM, NULL);
f->sourcesz = argc - 1;
if ((f->sources = calloc(f->sourcesz, sizeof(char *))) == NULL)
- err(1, "calloc");
+ err(ERR_NOMEM, NULL);
for (i = 0; i < argc - 1; i++)
if ((f->sources[i] = strdup(argv[i])) == NULL)
- err(1, "strdup");
+ err(ERR_NOMEM, NULL);
if ((f->sink = strdup(argv[i])) == NULL)
- err(1, "strdup");
+ err(ERR_NOMEM, NULL);
/*
* Test files for its locality.
@@ -109,15 +109,16 @@ fargs_parse(size_t argc, char *argv[], s
if (fargs_is_remote(f->sink)) {
f->mode = FARGS_SENDER;
if ((f->host = strdup(f->sink)) == NULL)
- err(1, "strdup");
+ err(ERR_NOMEM, NULL);
}
if (fargs_is_remote(f->sources[0])) {
if (f->host != NULL)
- errx(1, "both source and destination cannot be remote
files");
+ errx(ERR_SYNTAX, "both source and destination "
+ "cannot be remote files");
f->mode = FARGS_RECEIVER;
if ((f->host = strdup(f->sources[0])) == NULL)
- err(1, "strdup");
+ err(ERR_NOMEM, NULL);
}
if (f->host != NULL) {
@@ -127,7 +128,8 @@ fargs_parse(size_t argc, char *argv[], s
len = strlen(f->host) - 8 + 1;
memmove(f->host, f->host + 8, len);
if ((cp = strchr(f->host, '/')) == NULL)
- errx(1, "rsync protocol requires a module
name");
+ errx(ERR_SYNTAX,
+ "rsync protocol requires a module name");
*cp++ = '\0';
f->module = cp;
if ((cp = strchr(f->module, '/')) != NULL)
@@ -152,9 +154,9 @@ fargs_parse(size_t argc, char *argv[], s
}
}
if ((len = strlen(f->host)) == 0)
- errx(1, "empty remote host");
+ errx(ERR_SYNTAX, "empty remote host");
if (f->remote && strlen(f->module) == 0)
- errx(1, "empty remote module");
+ errx(ERR_SYNTAX, "empty remote module");
}
/* Make sure we have the same "hostspec" for all files. */
@@ -164,7 +166,7 @@ fargs_parse(size_t argc, char *argv[], s
for (i = 0; i < f->sourcesz; i++) {
if (!fargs_is_remote(f->sources[i]))
continue;
- errx(1,
+ errx(ERR_SYNTAX,
"remote file in list of local sources: %s",
f->sources[i]);
}
@@ -174,20 +176,20 @@ fargs_parse(size_t argc, char *argv[], s
!fargs_is_daemon(f->sources[i]))
continue;
if (fargs_is_daemon(f->sources[i]))
- errx(1, "remote daemon in list of "
- "remote sources: %s",
- f->sources[i]);
- errx(1, "local file in list of remote sources:
%s",
- f->sources[i]);
+ errx(ERR_SYNTAX,
+ "remote daemon in list of remote "
+ "sources: %s", f->sources[i]);
+ errx(ERR_SYNTAX, "local file in list of "
+ "remote sources: %s", f->sources[i]);
}
} else {
if (f->mode != FARGS_RECEIVER)
- errx(1, "sender mode for remote "
+ errx(ERR_SYNTAX, "sender mode for remote "
"daemon receivers not yet supported");
for (i = 0; i < f->sourcesz; i++) {
if (fargs_is_daemon(f->sources[i]))
continue;
- errx(1, "non-remote daemon file "
+ errx(ERR_SYNTAX, "non-remote daemon file "
"in list of remote daemon sources: "
"%s", f->sources[i]);
}
@@ -233,7 +235,7 @@ fargs_parse(size_t argc, char *argv[], s
*ccp = '\0';
if (strncmp(cp, f->host, len) ||
(cp[len] != '/' && cp[len] != '\0'))
- errx(1, "different remote host: %s",
+ errx(ERR_SYNTAX, "different remote host: %s",
f->sources[i]);
memmove(f->sources[i],
f->sources[i] + len + 8 + 1,
@@ -246,7 +248,7 @@ fargs_parse(size_t argc, char *argv[], s
/* host::path */
if (strncmp(cp, f->host, len) ||
(cp[len] != ':' && cp[len] != '\0'))
- errx(1, "different remote host: %s",
+ errx(ERR_SYNTAX, "different remote host: %s",
f->sources[i]);
memmove(f->sources[i], f->sources[i] + len + 2,
j - len - 1);
@@ -257,7 +259,7 @@ fargs_parse(size_t argc, char *argv[], s
/* host:path */
if (strncmp(cp, f->host, len) ||
(cp[len] != ':' && cp[len] != '\0'))
- errx(1, "different remote host: %s",
+ errx(ERR_SYNTAX, "different remote host: %s",
f->sources[i]);
memmove(f->sources[i],
f->sources[i] + len + 1, j - len);
@@ -318,7 +320,7 @@ main(int argc, char *argv[])
if (pledge("stdio unix rpath wpath cpath dpath inet fattr chown dns
getpw proc exec unveil",
NULL) == -1)
- err(1, "pledge");
+ err(ERR_IPC, "pledge");
memset(&opts, 0, sizeof(struct opts));
@@ -391,7 +393,8 @@ main(int argc, char *argv[])
case 5:
poll_timeout = strtonum(optarg, 0, 60*60, &errstr);
if (errstr != NULL)
- errx(1, "timeout is %s: %s", errstr, optarg);
+ errx(ERR_SYNTAX, "timeout is %s: %s",
+ errstr, optarg);
break;
case 6:
opts.no_motd = 1;
@@ -461,43 +464,36 @@ main(int argc, char *argv[])
if (pledge("stdio unix rpath wpath cpath dpath fattr chown getpw proc
exec unveil",
NULL) == -1)
- err(1, "pledge");
+ err(ERR_IPC, "pledge");
/* Create a bidirectional socket and start our child. */
if (socketpair(AF_UNIX, SOCK_STREAM | SOCK_NONBLOCK, 0, fds) == -1)
- err(1, "socketpair");
+ err(ERR_IPC, "socketpair");
switch ((child = fork())) {
case -1:
- err(1, "fork");
+ err(ERR_IPC, "fork");
case 0:
close(fds[0]);
if (pledge("stdio exec", NULL) == -1)
- err(1, "pledge");
+ err(ERR_IPC, "pledge");
memset(&sess, 0, sizeof(struct sess));
sess.opts = &opts;
- if ((args = fargs_cmdline(&sess, fargs, NULL)) == NULL) {
- ERRX1("fargs_cmdline");
- _exit(1);
- }
+ args = fargs_cmdline(&sess, fargs, NULL);
for (i = 0; args[i] != NULL; i++)
LOG2("exec[%d] = %s", i, args[i]);
/* Make sure the child's stdin is from the sender. */
- if (dup2(fds[1], STDIN_FILENO) == -1) {
- ERR("dup2");
- _exit(1);
- }
- if (dup2(fds[1], STDOUT_FILENO) == -1) {
- ERR("dup2");
- _exit(1);
- }
+ if (dup2(fds[1], STDIN_FILENO) == -1)
+ err(ERR_IPC, "dup2");
+ if (dup2(fds[1], STDOUT_FILENO) == -1)
+ err(ERR_IPC, "dup2");
execvp(args[0], args);
- _exit(1);
+ _exit(ERR_IPC);
/* NOTREACHED */
default:
close(fds[1]);
@@ -511,7 +507,7 @@ main(int argc, char *argv[])
close(fds[0]);
if (waitpid(child, &st, 0) == -1)
- err(1, "waitpid");
+ err(ERR_WAITPID, "waitpid");
/*
* If we don't already have an error (rc == 0), then inherit the
@@ -519,10 +515,14 @@ main(int argc, char *argv[])
* If it hasn't exited, it overrides our return value.
*/
- if (WIFEXITED(st) && rc == 0)
- rc = WEXITSTATUS(st);
- else if (!WIFEXITED(st))
- rc = 1;
+ if (rc == 0) {
+ if (WIFEXITED(st))
+ rc = WEXITSTATUS(st);
+ else if (WIFSIGNALED(st))
+ rc = ERR_TERMIMATED;
+ else
+ rc = ERR_WAITPID;
+ }
exit(rc);
usage:
@@ -532,5 +532,5 @@ usage:
"[--rsync-path=program]\n\t[--timeout=seconds] [--version] "
"source ... directory\n",
getprogname());
- exit(1);
+ exit(ERR_SYNTAX);
}
Index: misc.c
===================================================================
RCS file: /cvs/src/usr.bin/rsync/misc.c,v
retrieving revision 1.2
diff -u -p -r1.2 misc.c
--- misc.c 22 Mar 2021 11:14:42 -0000 1.2
+++ misc.c 7 May 2021 08:19:17 -0000
@@ -45,7 +45,7 @@ addargs(arglist *args, const char *fmt,
r = vasprintf(&cp, fmt, ap);
va_end(ap);
if (r == -1)
- err(1, "addargs: argument too long");
+ err(ERR_NOMEM, "addargs: argument too long");
nalloc = args->nalloc;
if (args->list == NULL) {
@@ -54,9 +54,10 @@ addargs(arglist *args, const char *fmt,
} else if (args->num+2 >= nalloc)
nalloc *= 2;
- args->list = recallocarray(args->list, args->nalloc, nalloc,
sizeof(char *));
+ args->list = recallocarray(args->list, args->nalloc, nalloc,
+ sizeof(char *));
if (!args->list)
- err(1, "malloc");
+ err(ERR_NOMEM, NULL);
args->nalloc = nalloc;
args->list[args->num++] = cp;
args->list[args->num] = NULL;
Index: mkpath.c
===================================================================
RCS file: /cvs/src/usr.bin/rsync/mkpath.c,v
retrieving revision 1.4
diff -u -p -r1.4 mkpath.c
--- mkpath.c 8 May 2019 21:30:11 -0000 1.4
+++ mkpath.c 7 May 2021 08:19:55 -0000
@@ -46,28 +46,34 @@ mkpath(char *path)
{
struct stat sb;
char *slash;
- int done = 0;
+ int done;
slash = path;
- while (!done) {
+ for (;;) {
slash += strspn(slash, "/");
slash += strcspn(slash, "/");
done = (*slash == '\0');
*slash = '\0';
- if (stat(path, &sb)) {
- if (errno != ENOENT || (mkdir(path, 0777) &&
- errno != EEXIST)) {
- ERR("%s: stat", path);
+ if (mkdir(path, 0777) != 0) {
+ int mkdir_errno = errno;
+
+ if (stat(path, &sb) == -1) {
+ /* Not there; use mkdir()s errno */
+ errno = mkdir_errno;
+ return (-1);
+ }
+ if (!S_ISDIR(sb.st_mode)) {
+ /* Is there, but isn't a directory */
+ errno = ENOTDIR;
return (-1);
}
- } else if (!S_ISDIR(sb.st_mode)) {
- errno = ENOTDIR;
- ERR("%s: stat", path);
- return (-1);
}
+
+ if (done)
+ break;
*slash = '/';
}
Index: receiver.c
===================================================================
RCS file: /cvs/src/usr.bin/rsync/receiver.c,v
retrieving revision 1.26
diff -u -p -r1.26 receiver.c
--- receiver.c 6 May 2021 07:29:59 -0000 1.26
+++ receiver.c 7 May 2021 08:21:48 -0000
@@ -20,6 +20,7 @@
#include <sys/stat.h>
#include <assert.h>
+#include <err.h>
#include <errno.h>
#include <fcntl.h>
#include <inttypes.h>
@@ -180,15 +181,12 @@ rsync_receiver(struct sess *sess, int fd
struct upload *ul = NULL;
mode_t oumask;
- if (pledge("stdio unix rpath wpath cpath dpath fattr chown getpw
unveil", NULL) == -1) {
- ERR("pledge");
- goto out;
- }
+ if (pledge("stdio unix rpath wpath cpath dpath fattr chown getpw
unveil", NULL) == -1)
+ err(ERR_IPC, "pledge");
/* Client sends zero-length exclusions. */
- if (!sess->opts->server &&
- !io_write_int(sess, fdout, 0)) {
+ if (!sess->opts->server && !io_write_int(sess, fdout, 0)) {
ERRX1("io_write_int");
goto out;
}
@@ -240,14 +238,10 @@ rsync_receiver(struct sess *sess, int fd
*/
if (!sess->opts->dry_run) {
- if ((tofree = strdup(root)) == NULL) {
- ERR("strdup");
- goto out;
- } else if (mkpath(tofree) < 0) {
- ERRX1("%s: mkpath", root);
- free(tofree);
- goto out;
- }
+ if ((tofree = strdup(root)) == NULL)
+ err(ERR_NOMEM, NULL);
+ if (mkpath(tofree) < 0)
+ err(ERR_FILE_IO, "%s: mkpath", tofree);
free(tofree);
}
@@ -260,10 +254,8 @@ rsync_receiver(struct sess *sess, int fd
if (!sess->opts->dry_run) {
dfd = open(root, O_RDONLY | O_DIRECTORY, 0);
- if (dfd == -1) {
- ERR("%s: open", root);
- goto out;
- }
+ if (dfd == -1)
+ err(ERR_FILE_IO, "%s: open", root);
}
/*
@@ -285,13 +277,10 @@ rsync_receiver(struct sess *sess, int fd
* writing into other parts of the file-system.
*/
- if (unveil(root, "rwc") == -1) {
- ERR("%s: unveil", root);
- goto out;
- } else if (unveil(NULL, NULL) == -1) {
- ERR("%s: unveil", root);
- goto out;
- }
+ if (unveil(root, "rwc") == -1)
+ err(ERR_IPC, "%s: unveil", root);
+ if (unveil(NULL, NULL) == -1)
+ err(ERR_IPC, "unveil");
/* If we have a local set, go for the deletion. */
Index: server.c
===================================================================
RCS file: /cvs/src/usr.bin/rsync/server.c,v
retrieving revision 1.12
diff -u -p -r1.12 server.c
--- server.c 8 May 2019 21:30:11 -0000 1.12
+++ server.c 7 May 2021 08:29:14 -0000
@@ -57,7 +57,7 @@ rsync_server(const struct opts *opts, si
if (pledge("stdio unix rpath wpath cpath dpath fattr chown getpw
unveil",
NULL) == -1)
- err(1, "pledge");
+ err(ERR_IPC, "pledge");
memset(&sess, 0, sizeof(struct sess));
sess.opts = opts;
Index: socket.c
===================================================================
RCS file: /cvs/src/usr.bin/rsync/socket.c,v
retrieving revision 1.29
diff -u -p -r1.29 socket.c
--- socket.c 31 Mar 2021 19:45:16 -0000 1.29
+++ socket.c 7 May 2021 08:29:20 -0000
@@ -281,7 +281,7 @@ rsync_connect(const struct opts *opts, i
if (pledge("stdio unix rpath wpath cpath dpath inet fattr chown dns
getpw unveil",
NULL) == -1)
- err(1, "pledge");
+ err(ERR_IPC, "pledge");
memset(&sess, 0, sizeof(struct sess));
sess.opts = opts;
@@ -365,7 +365,7 @@ rsync_socket(const struct opts *opts, in
if (pledge("stdio unix rpath wpath cpath dpath fattr chown getpw
unveil",
NULL) == -1)
- err(1, "pledge");
+ err(ERR_IPC, "pledge");
memset(&sess, 0, sizeof(struct sess));
sess.lver = RSYNC_PROTOCOL;
@@ -374,10 +374,7 @@ rsync_socket(const struct opts *opts, in
assert(f->host != NULL);
assert(f->module != NULL);
- if ((args = fargs_cmdline(&sess, f, &skip)) == NULL) {
- ERRX1("fargs_cmdline");
- exit(1);
- }
+ args = fargs_cmdline(&sess, f, &skip);
/* Initiate with the rsyncd version and module request. */
