Date: Mon, 6 May 2002 03:31:29 +1000 From: Joshua Goodall <joshua@roughtrade.net> To: Ian Dowse <iedowse@FreeBSD.org> Cc: cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sbin/restore tape.c Message-ID: <20020505173129.GE33863@roughtrade.net> In-Reply-To: <200205021739.g42HdKr70755@freefall.freebsd.org> References: <200205021739.g42HdKr70755@freefall.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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" <mad1@tapil.com>
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020505173129.GE33863>