From owner-freebsd-hackers Mon Jul 29 6:23:52 2002 Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.FreeBSD.org (mx1.FreeBSD.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 50B6D37B400; Mon, 29 Jul 2002 06:23:37 -0700 (PDT) Received: from chiark.greenend.org.uk (chiark.greenend.org.uk [212.135.138.206]) by mx1.FreeBSD.org (Postfix) with ESMTP id 0725743E4A; Mon, 29 Jul 2002 06:23:34 -0700 (PDT) (envelope-from fanf@chiark.greenend.org.uk) Received: from fanf by chiark.greenend.org.uk with local (Exim 3.12 #1) id 17ZAUk-000883-00 (Debian); Mon, 29 Jul 2002 14:23:30 +0100 Date: Mon, 29 Jul 2002 14:23:30 +0100 From: Tony Finch To: Cyrille Lefevre Cc: freebsd-hackers@freebsd.org, freebsd-ports@freebsd.org Subject: Re: sed -i has difficulty with read-only files Message-ID: <20020729142330.B6864@chiark.greenend.org.uk> References: <20020726165021.C7551@chiark.greenend.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5i In-Reply-To: <20020726165021.C7551@chiark.greenend.org.uk>; from dot@dotat.at on Fri, Jul 26, 2002 at 04:50:21PM +0100 Sender: owner-freebsd-hackers@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG Updated patches. I still prefer the one that's 40 lines shorter :-) Tony. -- f.a.n.finch http://dotat.at/ SOUTHEAST ICELAND: SOUTHWEST BECOMING VARIABLE 4. FAIR. GOOD. --- main.c Mon Jul 29 13:16:44 2002 +++ main.c.two Mon Jul 29 13:08:49 2002 @@ -413,9 +413,7 @@ char **filename; { struct stat orig; - int input, output; char backup[MAXPATHLEN]; - char *buffer; if (lstat(*filename, &orig) == -1) err(1, "lstat"); @@ -425,36 +423,36 @@ } if (*inplace == '\0') { - char template[] = "/tmp/sed.XXXXXXXXXX"; - - output = mkstemp(template); - if (output == -1) - err(1, "mkstemp"); - strlcpy(backup, template, MAXPATHLEN); + /* + * This is a bit of a hack: we use mkstemp() to avoid the + * mktemp() link-time warning, although mktemp() would fit in + * this context much better. We're only interested in getting + * a name for use in the rename(); there aren't any security + * issues here that don't already exist in relation to the + * original file and its directory. + */ + int fd; + strlcpy(backup, *filename, sizeof(backup)); + strlcat(backup, ".XXXXXXXXXX", sizeof(backup)); + fd = mkstemp(backup); + if (fd == -1) + errx(1, "could not create backup of %s", *filename); + else + close(fd); } else { - strlcpy(backup, *filename, MAXPATHLEN); - strlcat(backup, inplace, MAXPATHLEN); - output = open(backup, O_WRONLY | O_CREAT | O_TRUNC); - if (output == -1) - err(1, "open(%s)", backup); + strlcpy(backup, *filename, sizeof(backup)); + strlcat(backup, inplace, sizeof(backup)); } - input = open(*filename, O_RDONLY); - if (input == -1) - err(1, "open(%s)", *filename); - if (fchmod(output, orig.st_mode & ~S_IFMT) == -1) - err(1, "chmod"); - buffer = (char *)mmap(0, orig.st_size, PROT_READ, MAP_SHARED, input, 0); - if (buffer == MAP_FAILED) - err(1, "mmap(%s)", *filename); - if (write(output, buffer, orig.st_size) == -1) - err(1, "write(%s)", backup); - if (munmap(buffer, orig.st_size) == -1) - err(1, "munmap(%s)", *filename); - close(input); - close(output); - freopen(*filename, "w", stdout); + if (rename(*filename, backup) == -1) + err(1, "rename(\"%s\", \"%s\")", *filename, backup); + if (freopen(*filename, "w", stdout) == NULL) + err(1, "open(\"%s\")", *filename); + if (fchmod(fileno(stdout), orig.st_mode) == -1) + err(1, "chmod(\"%s\")", *filename); *filename = strdup(backup); + if (*filename == NULL) + err(1, "malloc"); return 0; } --- main.c Mon Jul 29 13:16:44 2002 +++ main.c.one Mon Jul 29 13:15:03 2002 @@ -68,6 +68,11 @@ #include "extern.h" /* + * Maximum amount of data to mmap at a time when copying a file. + */ +#define MMAP_MAX (16*1024*1024) + +/* * Linked list of units (strings and files) to be compiled */ struct s_compunit { @@ -407,6 +412,8 @@ /* * Modify a pointer to a filename for inplace editing and reopen stdout + * + * We remove the files before opening them in case of permissions problems */ static int inplace_edit(filename) @@ -416,6 +423,7 @@ int input, output; char backup[MAXPATHLEN]; char *buffer; + off_t off, size; if (lstat(*filename, &orig) == -1) err(1, "lstat"); @@ -424,37 +432,57 @@ return -1; } + /* create and open the backup file */ if (*inplace == '\0') { - char template[] = "/tmp/sed.XXXXXXXXXX"; + const char *tmp; - output = mkstemp(template); + tmp = getenv("TMPDIR"); + if (tmp == NULL) + tmp = "/tmp"; + strlcpy(backup, tmp, sizeof(backup)); + strlcat(backup, "/sed.XXXXXXXXXX", sizeof(backup)); + output = mkstemp(backup); if (output == -1) err(1, "mkstemp"); - strlcpy(backup, template, MAXPATHLEN); + if (fchmod(output, orig.st_mode & ~S_IFMT) == -1) + err(1, "chmod(\"%s\")", backup); } else { - strlcpy(backup, *filename, MAXPATHLEN); - strlcat(backup, inplace, MAXPATHLEN); - output = open(backup, O_WRONLY | O_CREAT | O_TRUNC); + strlcpy(backup, *filename, sizeof(backup)); + strlcat(backup, inplace, sizeof(backup)); + if (unlink(backup) == -1 && errno != ENOENT) + err(1, "unlink(\"%s\")", backup); + output = open(backup, O_WRONLY | O_CREAT | O_TRUNC, orig.st_mode); if (output == -1) - err(1, "open(%s)", backup); + err(1, "open(\"%s\")", backup); } - + /* copy the original file to the backup */ input = open(*filename, O_RDONLY); if (input == -1) - err(1, "open(%s)", *filename); - if (fchmod(output, orig.st_mode & ~S_IFMT) == -1) - err(1, "chmod"); - buffer = (char *)mmap(0, orig.st_size, PROT_READ, MAP_SHARED, input, 0); - if (buffer == MAP_FAILED) - err(1, "mmap(%s)", *filename); - if (write(output, buffer, orig.st_size) == -1) - err(1, "write(%s)", backup); - if (munmap(buffer, orig.st_size) == -1) - err(1, "munmap(%s)", *filename); + err(1, "open(\"%s\")", *filename); + for (off = 0; off < orig.st_size; off += MMAP_MAX) { + size = orig.st_size - off; + if (size > MMAP_MAX) + size = MMAP_MAX; + buffer = mmap(NULL, size, PROT_READ, MAP_SHARED, input, off); + if (buffer == MAP_FAILED) + err(1, "mmap(\"%s\")", *filename); + if (write(output, buffer, size) == -1) + err(1, "write(\"%s\")", backup); + if (munmap(buffer, size) == -1) + err(1, "munmap(\"%s\")", *filename); + } close(input); close(output); - freopen(*filename, "w", stdout); + /* create the replacement for the original file */ + if (unlink(*filename) == -1) + err(1, "unlink(\"%s\")", *filename); + if (freopen(*filename, "w", stdout) == NULL) + err(1, "open(\"%s\")", *filename); + if (fchmod(fileno(stdout), orig.st_mode) == -1) + err(1, "chmod(\"%s\")", *filename); *filename = strdup(backup); + if (*filename == NULL) + err(1, "malloc"); return 0; } To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-hackers" in the body of the message