On Fri, Aug 7, 2015 at 12:24 AM, Martijn van Duren <martijn...@gmail.com> wrote: > Hello tech@, > > I was reading mv.c and found two minor things in fastcopy: > 1) fd leak on seldom reached code
I think this one is handled more cleanly by moving the "if (!blen)" block up before two open()s. That way if the malloc fails a zero length file won't be left behind. Diff below. > 2) At the end of the function utimes is called, while the rest of the code > calls the f* variant. Consistency fix. This has already been done as part of converting mv to preserve nanoseconds with futimens. You should update your src tree to -current by using the -A option with cvs up, ala "cvs -q up -APd". Philip Guenther Index: mv.c =================================================================== RCS file: /data/src/openbsd/src/bin/mv/mv.c,v retrieving revision 1.39 diff -u -p -r1.39 mv.c --- mv.c 3 May 2015 19:44:59 -0000 1.39 +++ mv.c 7 Aug 2015 18:23:57 -0000 @@ -260,6 +260,15 @@ fastcopy(char *from, char *to, struct st int nread, from_fd, to_fd; int badchown = 0, serrno = 0; + if (!blen) { + blen = sbp->st_blksize; + if ((bp = malloc(blen)) == NULL) { + warn(NULL); + blen = 0; + return (1); + } + } + if ((from_fd = open(from, O_RDONLY, 0)) < 0) { warn("%s", from); return (1); @@ -276,14 +285,6 @@ fastcopy(char *from, char *to, struct st } (void) fchmod(to_fd, sbp->st_mode & ~(S_ISUID|S_ISGID)); - if (!blen) { - blen = sbp->st_blksize; - if ((bp = malloc(blen)) == NULL) { - warn(NULL); - blen = 0; - return (1); - } - } while ((nread = read(from_fd, bp, blen)) > 0) if (write(to_fd, bp, nread) != nread) { warn("%s", to);