On 08/14/18 14:07, Todd C. Miller wrote:
> On Tue, 14 Aug 2018 10:43:30 +0200, Martijn van Duren wrote:
>
>> The diff below fixes this. Note that I took special care to make a
>> distinction between in place and normal for the 'q' command.
>> When running normally the files are concatenated, so we should quit
>> immediately. When running in in place mode I reckon the script
>> should be treated as if it were to run for every file individually,
>> since the line numbers are also reset in between files.
>
> I'm not convinced this is the right approach. If there is a quit
> command, the script should quit without further processing.
>
> We could do something like this instead which matches what GNU sed
> does more closely. Since the -i flag is a GNU invention I think
> it is worthwhile trying to behave similarly.
If we want to mimic what gnu sed does we also need to reset the
hold space:
$ printf '1\n2\n3\n' | tee /tmp/test > /tmp/test1
$ sed -i 'x' /tmp/test{,1}
$ cat /tmp/test
1
2
$ cat /tmp/test1
3
1
2
$ printf '1\n2\n3\n' | tee /tmp/test > /tmp/test1
$ gsed -i 'x' /tmp/test{,1}
$ cat /tmp/test
1
2
$ cat /tmp/test1
1
2
Do note the difference with using it without -i:
$ printf '1\n2\n3\n' | tee /tmp/test > /tmp/test1
$ gsed 'x' /tmp/test{,1}
1
2
3
1
2
So gnu is somewhat contradictory here. On the one hand it treats
every file as a new script, based on the hold-space, but the 'q'
command exits the editor all together.
So the question is how close do we want to follow gnu sed and
how consistent do we want to be in our definitions.
- Do we treat "-i" as a single script passing, then I'd expect
sed to quit, similar to what gnu sed does and keep our hold space
over multiple files (unlike what gnu sed does, but we currently do)
- Do we treat "-i" as multiple executions of the same script
(similar to "for file in *; do sed -i ...; done"), then I'd expect
sed to reset the hold space, similar to what gnu sed does and
not quit the editor itself, but merely the execution on the
current file.
- Do we treat "-i" as quirk by quirk compatible with gnu sed, then
the diff also fixes the hold space case.
martijn@
Index: extern.h
===================================================================
RCS file: /cvs/src/usr.bin/sed/extern.h,v
retrieving revision 1.13
diff -u -p -r1.13 extern.h
--- extern.h 1 Aug 2017 18:05:53 -0000 1.13
+++ extern.h 14 Aug 2018 13:12:35 -0000
@@ -44,6 +44,7 @@ extern int Eflag, aflag, eflag, nflag;
extern int pledge_wpath, pledge_rpath;
extern const char *fname, *outfname;
extern FILE *infile, *outfile;
+extern SPACE HS;
void cfclose(struct s_command *, struct s_command *);
void compile(void);
@@ -53,6 +54,7 @@ __dead void error(int, const char *, ...
void warning(const char *, ...);
int mf_fgets(SPACE *, enum e_spflag);
int lastline(void);
+void finish_file(void);
void process(void);
void resetranges(void);
char *strregerror(int, regex_t *);
Index: main.c
===================================================================
RCS file: /cvs/src/usr.bin/sed/main.c,v
retrieving revision 1.37
diff -u -p -r1.37 main.c
--- main.c 11 Jul 2018 06:57:18 -0000 1.37
+++ main.c 14 Aug 2018 13:12:35 -0000
@@ -310,6 +310,30 @@ again:
return (NULL);
}
+void
+finish_file(void)
+{
+ if (infile != NULL) {
+ fclose(infile);
+ if (*oldfname != '\0') {
+ if (rename(fname, oldfname) != 0) {
+ warning("rename()");
+ unlink(tmpfname);
+ exit(1);
+ }
+ *oldfname = '\0';
+ }
+ if (*tmpfname != '\0') {
+ if (outfile != NULL && outfile != stdout)
+ fclose(outfile);
+ outfile = NULL;
+ rename(tmpfname, fname);
+ *tmpfname = '\0';
+ }
+ outfname = NULL;
+ }
+}
+
/*
* Like fgets, but go through the list of files chaining them together.
* Set len to the length of the line.
@@ -347,25 +371,7 @@ mf_fgets(SPACE *sp, enum e_spflag spflag
sp->len = 0;
return (0);
}
- if (infile != NULL) {
- fclose(infile);
- if (*oldfname != '\0') {
- if (rename(fname, oldfname) != 0) {
- warning("rename()");
- unlink(tmpfname);
- exit(1);
- }
- *oldfname = '\0';
- }
- if (*tmpfname != '\0') {
- if (outfile != NULL && outfile != stdout)
- fclose(outfile);
- outfile = NULL;
- rename(tmpfname, fname);
- *tmpfname = '\0';
- }
- outfname = NULL;
- }
+ finish_file();
if (firstfile == 0)
files = files->next;
else
@@ -376,6 +382,8 @@ mf_fgets(SPACE *sp, enum e_spflag spflag
}
fname = files->fname;
if (inplace != NULL) {
+ free(HS.back);
+ memset(&HS, 0, sizeof(HS));
if (lstat(fname, &sb) != 0)
error(FATAL, "%s: %s", fname,
strerror(errno ? errno : EIO));
Index: process.c
===================================================================
RCS file: /cvs/src/usr.bin/sed/process.c,v
retrieving revision 1.33
diff -u -p -r1.33 process.c
--- process.c 13 Dec 2017 16:06:34 -0000 1.33
+++ process.c 14 Aug 2018 13:12:35 -0000
@@ -50,7 +50,8 @@
#include "defs.h"
#include "extern.h"
-static SPACE HS, PS, SS;
+static SPACE PS, SS;
+SPACE HS;
#define pd PS.deleted
#define ps PS.space
#define psl PS.len
@@ -196,6 +197,7 @@ redirect:
if (!nflag && !pd)
OUT();
flush_appends();
+ finish_file();
exit(0);
case 'r':
if (appendx >= appendnum) {
Index: sed.1
===================================================================
RCS file: /cvs/src/usr.bin/sed/sed.1,v
retrieving revision 1.56
diff -u -p -r1.56 sed.1
--- sed.1 11 Jul 2018 06:47:38 -0000 1.56
+++ sed.1 14 Aug 2018 13:12:35 -0000
@@ -106,6 +106,7 @@ It is not recommended to give a zero len
.Ar extension
when in place editing files, as it risks corruption or partial content
in situations where disk space is exhausted, etc.
+Editing multiple files will reset the hold space in between files.
.It Fl r
An alias for
.Fl E ,
