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.c 6 May 2021 07:35:22 -0000 1.25 > +++ uploader.c 6 May 2021 15:34:36 -0000 > @@ -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 into the > @@ -682,19 +688,21 @@ pre_file(const struct upload *p, int *fi > > if (fstat(*filefd, st) == -1) { > ERR("%s: fstat", f->path); > close(*filefd); > *filefd = -1; > return -1; > - } else if (!S_ISREG(st->st_mode)) { > + } > + if (!S_ISREG(st->st_mode)) { > ERRX("%s: not regular", f->path); > close(*filefd); > *filefd = -1; > return -1; > } > > + /* quick check if file is the same */ > if (st->st_size == f->st.size && > st->st_mtime == f->st.mtime) { > LOG3("%s: skipping: up to date", f->path); > if (!rsync_set_metadata(sess, 0, *filefd, f, f->path)) { > ERRX1("rsync_set_metadata"); > close(*filefd); > @@ -910,24 +918,26 @@ rsync_uploader(struct upload *u, int *fi > } > > if ((mbuf = malloc(blk.len)) == NULL) { > ERR("malloc"); > close(*fileinfd); > *fileinfd = -1; > + free(blk.blks); > return -1; > } > > offs = 0; > i = 0; > do { > msz = pread(*fileinfd, mbuf, blk.len, offs); > if ((size_t)msz != blk.len && (size_t)msz != blk.rem) { > ERR("pread"); > close(*fileinfd); > *fileinfd = -1; > free(mbuf); > + free(blk.blks); > return -1; > } > init_blk(&blk.blks[i], &blk, offs, i, mbuf, sess); > offs += blk.len; > LOG3( > "i=%ld, offs=%lld, msz=%ld, blk.len=%lu, > blk.rem=%lu", > @@ -964,12 +974,13 @@ rsync_uploader(struct upload *u, int *fi > (sizeof(int32_t) + /* short checksum */ > blk.csum); /* long checksum */ > > if (u->bufsz > u->bufmax) { > if ((bufp = realloc(u->buf, u->bufsz)) == NULL) { > ERR("realloc"); > + free(blk.blks); > return -1; > } > u->buf = bufp; > u->bufmax = u->bufsz; > } > >