Re: more rsync cleanup
Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.05.06 17:59:32 +0200:
> As noticed by benno@ the blk.blks buffer is leaked in some cases.
> Fix those and cleanup up the pre_* functions a bit more.
> I increased the diff context a bit to make the diff easier to read.
reads ok
>
> --
> :wq Claudio
>
> Index: uploader.c
> ===
> RCS file: /cvs/src/usr.bin/rsync/uploader.c,v
> retrieving revision 1.25
> diff -u -p -U6 -r1.25 uploader.c
> --- uploader.c6 May 2021 07:35:22 - 1.25
> +++ uploader.c6 May 2021 15:34:36 -
> @@ -191,22 +191,24 @@ pre_link(struct upload *p, struct sess *
>* to overwriting with a symbolic link.
>* If it's a non-directory, we just overwrite it.
>*/
>
> assert(p->rootfd != -1);
> rc = fstatat(p->rootfd, f->path, , AT_SYMLINK_NOFOLLOW);
> +
> + if (rc == -1 && errno != ENOENT) {
> + ERR("%s: fstatat", f->path);
> + return -1;
> + }
> if (rc != -1 && !S_ISLNK(st.st_mode)) {
> if (S_ISDIR(st.st_mode) &&
> unlinkat(p->rootfd, f->path, AT_REMOVEDIR) == -1) {
> ERR("%s: unlinkat", f->path);
> return -1;
> }
> rc = -1;
> - } else if (rc == -1 && errno != ENOENT) {
> - ERR("%s: fstatat", f->path);
> - return -1;
> }
>
> /*
>* If the symbolic link already exists, then make sure that it
>* points to the correct place.
>*/
> @@ -294,22 +296,23 @@ pre_dev(struct upload *p, struct sess *s
>* If it replaces a directory, remove the directory first.
>*/
>
> assert(p->rootfd != -1);
> rc = fstatat(p->rootfd, f->path, , AT_SYMLINK_NOFOLLOW);
>
> + if (rc == -1 && errno != ENOENT) {
> + ERR("%s: fstatat", f->path);
> + return -1;
> + }
> if (rc != -1 && !(S_ISBLK(st.st_mode) || S_ISCHR(st.st_mode))) {
> if (S_ISDIR(st.st_mode) &&
> unlinkat(p->rootfd, f->path, AT_REMOVEDIR) == -1) {
> ERR("%s: unlinkat", f->path);
> return -1;
> }
> rc = -1;
> - } else if (rc == -1 && errno != ENOENT) {
> - ERR("%s: fstatat", f->path);
> - return -1;
> }
>
> /* Make sure existing device is of the correct type. */
>
> if (rc != -1) {
> if ((f->st.mode & (S_IFCHR|S_IFBLK)) !=
> @@ -382,22 +385,23 @@ pre_fifo(struct upload *p, struct sess *
>* mark it from replacement.
>*/
>
> assert(p->rootfd != -1);
> rc = fstatat(p->rootfd, f->path, , AT_SYMLINK_NOFOLLOW);
>
> + if (rc == -1 && errno != ENOENT) {
> + ERR("%s: fstatat", f->path);
> + return -1;
> + }
> if (rc != -1 && !S_ISFIFO(st.st_mode)) {
> if (S_ISDIR(st.st_mode) &&
> unlinkat(p->rootfd, f->path, AT_REMOVEDIR) == -1) {
> ERR("%s: unlinkat", f->path);
> return -1;
> }
> rc = -1;
> - } else if (rc == -1 && errno != ENOENT) {
> - ERR("%s: fstatat", f->path);
> - return -1;
> }
>
> if (rc == -1) {
> newfifo = 1;
> if (mktemplate(, f->path, sess->opts->recursive) == -1) {
> ERRX1("mktemplate");
> @@ -458,22 +462,23 @@ pre_sock(struct upload *p, struct sess *
>* mark it from replacement.
>*/
>
> assert(p->rootfd != -1);
> rc = fstatat(p->rootfd, f->path, , AT_SYMLINK_NOFOLLOW);
>
> + if (rc == -1 && errno != ENOENT) {
> + ERR("%s: fstatat", f->path);
> + return -1;
> + }
> if (rc != -1 && !S_ISSOCK(st.st_mode)) {
> if (S_ISDIR(st.st_mode) &&
> unlinkat(p->rootfd, f->path, AT_REMOVEDIR) == -1) {
> ERR("%s: unlinkat", f->path);
> return -1;
> }
> rc = -1;
> - } else if (rc == -1 && errno != ENOENT) {
> - ERR("%s: fstatat", f->path);
> - return -1;
> }
>
> if (rc == -1) {
> newsock = 1;
> if (mktemplate(, f->path, sess->opts->recursive) == -1) {
> ERRX1("mktemplate");
> @@ -530,13 +535,14 @@ pre_dir(const struct upload *p, struct s
> assert(p->rootfd != -1);
> rc = fstatat(p->rootfd, f->path, , AT_SYMLINK_NOFOLLOW);
>
> if (rc == -1 && errno != ENOENT) {
> ERR("%s: fstatat", f->path);
> return -1;
> - } else if (rc != -1 && !S_ISDIR(st.st_mode)) {
> + }
> + if (rc != -1 && !S_ISDIR(st.st_mode)) {
> ERRX("%s: not a directory", f->path);
> return -1;
> } else if (rc != -1) {
> /*
>
more rsync cleanup
As noticed by benno@ the blk.blks buffer is leaked in some cases.
Fix those and cleanup up the pre_* functions a bit more.
I increased the diff context a bit to make the diff easier to read.
--
:wq Claudio
Index: uploader.c
===
RCS file: /cvs/src/usr.bin/rsync/uploader.c,v
retrieving revision 1.25
diff -u -p -U6 -r1.25 uploader.c
--- uploader.c 6 May 2021 07:35:22 - 1.25
+++ uploader.c 6 May 2021 15:34:36 -
@@ -191,22 +191,24 @@ pre_link(struct upload *p, struct sess *
* to overwriting with a symbolic link.
* If it's a non-directory, we just overwrite it.
*/
assert(p->rootfd != -1);
rc = fstatat(p->rootfd, f->path, , AT_SYMLINK_NOFOLLOW);
+
+ if (rc == -1 && errno != ENOENT) {
+ ERR("%s: fstatat", f->path);
+ return -1;
+ }
if (rc != -1 && !S_ISLNK(st.st_mode)) {
if (S_ISDIR(st.st_mode) &&
unlinkat(p->rootfd, f->path, AT_REMOVEDIR) == -1) {
ERR("%s: unlinkat", f->path);
return -1;
}
rc = -1;
- } else if (rc == -1 && errno != ENOENT) {
- ERR("%s: fstatat", f->path);
- return -1;
}
/*
* If the symbolic link already exists, then make sure that it
* points to the correct place.
*/
@@ -294,22 +296,23 @@ pre_dev(struct upload *p, struct sess *s
* If it replaces a directory, remove the directory first.
*/
assert(p->rootfd != -1);
rc = fstatat(p->rootfd, f->path, , AT_SYMLINK_NOFOLLOW);
+ if (rc == -1 && errno != ENOENT) {
+ ERR("%s: fstatat", f->path);
+ return -1;
+ }
if (rc != -1 && !(S_ISBLK(st.st_mode) || S_ISCHR(st.st_mode))) {
if (S_ISDIR(st.st_mode) &&
unlinkat(p->rootfd, f->path, AT_REMOVEDIR) == -1) {
ERR("%s: unlinkat", f->path);
return -1;
}
rc = -1;
- } else if (rc == -1 && errno != ENOENT) {
- ERR("%s: fstatat", f->path);
- return -1;
}
/* Make sure existing device is of the correct type. */
if (rc != -1) {
if ((f->st.mode & (S_IFCHR|S_IFBLK)) !=
@@ -382,22 +385,23 @@ pre_fifo(struct upload *p, struct sess *
* mark it from replacement.
*/
assert(p->rootfd != -1);
rc = fstatat(p->rootfd, f->path, , AT_SYMLINK_NOFOLLOW);
+ if (rc == -1 && errno != ENOENT) {
+ ERR("%s: fstatat", f->path);
+ return -1;
+ }
if (rc != -1 && !S_ISFIFO(st.st_mode)) {
if (S_ISDIR(st.st_mode) &&
unlinkat(p->rootfd, f->path, AT_REMOVEDIR) == -1) {
ERR("%s: unlinkat", f->path);
return -1;
}
rc = -1;
- } else if (rc == -1 && errno != ENOENT) {
- ERR("%s: fstatat", f->path);
- return -1;
}
if (rc == -1) {
newfifo = 1;
if (mktemplate(, f->path, sess->opts->recursive) == -1) {
ERRX1("mktemplate");
@@ -458,22 +462,23 @@ pre_sock(struct upload *p, struct sess *
* mark it from replacement.
*/
assert(p->rootfd != -1);
rc = fstatat(p->rootfd, f->path, , AT_SYMLINK_NOFOLLOW);
+ if (rc == -1 && errno != ENOENT) {
+ ERR("%s: fstatat", f->path);
+ return -1;
+ }
if (rc != -1 && !S_ISSOCK(st.st_mode)) {
if (S_ISDIR(st.st_mode) &&
unlinkat(p->rootfd, f->path, AT_REMOVEDIR) == -1) {
ERR("%s: unlinkat", f->path);
return -1;
}
rc = -1;
- } else if (rc == -1 && errno != ENOENT) {
- ERR("%s: fstatat", f->path);
- return -1;
}
if (rc == -1) {
newsock = 1;
if (mktemplate(, f->path, sess->opts->recursive) == -1) {
ERRX1("mktemplate");
@@ -530,13 +535,14 @@ pre_dir(const struct upload *p, struct s
assert(p->rootfd != -1);
rc = fstatat(p->rootfd, f->path, , AT_SYMLINK_NOFOLLOW);
if (rc == -1 && errno != ENOENT) {
ERR("%s: fstatat", f->path);
return -1;
- } else if (rc != -1 && !S_ISDIR(st.st_mode)) {
+ }
+ if (rc != -1 && !S_ISDIR(st.st_mode)) {
ERRX("%s: not a directory", f->path);
return -1;
} else if (rc != -1) {
/*
* FIXME: we should fchmod the permissions here as well,
* as we may locally have shut down writing into the
@@ -682,19 +688,21
