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);

Reply via email to