I ran into a few minor nuisances with sed's -i mode, which are mostly
compatible with gnu sed, but I reckon we should address.
The problem is sed works by writing the output to a second file in the
same directory as the original and after completion renaming the file
to the original. This has two disadvantages:
1) The inode number changes, resulting in loss of carefully crafted
hardlinks.
2) We require to have write permission in the same directory as the
original file, even if we don't want to have a backup file.
Diff below tries to solve this by doing the following.
Copy the file to the backup location (/tmp/sedXXXXXXXXXX if no
extension) and use the backup as the infile and the original as the
outfile.
Furthermore I changed the lstat to fstat, so we can edit symlinks (gsed
supports symlinks by replacing the symlink by a new real file, which is
also fun), and I extended the warning messages in process to show the
backup file if we crash during operation, so people will always know
where to recover the file in case of disaster.
Because process also error(FATAL, ...)s during process and we always
have a backup file I don't think the warning in sed.1 is worth keeping.
The only downside to this new approach (that I can think of) is that we
now temporarily have a file that is in an inconsistent state, but that's
not much different to writing a file with any other editor.
$ echo test > /usr/obj/test && dd if=/dev/zero of=/usr/obj/bloat
/usr/obj: write failed, file system is full
dd: /usr/obj/bloat: No space left on device
614113+0 records in
614112+0 records out
314425344 bytes transferred in 2.206 secs (142470196 bytes/sec)
$ ./obj/sed -i -f /tmp/test.sed /usr/obj/test
/usr/obj: write failed, file system is full
sed: /usr/obj/test: No space left on device (backup at /tmp/sedgRPSLImG9N)
$ cat /tmp/test.sed
s/test/aaa<repeat to infinity>/
$ cat /tmp/sedgRPSLImG9N
test
$ ls -i /tmp/test
104 /tmp/test
$ sed -i s/test/foo/ /tmp/test
$ ls -i /tmp/test
104 /tmp/test
$ doas touch /etc/test
$ doas chown martijn /etc/test
$ echo test > /etc/test
$ sed -i s/test/foo/ /etc/test
$ cat /etc/test
foo
The diff does change quite a few mechanics, so some scrutiny is welcome.
martijn@
Index: main.c
===================================================================
RCS file: /cvs/src/usr.bin/sed/main.c,v
retrieving revision 1.39
diff -u -p -r1.39 main.c
--- main.c 6 Dec 2018 20:16:04 -0000 1.39
+++ main.c 7 Dec 2018 07:31:54 -0000
@@ -35,6 +35,7 @@
#include <sys/types.h>
#include <sys/ioctl.h>
+#include <sys/param.h>
#include <sys/stat.h>
#include <ctype.h>
@@ -94,13 +95,13 @@ static int rval; /* Exit status */
*/
const char *fname; /* File name. */
const char *outfname; /* Output file name */
-static char oldfname[PATH_MAX]; /* Old file name (for in-place editing)
*/
-static char tmpfname[PATH_MAX]; /* Temporary file name (for in-place
editing) */
+char oldfname[PATH_MAX]; /* Old file name (for in-place editing) */
char *inplace; /* Inplace edit file extension */
u_long linenum;
static void add_compunit(enum e_cut, char *);
static void add_file(char *);
+int copy(FILE *, FILE *);
static int next_files_have_lines(void);
int termwidth;
@@ -310,26 +311,46 @@ again:
return (NULL);
}
+int
+copy(FILE *src, FILE *dst)
+{
+ unsigned char buf[MAXBSIZE];
+ size_t r, w, tw;
+
+ while(1) {
+ if ((r = fread(buf, sizeof(*buf), sizeof(buf), src)) == 0) {
+ if (feof(src)) {
+ if (fflush(dst) == EOF)
+ return 0;
+ return 1;
+ }
+ if (errno != EINTR)
+ return 0;
+ continue;
+ }
+ tw = 0;
+ while(tw < r) {
+ w = fwrite(buf + tw, sizeof(*buf), r - tw, dst);
+ if (w == 0) {
+ if (errno != EINTR)
+ return 0;
+ continue;
+ }
+ tw += w;
+ }
+ }
+}
+
void
finish_file(void)
{
if (infile != NULL) {
fclose(infile);
- if (*oldfname != '\0') {
- if (rename(fname, oldfname) != 0) {
- warning("rename()");
- unlink(tmpfname);
- exit(1);
- }
+ if (inplace != NULL) {
+ if (*oldfname == '\0')
+ unlink(oldfname);
*oldfname = '\0';
}
- if (*tmpfname != '\0') {
- if (outfile != NULL && outfile != stdout)
- fclose(outfile);
- outfile = NULL;
- rename(tmpfname, fname);
- *tmpfname = '\0';
- }
outfname = NULL;
}
}
@@ -341,7 +362,7 @@ finish_file(void)
int
mf_fgets(SPACE *sp, enum e_spflag spflag)
{
- struct stat sb;
+ struct stat sb, isb;
size_t len;
char *p;
int c, fd;
@@ -369,10 +390,18 @@ mf_fgets(SPACE *sp, enum e_spflag spflag
outfname = "stdout";
break;
}
+ infile = fopen(fname, inplace == NULL ? "r" : "r+");
+ if (infile == NULL) {
+ warning("%s", strerror(errno));
+ rval = 1;
+ continue;
+ }
if (inplace != NULL) {
- if (lstat(fname, &sb) != 0)
+ /* inplace reads from copy */
+ outfile = infile;
+ if (fstat(fileno(outfile), &sb) != 0)
error(FATAL, "%s: %s", fname,
- strerror(errno ? errno : EIO));
+ strerror(errno));
if (!S_ISREG(sb.st_mode))
error(FATAL, "%s: %s %s", fname,
"in-place editing only",
@@ -382,32 +411,48 @@ mf_fgets(SPACE *sp, enum e_spflag spflag
sizeof(oldfname));
len = strlcat(oldfname, inplace,
sizeof(oldfname));
- if (len > sizeof(oldfname))
- error(FATAL, "%s: name too long",
fname);
- }
- len = snprintf(tmpfname, sizeof(tmpfname),
"%s/sedXXXXXXXXXX",
- dirname(fname));
- if (len >= sizeof(tmpfname))
- error(FATAL, "%s: name too long", fname);
- if ((fd = mkstemp(tmpfname)) == -1)
- error(FATAL, "%s: %s", fname, strerror(errno));
- if ((outfile = fdopen(fd, "w")) == NULL) {
- unlink(tmpfname);
- error(FATAL, "%s", fname);
+ if (len >= sizeof(oldfname))
+ error(FATAL, "backup %s: name too long",
+ fname);
+ (void)unlink(oldfname);
+ if ((infile = fopen(oldfname, "w+")) == NULL)
+ error(FATAL, "can't create backup "
+ "file: %s", oldfname);
+ if (fstat(fileno(infile), &isb) != 0)
+ error(FATAL, "%s: %s", oldfname,
+ strerror(errno));
+ if (!S_ISREG(isb.st_mode))
+ error(FATAL, "backup file %s not a "
+ "regular file and can't be "
+ "replaced", oldfname);
+ fchown(fileno(infile), sb.st_uid, sb.st_gid);
+ fchmod(fileno(infile), sb.st_mode & ALLPERMS);
+ } else {
+ (void)strlcpy(oldfname, "/tmp/sedXXXXXXXXXX",
+ sizeof(oldfname));
+ if ((fd = mkstemp(oldfname)) == -1)
+ error(FATAL, "%s: %s", fname,
+ strerror(errno));
+ if ((infile = fdopen(fd, "a+")) == NULL) {
+ unlink(oldfname);
+ error(FATAL, "%s", fname);
+ }
}
- fchown(fileno(outfile), sb.st_uid, sb.st_gid);
- fchmod(fileno(outfile), sb.st_mode & ALLPERMS);
- outfname = tmpfname;
+ outfname = fname;
linenum = 0;
resetstate();
+ if (!copy(outfile, infile)) {
+ unlink(oldfname);
+ error(FATAL, "%s: failed to create backup copy "
+ "to %s: %s", fname, oldfname,
+ strerror(errno));
+ }
+ ftruncate(fileno(outfile), 0);
+ rewind(outfile);
+ rewind(infile);
} else {
outfile = stdout;
outfname = "stdout";
- }
- if ((infile = fopen(fname, "r")) == NULL) {
- warning("%s", strerror(errno));
- rval = 1;
- continue;
}
}
Index: process.c
===================================================================
RCS file: /cvs/src/usr.bin/sed/process.c,v
retrieving revision 1.34
diff -u -p -r1.34 process.c
--- process.c 14 Nov 2018 10:59:33 -0000 1.34
+++ process.c 7 Dec 2018 07:31:54 -0000
@@ -75,6 +75,7 @@ static int sdone; /* If any substitutes
/* Iov structure for 'w' commands. */
static regex_t *defpreg;
size_t maxnsub;
+extern char oldfname[PATH_MAX];
regmatch_t *match;
#define OUT() do {\
@@ -473,7 +474,11 @@ flush_appends(void)
break;
}
if (ferror(outfile))
- error(FATAL, "%s: %s", outfname, strerror(errno ? errno : EIO));
+ error(FATAL, "%s: %s%s%s%s", outfname,
+ strerror(errno ? errno : EIO),
+ *oldfname == '\0' ? "" : " (backup at ",
+ *oldfname == '\0' ? "" : oldfname,
+ *oldfname == '\0' ? "" : ")");
appendx = sdone = 0;
}
@@ -513,7 +518,11 @@ lputs(char *s)
(void)fputc('$', outfile);
(void)fputc('\n', outfile);
if (ferror(outfile))
- error(FATAL, "%s: %s", outfname, strerror(errno ? errno : EIO));
+ error(FATAL, "%s: %s%s%s%s", outfname,
+ strerror(errno ? errno : EIO),
+ *oldfname == '\0' ? "" : " (backup at ",
+ *oldfname == '\0' ? "" : oldfname,
+ *oldfname == '\0' ? "" : ")");
}
static inline int
Index: sed.1
===================================================================
RCS file: /cvs/src/usr.bin/sed/sed.1,v
retrieving revision 1.57
diff -u -p -r1.57 sed.1
--- sed.1 14 Nov 2018 10:59:33 -0000 1.57
+++ sed.1 7 Dec 2018 07:31:54 -0000
@@ -102,10 +102,6 @@ Edit files in place, saving backups with
If a zero length
.Ar extension
is given, no backup will be saved.
-It is not recommended to give a zero length
-.Ar extension
-when in place editing files, as it risks corruption or partial content
-in situations where disk space is exhausted, etc.
In
.Fl i
mode, the hold space, line numbers, and ranges are reset between files.
