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, &st, 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, &st, 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, &st, 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(&temp, 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, &st, 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(&temp, 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, &st, 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, &st, 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, &st, 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, &st, 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(&temp, 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, &st, 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(&temp, 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, &st, 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 int