Re: rsync exit code and error cleanup
Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.05.07 12:16:26 +0200:
> 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?
ok benno
> --
> :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 - 1.15
> +++ client.c 7 May 2021 08:29:10 -
> @@ -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 - 1.36
> +++ extern.h 7 May 2021 08:22:32 -
> @@ -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_WIREPROTO12
> +#define ERR_IPC 14 /* catchall for any kind of syscall
> error */
> +#define ERR_TERMIMATED 16
> +#define ERR_WAITPID 21
> +#define ERR_NOMEM22
> +
> +/*
> * 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 - 1.17
> +++ fargs.c 7 May 2021 08:19:29 -
> @@ -17,6 +17,7 @@
> #include
>
> #include
> +#include
> #include
> #include
> #include
> @@ -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.c31 Mar 2021 19:45:16 - 1.53
> +++ main.c7 May 2021 08:30:18 -
> @@ -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");
> +
rsync exit code and error cleanup
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.c8 May 2019 20:00:25 - 1.15
+++ client.c7 May 2021 08:29:10 -
@@ -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.h31 Mar 2021 19:45:16 - 1.36
+++ extern.h7 May 2021 08:22:32 -
@@ -42,6 +42,19 @@
#defineCSUM_LENGTH_PHASE2 (16)
/*
+ * Rsync error codes.
+ */
+#define ERR_SYNTAX 1
+#define ERR_PROTOCOL 2
+#define ERR_SOCK_IO10
+#define ERR_FILE_IO11
+#define ERR_WIREPROTO 12
+#define ERR_IPC14 /* catchall for any kind of syscall
error */
+#define ERR_TERMIMATED 16
+#define ERR_WAITPID21
+#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 - 1.17
+++ fargs.c 7 May 2021 08:19:29 -
@@ -17,6 +17,7 @@
#include
#include
+#include
#include
#include
#include
@@ -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 - 1.53
+++ main.c 7 May 2021 08:30:18 -
@@ -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");
