From owner-cvs-all Sun May 5 10:31:52 2002 Delivered-To: cvs-all@freebsd.org Received: from yello.shallow.net (yello.shallow.net [203.18.243.120]) by hub.freebsd.org (Postfix) with ESMTP id B026237B407; Sun, 5 May 2002 10:31:36 -0700 (PDT) Received: by yello.shallow.net (Postfix, from userid 1001) id B18DC2A73; Mon, 6 May 2002 03:31:29 +1000 (EST) Date: Mon, 6 May 2002 03:31:29 +1000 From: Joshua Goodall To: Ian Dowse Cc: cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sbin/restore tape.c Message-ID: <20020505173129.GE33863@roughtrade.net> References: <200205021739.g42HdKr70755@freefall.freebsd.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <200205021739.g42HdKr70755@freefall.freebsd.org> User-Agent: Mutt/1.3.28i Sender: owner-cvs-all@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On Thu, May 02, 2002 at 10:39:20AM -0700, Ian Dowse wrote: > iedowse 2002/05/02 10:39:20 PDT > > Modified files: > sbin/restore tape.c > Log: > Set the permissions on restored symbolic links. > > PR: bin/37665 > Submitted by: "Michael C. Adler" Wish I'd spotted this earlier. * It's inconsistent with the rest of extractfile (which doesn't perror on failure of attribute setting anywhere else) * It separates the uid/gid variables declaration and initialisation from flags & mode for no good reason (particularly when the whole thing could benefit since the cases for FIFO, BLK and REG do their own lookup of curfile.dip->di_uid & ->di_gid) and introduces an (arguably) ugly, unnecessary block inside the case. * It's already covered in the (admittedly wider-reaching) PR kern/29355 An alternative is below for your consideration. Cheers, Joshua Index: tape.c =================================================================== RCS file: /cvs/src/sbin/restore/tape.c,v retrieving revision 1.32 diff -u -r1.32 tape.c --- tape.c 2 May 2002 17:39:19 -0000 1.32 +++ tape.c 5 May 2002 17:23:17 -0000 @@ -529,6 +529,8 @@ extractfile(char *name) { int flags; + uid_t uid; + gid_t gid; mode_t mode; struct timeval timep[2]; struct entry *ep; @@ -539,6 +541,8 @@ timep[0].tv_usec = curfile.dip->di_atimensec / 1000; timep[1].tv_sec = curfile.dip->di_mtime; timep[1].tv_usec = curfile.dip->di_mtimensec / 1000; + uid = curfile.dip->di_uid; + gid = curfile.dip->di_gid; mode = curfile.dip->di_mode; flags = curfile.dip->di_flags; switch (mode & IFMT) { @@ -565,14 +569,6 @@ return (genliteraldir(name, curfile.ino)); case IFLNK: - { - uid_t uid; - gid_t gid; - int ret; - - uid = curfile.dip->di_uid; - gid = curfile.dip->di_gid; - lnkbuf[0] = '\0'; pathlen = 0; getfile(xtrlnkfile, xtrlnkskip); @@ -581,17 +577,13 @@ "%s: zero length symbolic link (ignored)\n", name); return (GOOD); } - ret = linkit(lnkbuf, name, SYMLINK); - if (ret == GOOD) { - if (lchown(name, uid, gid)) - perror(name); - if (lchmod(name, mode)) - perror(name); - lutimes(name, timep); - } - /* symbolic link doesn't have any flags */ - return (ret); - } + if (linkit(lnkbuf, name, SYMLINK) == GOOD) { + (void) lchown(name, uid, gid); + (void) lchmod(name, mode); + (void) lutimes(name, timep); + return (GOOD); + } + return (FAIL); case IFIFO: vprintf(stdout, "extract fifo %s\n", name); @@ -607,9 +599,9 @@ skipfile(); return (FAIL); } - (void) chown(name, curfile.dip->di_uid, curfile.dip->di_gid); + (void) chown(name, uid, gid); (void) chmod(name, mode); - utimes(name, timep); + (void) utimes(name, timep); (void) chflags(name, flags); skipfile(); return (GOOD); @@ -629,9 +621,9 @@ skipfile(); return (FAIL); } - (void) chown(name, curfile.dip->di_uid, curfile.dip->di_gid); + (void) chown(name, uid, gid); (void) chmod(name, mode); - utimes(name, timep); + (void) utimes(name, timep); (void) chflags(name, flags); skipfile(); return (GOOD); @@ -651,7 +643,7 @@ skipfile(); return (FAIL); } - (void) fchown(ofile, curfile.dip->di_uid, curfile.dip->di_gid); + (void) fchown(ofile, uid, gid); (void) fchmod(ofile, mode); getfile(xtrfile, xtrskip); (void) close(ofile); To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe cvs-all" in the body of the message