3b1b6ef fix: harden watching, decoding and writing after review · kvconf

M README.md +1 -1


@@ -21,7 +21,7 @@ output.DP-1.scale = 1.5
21	21		- Safe for real configs: atomic saves, symlinked dotfiles, editors that
22	22		save by rename, missing directories, broken files that keep the last good
23	23		config, and warnings that never print values (they may be secrets).
24		-	- Fast: about 540 MB/s to parse, no allocation per watched event.
	24	+	- Fast: about 800 MB/s to parse, a few allocations per reported change.
25	25
26	26		```sh
27	27		go get github.com/bnema/kvconf


@@ -21,7 +21,7 @@ output.DP-1.scale = 1.5
21		- Safe for real configs: atomic saves, symlinked dotfiles, editors that	21		- Safe for real configs: atomic saves, symlinked dotfiles, editors that
22		save by rename, missing directories, broken files that keep the last good	22		save by rename, missing directories, broken files that keep the last good
23		config, and warnings that never print values (they may be secrets).	23		config, and warnings that never print values (they may be secrets).
24	-	- Fast: about 540 MB/s to parse, no allocation per watched event.	24	+	- Fast: about 800 MB/s to parse, a few allocations per reported change.
25			25
26		```sh	26		```sh
27		go get github.com/bnema/kvconf	27		go get github.com/bnema/kvconf

M bench_test.go +3 -1


@@ -113,6 +113,8 @@ func BenchmarkSet(b *testing.B) {
113	113		values := map[string]*string{"output.DP-3.scale": ptr("2")}
114	114		b.ReportAllocs()
115	115		for b.Loop() {
116		-	Set(benchFile, values)
	116	+	if _, err := Set(benchFile, values); err != nil {
	117	+	b.Fatal(err)
	118	+	}
117	119		}
118	120		}

 	values := map[string]*string{"output.DP-3.scale": ptr("2")}
 	b.ReportAllocs()
 	for b.Loop() {
-		Set(benchFile, values)
+		if _, err := Set(benchFile, values); err != nil {
+			b.Fatal(err)
+		}
 	}
 }

M decode.go +20 -2


@@ -4,6 +4,7 @@ import (
4	4		"encoding"
5	5		"errors"
6	6		"fmt"
	7	+	"math"
7	8		"reflect"
8	9		"slices"
9	10		"strconv"

@@ -31,6 +32,13 @@ import (
31	32		// pointer implements encoding.TextUnmarshaler, such as [Path]. Named types
32	33		// work too: type Mode string with oneof is an enum.
33	34
	35	+	// ValueError is an error message that is safe to show in a Warning: it
	36	+	// does not contain the value. Return one from UnmarshalText to explain a
	37	+	// rejected value; any other error is reported as "invalid <type>".
	38	+	type ValueError string
	39	+
	40	+	func (e ValueError) Error() string { return string(e) }
	41	+
34	42		// Validator is implemented by a config that checks itself after decoding,
35	43		// for rules that span several keys. Its error always fails the load.
36	44		type Validator interface{ Validate() error }

@@ -55,6 +63,7 @@ type field struct {
55	63		index []int
56	64		typ reflect.Type
57	65		text bool // *typ implements encoding.TextUnmarshaler
	66	+	marshal bool // typ or *typ implements encoding.TextMarshaler
58	67		duration bool
59	68		off bool
60	69		nonempty bool

@@ -198,7 +207,8 @@ func (s *schema) add(t reflect.Type, prefix string, index []int) error {
198	207
199	208		// newField checks that t can be decoded and precomputes how.
200	209		func newField(where string, t reflect.Type, tag reflect.StructTag, opts []string) (*field, error) {
201		-	f := &field{typ: t, text: isText(t), duration: t == durationType, sep: tag.Get("sep")}
	210	+	f := &field{typ: t, text: isText(t), duration: t == durationType, sep: tag.Get("sep"),
	211	+	marshal: t.Implements(marshalType) \|\| reflect.PointerTo(t).Implements(marshalType)}
202	212		f.off = slices.Contains(opts, "off")
203	213		f.nonempty = slices.Contains(opts, "nonempty")
204	214		if f.sep == "" {

@@ -460,7 +470,12 @@ func (f *field) decode(dst reflect.Value, raw string) error {
460	470		if f.text {
461	471		p := reflect.New(f.typ)
462	472		if err := p.Interface().(encoding.TextUnmarshaler).UnmarshalText([]byte(raw)); err != nil {
463		-	return err
	473	+	// Third-party errors often quote the input: show only ours.
	474	+	var ve ValueError
	475	+	if errors.As(err, &ve) {
	476	+	return ve
	477	+	}
	478	+	return errors.New("invalid " + f.typ.String())
464	479		}
465	480		dst.Set(p.Elem())
466	481		return nil

@@ -537,6 +552,9 @@ func (f *field) plain() bool {
537	552		}
538	553
539	554		func (f *field) check(x float64) error {
	555	+	if x != x \|\| (f.hasMin \|\| f.hasMax) && math.IsInf(x, 0) {
	556	+	return errors.New("must be a finite number")
	557	+	}
540	558		if f.hasMin && x < f.min {
541	559		return errors.New("must be at least " + f.minText)
542	560		}

 	"encoding"
 	"errors"
 	"fmt"
+	"math"
 	"reflect"
 	"slices"
 	"strconv"
 // pointer implements encoding.TextUnmarshaler, such as [Path]. Named types
 // work too: type Mode string with oneof is an enum.
+// ValueError is an error message that is safe to show in a Warning: it
+// does not contain the value. Return one from UnmarshalText to explain a
+// rejected value; any other error is reported as "invalid <type>".
+type ValueError string
+func (e ValueError) Error() string { return string(e) }
 // Validator is implemented by a config that checks itself after decoding,
 // for rules that span several keys. Its error always fails the load.
 type Validator interface{ Validate() error }
 	index    []int
 	typ      reflect.Type
 	text     bool // *typ implements encoding.TextUnmarshaler
+	marshal  bool // typ or *typ implements encoding.TextMarshaler
 	duration bool
 	off      bool
 	nonempty bool
 // newField checks that t can be decoded and precomputes how.
 func newField(where string, t reflect.Type, tag reflect.StructTag, opts []string) (*field, error) {
-	f := &field{typ: t, text: isText(t), duration: t == durationType, sep: tag.Get("sep")}
+	f := &field{typ: t, text: isText(t), duration: t == durationType, sep: tag.Get("sep"),
+		marshal: t.Implements(marshalType) || reflect.PointerTo(t).Implements(marshalType)}
 	f.off = slices.Contains(opts, "off")
 	f.nonempty = slices.Contains(opts, "nonempty")
 	if f.sep == "" {
 	if f.text {
 		p := reflect.New(f.typ)
 		if err := p.Interface().(encoding.TextUnmarshaler).UnmarshalText([]byte(raw)); err != nil {
-			return err
+			// Third-party errors often quote the input: show only ours.
+			var ve ValueError
+			if errors.As(err, &ve) {
+				return ve
+			}
+			return errors.New("invalid " + f.typ.String())
 		}
 		dst.Set(p.Elem())
 		return nil
 }
 func (f *field) check(x float64) error {
+	if x != x || (f.hasMin || f.hasMax) && math.IsInf(x, 0) {
+		return errors.New("must be a finite number")
+	}
 	if f.hasMin && x < f.min {
 		return errors.New("must be at least " + f.minText)
 	}

M decode_test.go +38 -0


@@ -2,6 +2,7 @@ package kvconf
2	2
3	3		import (
4	4		"errors"
	5	+	"fmt"
5	6		"os"
6	7		"path/filepath"
7	8		"reflect"

@@ -330,6 +331,43 @@ func TestSchemaErrors(t *testing.T) {
330	331		}
331	332		}
332	333
	334	+	// leaky mimics a third-party type whose errors quote the input.
	335	+	type leaky string
	336	+
	337	+	func (l *leaky) UnmarshalText(b []byte) error { return fmt.Errorf("bad input %q", b) }
	338	+
	339	+	// explained returns a ValueError, which is shown.
	340	+	type explained string
	341	+
	342	+	func (e *explained) UnmarshalText([]byte) error { return ValueError("must be a colour") }
	343	+
	344	+	func TestDecodeNeverLeaksValues(t *testing.T) {
	345	+	type cfg struct {
	346	+	L leaky `kv:"l"`
	347	+	E explained `kv:"e"`
	348	+	Ratio float64 `kv:"ratio" min:"0" max:"1"`
	349	+	Any float64 `kv:"any"`
	350	+	}
	351	+	tests := []struct{ in, want string }{
	352	+	{"l = hunter2", "line 1: l: invalid kvconf.leaky"},
	353	+	{"e = hunter2", "line 1: e: must be a colour"},
	354	+	{"ratio = NaN", "line 1: ratio: must be a finite number"},
	355	+	{"ratio = +Inf", "line 1: ratio: must be a finite number"},
	356	+	{"any = NaN", "line 1: any: must be a finite number"},
	357	+	}
	358	+	for _, tt := range tests {
	359	+	t.Run(tt.in, func(t *testing.T) {
	360	+	var c cfg
	361	+	w, err := Unmarshal([]byte(tt.in+"\n"), &c, Options{})
	362	+	must(t, err)
	363	+	eq(t, warningStrings(w), []string{tt.want})
	364	+	if strings.Contains(tt.want, "hunter2") {
	365	+	t.Fatal("value leaked")
	366	+	}
	367	+	})
	368	+	}
	369	+	}
	370	+
333	371		func TestParseBool(t *testing.T) {
334	372		tests := []struct {
335	373		in string


@@ -2,6 +2,7 @@ package kvconf
2		2
3	import (	3		import (
4	"errors"	4		"errors"
		5	+	"fmt"
5	"os"	6		"os"
6	"path/filepath"	7		"path/filepath"
7	"reflect"	8		"reflect"

@@ -330,6 +331,43 @@ func TestSchemaErrors(t *testing.T) {
330	}	331		}
331	}	332		}
332		333
		334	+	// leaky mimics a third-party type whose errors quote the input.
		335	+	type leaky string
		336	+
		337	+	func (l *leaky) UnmarshalText(b []byte) error { return fmt.Errorf("bad input %q", b) }
		338	+
		339	+	// explained returns a ValueError, which is shown.
		340	+	type explained string
		341	+
		342	+	func (e *explained) UnmarshalText([]byte) error { return ValueError("must be a colour") }
		343	+
		344	+	func TestDecodeNeverLeaksValues(t *testing.T) {
		345	+	type cfg struct {
		346	+	L leaky `kv:"l"`
		347	+	E explained `kv:"e"`
		348	+	Ratio float64 `kv:"ratio" min:"0" max:"1"`
		349	+	Any float64 `kv:"any"`
		350	+	}
		351	+	tests := []struct{ in, want string }{
		352	+	{"l = hunter2", "line 1: l: invalid kvconf.leaky"},
		353	+	{"e = hunter2", "line 1: e: must be a colour"},
		354	+	{"ratio = NaN", "line 1: ratio: must be a finite number"},
		355	+	{"ratio = +Inf", "line 1: ratio: must be a finite number"},
		356	+	{"any = NaN", "line 1: any: must be a finite number"},
		357	+	}
		358	+	for _, tt := range tests {
		359	+	t.Run(tt.in, func(t *testing.T) {
		360	+	var c cfg
		361	+	w, err := Unmarshal([]byte(tt.in+"\n"), &c, Options{})
		362	+	must(t, err)
		363	+	eq(t, warningStrings(w), []string{tt.want})
		364	+	if strings.Contains(tt.want, "hunter2") {
		365	+	t.Fatal("value leaked")
		366	+	}
		367	+	})
		368	+	}
		369	+	}
		370	+
333	func TestParseBool(t *testing.T) {	371		func TestParseBool(t *testing.T) {
334	tests := []struct {	372		tests := []struct {
335	in string	373		in string

M file.go +6 -5


@@ -24,7 +24,7 @@ func ReadFile(path string, opts Options) ([]byte, error) {
24	24		return nil, err
25	25		}
26	26		if !info.Mode().IsRegular() {
27		-	return nil, &os.PathError{Op: "read", Path: path, Err: errNotRegular}
	27	+	return nil, &os.PathError{Op: "read", Path: path, Err: ErrNotRegular}
28	28		}
29	29		data, err := readLimited(f, opts.maxBytes())
30	30		if errors.Is(err, ErrTooLarge) {

@@ -79,7 +79,7 @@ func (p Path) MarshalText() ([]byte, error) { return []byte(p), nil }
79	79		// ExpandPath expands a leading ~/ and requires an absolute, NUL-free path.
80	80		func ExpandPath(s string) (string, error) {
81	81		if s == "" \|\| strings.IndexByte(s, 0) >= 0 {
82		-	return "", errors.New("must be a non-empty path")
	82	+	return "", ValueError("must be a non-empty path")
83	83		}
84	84		if rest, ok := strings.CutPrefix(s, "~/"); ok \|\| s == "~" {
85	85		if !ok {

@@ -87,17 +87,18 @@ func ExpandPath(s string) (string, error) {
87	87		}
88	88		home, err := os.UserHomeDir()
89	89		if err != nil \|\| !filepath.IsAbs(home) {
90		-	return "", errors.New("no absolute home directory for ~")
	90	+	return "", ValueError("no absolute home directory for ~")
91	91		}
92	92		s = filepath.Join(home, rest)
93	93		}
94	94		if !filepath.IsAbs(s) {
95		-	return "", errors.New("must be an absolute path or start with ~/")
	95	+	return "", ValueError("must be an absolute path or start with ~/")
96	96		}
97	97		return filepath.Clean(s), nil
98	98		}
99	99
100		-	var errNotRegular = errors.New("not a regular file")
	100	+	// ErrNotRegular is returned by ReadFile for a directory, FIFO or device.
	101	+	var ErrNotRegular = errors.New("not a regular file")
101	102
102	103		// isNotExist reports a missing file, including a missing parent directory.
103	104		func isNotExist(err error) bool { return errors.Is(err, fs.ErrNotExist) }

 		return nil, err
 	}
 	if !info.Mode().IsRegular() {
-		return nil, &os.PathError{Op: "read", Path: path, Err: errNotRegular}
+		return nil, &os.PathError{Op: "read", Path: path, Err: ErrNotRegular}
 	}
 	data, err := readLimited(f, opts.maxBytes())
 	if errors.Is(err, ErrTooLarge) {
 // ExpandPath expands a leading ~/ and requires an absolute, NUL-free path.
 func ExpandPath(s string) (string, error) {
 	if s == "" || strings.IndexByte(s, 0) >= 0 {
-		return "", errors.New("must be a non-empty path")
+		return "", ValueError("must be a non-empty path")
 	}
 	if rest, ok := strings.CutPrefix(s, "~/"); ok || s == "~" {
 		if !ok {
 		}
 		home, err := os.UserHomeDir()
 		if err != nil || !filepath.IsAbs(home) {
-			return "", errors.New("no absolute home directory for ~")
+			return "", ValueError("no absolute home directory for ~")
 		}
 		s = filepath.Join(home, rest)
 	}
 	if !filepath.IsAbs(s) {
-		return "", errors.New("must be an absolute path or start with ~/")
+		return "", ValueError("must be an absolute path or start with ~/")
 	}
 	return filepath.Clean(s), nil
 }
-var errNotRegular = errors.New("not a regular file")
+// ErrNotRegular is returned by ReadFile for a directory, FIFO or device.
+var ErrNotRegular = errors.New("not a regular file")
 // isNotExist reports a missing file, including a missing parent directory.
 func isNotExist(err error) bool { return errors.Is(err, fs.ErrNotExist) }

M live.go +35 -6


@@ -5,6 +5,7 @@ import (
5	5		"errors"
6	6		"fmt"
7	7		"io/fs"
	8	+	"slices"
8	9		"sync/atomic"
9	10		)
10	11

@@ -82,7 +83,8 @@ type Live[T any] struct {
82	83		path string
83	84		opts LiveOptions[T]
84	85		cur atomic.Pointer[T]
85		-	entries []Entry
	86	+	entries []Entry // owned by Run
	87	+	running atomic.Bool
86	88		}
87	89
88	90		// Open loads the config file at path and returns it with the warnings of

@@ -106,8 +108,11 @@ func (l *Live[T]) Path() string { return l.path }
106	108		// Run watches the file and reloads it until ctx ends. A reload that fails,
107	109		// or a removed file, keeps the current config. A save that changes no value
108	110		// (a comment, spacing, the same file written again) reloads nothing. Run
109		-	// returns nil when ctx ends, or the error of the watcher.
	111	+	// returns nil when ctx ends, or the error of the watcher. Call it once.
110	112		func (l *Live[T]) Run(ctx context.Context) error {
	113	+	if !l.running.CompareAndSwap(false, true) {
	114	+	return errors.New("kvconf: Live.Run called twice")
	115	+	}
111	116		ctx, cancel := context.WithCancel(ctx)
112	117		defer cancel()
113	118		changed := make(chan struct{}, 1)

@@ -159,17 +164,41 @@ func (l *Live[T]) reload() {
159	164		if sameEntries(entries, l.entries) {
160	165		return
161	166		}
162		-	keys := Changed(Values(l.entries), Values(entries))
	167	+	// Compare every assignment, not the last one: a repeated key whose
	168	+	// last value is invalid decodes to an earlier one.
	169	+	keys := changedKeys(l.entries, entries)
163	170		l.entries = entries
164		-	if len(keys) == 0 {
165		-	return
166		-	}
167	171		old := l.cur.Swap(cfg)
168	172		if l.opts.OnChange != nil {
169	173		l.opts.OnChange(Change[T]{Old: old, New: cfg, Keys: keys, Warnings: warnings})
170	174		}
171	175		}
172	176
	177	+	// changedKeys lists, sorted, the keys whose assignments differ.
	178	+	func changedKeys(old, cur []Entry) []string {
	179	+	group := func(es []Entry) map[string][]string {
	180	+	m := make(map[string][]string, len(es))
	181	+	for _, e := range es {
	182	+	m[e.Key] = append(m[e.Key], e.Value)
	183	+	}
	184	+	return m
	185	+	}
	186	+	a, b := group(old), group(cur)
	187	+	var keys []string
	188	+	for k, v := range b {
	189	+	if !slices.Equal(a[k], v) {
	190	+	keys = append(keys, k)
	191	+	}
	192	+	}
	193	+	for k := range a {
	194	+	if _, ok := b[k]; !ok {
	195	+	keys = append(keys, k)
	196	+	}
	197	+	}
	198	+	slices.Sort(keys)
	199	+	return keys
	200	+	}
	201	+
173	202		func (l *Live[T]) report(err error) {
174	203		if l.opts.OnError != nil && !errors.Is(err, context.Canceled) {
175	204		l.opts.OnError(err)

 	"errors"
 	"fmt"
 	"io/fs"
+	"slices"
 	"sync/atomic"
 )
 	path    string
 	opts    LiveOptions[T]
 	cur     atomic.Pointer[T]
-	entries []Entry
+	entries []Entry // owned by Run
+	running atomic.Bool
 }
 // Open loads the config file at path and returns it with the warnings of
 // Run watches the file and reloads it until ctx ends. A reload that fails,
 // or a removed file, keeps the current config. A save that changes no value
 // (a comment, spacing, the same file written again) reloads nothing. Run
-// returns nil when ctx ends, or the error of the watcher.
+// returns nil when ctx ends, or the error of the watcher. Call it once.
 func (l *Live[T]) Run(ctx context.Context) error {
+	if !l.running.CompareAndSwap(false, true) {
+		return errors.New("kvconf: Live.Run called twice")
+	}
 	ctx, cancel := context.WithCancel(ctx)
 	defer cancel()
 	changed := make(chan struct{}, 1)
 	if sameEntries(entries, l.entries) {
 		return
 	}
-	keys := Changed(Values(l.entries), Values(entries))
+	// Compare every assignment, not the last one: a repeated key whose
+	// last value is invalid decodes to an earlier one.
+	keys := changedKeys(l.entries, entries)
 	l.entries = entries
-	if len(keys) == 0 {
-		return
-	}
 	old := l.cur.Swap(cfg)
 	if l.opts.OnChange != nil {
 		l.opts.OnChange(Change[T]{Old: old, New: cfg, Keys: keys, Warnings: warnings})
 	}
 }
+// changedKeys lists, sorted, the keys whose assignments differ.
+func changedKeys(old, cur []Entry) []string {
+	group := func(es []Entry) map[string][]string {
+		m := make(map[string][]string, len(es))
+		for _, e := range es {
+			m[e.Key] = append(m[e.Key], e.Value)
+		}
+		return m
+	}
+	a, b := group(old), group(cur)
+	var keys []string
+	for k, v := range b {
+		if !slices.Equal(a[k], v) {
+			keys = append(keys, k)
+		}
+	}
+	for k := range a {
+		if _, ok := b[k]; !ok {
+			keys = append(keys, k)
+		}
+	}
+	slices.Sort(keys)
+	return keys
+}
 func (l *Live[T]) report(err error) {
 	if l.opts.OnError != nil && !errors.Is(err, context.Canceled) {
 		l.opts.OnError(err)

M live_test.go +24 -0


@@ -201,6 +201,18 @@ func TestLive(t *testing.T) {
201	201		eq(t, l.Get(), liveConfig{"dark", 5 time.Second})
202	202		},
203	203		},
	204	+	{
	205	+	name: "repeated key with an invalid last value",
	206	+	initial: ptr("after = 5s\nafter = bad\n"),
	207	+	steps: func(t testing.T, path string, l Live[liveConfig], r *liveRecorder) {
	208	+	eq(t, l.Get().After, 5*time.Second)
	209	+	must(t, WriteFile(path, []byte("after = 6s\nafter = bad\n"), 0o600))
	210	+	r.wait(t, "a change of the valid value")
	211	+	eq(t, l.Get().After, 6*time.Second)
	212	+	c, _ := r.last()
	213	+	eq(t, c.Keys, []string{"after"})
	214	+	},
	215	+	},
204	216		{
205	217		name: "warnings are reported with the change",
206	218		initial: ptr("after = 1s\n"),

@@ -250,6 +262,18 @@ func TestLiveSeesSaveBetweenOpenAndRun(t *testing.T) {
250	262		eq(t, l.Get().Theme, "light")
251	263		}
252	264
	265	+	func TestLiveRunTwice(t *testing.T) {
	266	+	l, _, err := Open(filepath.Join(t.TempDir(), "config"), LiveOptions[liveConfig]{})
	267	+	must(t, err)
	268	+	ctx, cancel := context.WithCancel(context.Background())
	269	+	defer cancel()
	270	+	go func() { _ = l.Run(ctx) }()
	271	+	time.Sleep(50 * time.Millisecond)
	272	+	if err := l.Run(ctx); err == nil {
	273	+	t.Fatal("a second Run was accepted")
	274	+	}
	275	+	}
	276	+
253	277		func TestLiveConcurrentGet(t *testing.T) {
254	278		path := writeTemp(t, t.TempDir(), "config", "after = 1s\n")
255	279		r := newRecorder()


@@ -201,6 +201,18 @@ func TestLive(t *testing.T) {
201	eq(t, l.Get(), liveConfig{"dark", 5 time.Second})	201		eq(t, l.Get(), liveConfig{"dark", 5 time.Second})
202	},	202		},
203	},	203		},
		204	+	{
		205	+	name: "repeated key with an invalid last value",
		206	+	initial: ptr("after = 5s\nafter = bad\n"),
		207	+	steps: func(t testing.T, path string, l Live[liveConfig], r *liveRecorder) {
		208	+	eq(t, l.Get().After, 5*time.Second)
		209	+	must(t, WriteFile(path, []byte("after = 6s\nafter = bad\n"), 0o600))
		210	+	r.wait(t, "a change of the valid value")
		211	+	eq(t, l.Get().After, 6*time.Second)
		212	+	c, _ := r.last()
		213	+	eq(t, c.Keys, []string{"after"})
		214	+	},
		215	+	},
204	{	216		{
205	name: "warnings are reported with the change",	217		name: "warnings are reported with the change",
206	initial: ptr("after = 1s\n"),	218		initial: ptr("after = 1s\n"),

@@ -250,6 +262,18 @@ func TestLiveSeesSaveBetweenOpenAndRun(t *testing.T) {
250	eq(t, l.Get().Theme, "light")	262		eq(t, l.Get().Theme, "light")
251	}	263		}
252		264
		265	+	func TestLiveRunTwice(t *testing.T) {
		266	+	l, _, err := Open(filepath.Join(t.TempDir(), "config"), LiveOptions[liveConfig]{})
		267	+	must(t, err)
		268	+	ctx, cancel := context.WithCancel(context.Background())
		269	+	defer cancel()
		270	+	go func() { _ = l.Run(ctx) }()
		271	+	time.Sleep(50 * time.Millisecond)
		272	+	if err := l.Run(ctx); err == nil {
		273	+	t.Fatal("a second Run was accepted")
		274	+	}
		275	+	}
		276	+
253	func TestLiveConcurrentGet(t *testing.T) {	277		func TestLiveConcurrentGet(t *testing.T) {
254	path := writeTemp(t, t.TempDir(), "config", "after = 1s\n")	278		path := writeTemp(t, t.TempDir(), "config", "after = 1s\n")
255	r := newRecorder()	279		r := newRecorder()

M marshal.go +36 -16


@@ -29,17 +29,15 @@ func needsQuote(v string) bool {
29	29		if v[0] == '"' \|\| v[0] == '\'' {
30	30		return true // it would be unquoted on reading
31	31		}
32		-	for i := 0; i < len(v); i++ {
33		-	switch v[i] {
34		-	case '\n', '\r':
35		-	return true
36		-	case '#':
37		-	if i > 0 && (v[i-1] == ' ' \|\| v[i-1] == '\t') {
38		-	return true
39		-	}
40		-	}
41		-	}
42		-	return false
	32	+	// A '#' after a space ends the value unless it is inside a quoted word,
	33	+	// so quote any value where Parse would cut it.
	34	+	return strings.ContainsAny(v, "\n\r") \|\| stripComment(v, CommentSpaced) != v
	35	+	}
	36	+
	37	+	// validKey reports whether key can be written and read back as one key.
	38	+	func validKey(key string) bool {
	39	+	return key != "" && key[0] != '#' && key == strings.TrimSpace(key) &&
	40	+	!strings.ContainsAny(key, "=\n\r")
43	41		}
44	42
45	43		func appendQuoted(b []byte, v string) []byte {

@@ -103,12 +101,16 @@ func Marshal(v any) ([]byte, error) {
103	101		fv := rv.FieldByIndex(s.catchAll.index)
104	102		if es, ok := fv.Interface().([]Entry); ok {
105	103		for _, e := range es {
106		-	m.raw(e.Key, e.Value)
	104	+	if err := m.raw(e.Key, e.Value); err != nil {
	105	+	return nil, err
	106	+	}
107	107		}
108	108		} else {
109	109		kv := fv.Interface().(map[string]string)
110	110		for _, k := range slices.Sorted(maps.Keys(kv)) {
111		-	m.raw(k, kv[k])
	111	+	if err := m.raw(k, kv[k]); err != nil {
	112	+	return nil, err
	113	+	}
112	114		}
113	115		}
114	116		}

@@ -120,16 +122,23 @@ type marshaler struct {
120	122		tmp []byte // the value being formatted, reused
121	123		}
122	124
123		-	func (m *marshaler) raw(key, value string) {
	125	+	func (m *marshaler) raw(key, value string) error {
	126	+	if !validKey(key) {
	127	+	return fmt.Errorf("kvconf: invalid key %q", key)
	128	+	}
124	129		m.b = append(m.b, key...)
125	130		m.b = append(m.b, " = "...)
126	131		m.b = appendQuoted(m.b, value)
127	132		m.b = append(m.b, '\n')
	133	+	return nil
128	134		}
129	135
130	136		// line writes prefix+name[.sub] = value.
131	137		func (m marshaler) line(prefix, name, sub string, v reflect.Value, f field) error {
132	138		var err error
	139	+	if name != "" && (strings.ContainsAny(name, "=\n\r") \|\| name != strings.TrimSpace(name)) {
	140	+	return fmt.Errorf("kvconf: %s: invalid map name %q", prefix, name)
	141	+	}
133	142		if m.tmp, err = appendValue(m.tmp[:0], v, f); err != nil {
134	143		key := prefix + name
135	144		if sub != "" {

@@ -154,8 +163,19 @@ func appendValue(b []byte, v reflect.Value, f *field) ([]byte, error) {
154	163		if f.off && v.IsZero() {
155	164		return append(b, "off"...), nil
156	165		}
157		-	if v.Type().Implements(marshalType) {
158		-	text, err := v.Interface().(encoding.TextMarshaler).MarshalText()
	166	+	if f.marshal {
	167	+	if !v.CanAddr() { // a map element: copy it to call a pointer method
	168	+	p := reflect.New(v.Type())
	169	+	p.Elem().Set(v)
	170	+	v = p.Elem()
	171	+	}
	172	+	var tm encoding.TextMarshaler
	173	+	if v.Type().Implements(marshalType) {
	174	+	tm = v.Interface().(encoding.TextMarshaler)
	175	+	} else {
	176	+	tm = v.Addr().Interface().(encoding.TextMarshaler)
	177	+	}
	178	+	text, err := tm.MarshalText()
159	179		return append(b, text...), err
160	180		}
161	181		switch v.Kind() {


@@ -29,17 +29,15 @@ func needsQuote(v string) bool {
29		if v[0] == '"' \|\| v[0] == '\'' {	29		if v[0] == '"' \|\| v[0] == '\'' {
30		return true // it would be unquoted on reading	30		return true // it would be unquoted on reading
31		}	31		}
32	-	for i := 0; i < len(v); i++ {	32	+	// A '#' after a space ends the value unless it is inside a quoted word,
33	-	switch v[i] {	33	+	// so quote any value where Parse would cut it.
34	-	case '\n', '\r':	34	+	return strings.ContainsAny(v, "\n\r") \|\| stripComment(v, CommentSpaced) != v
35	-	return true	35	+	}
36	-	case '#':	36	+
37	-	if i > 0 && (v[i-1] == ' ' \|\| v[i-1] == '\t') {	37	+	// validKey reports whether key can be written and read back as one key.
38	-	return true	38	+	func validKey(key string) bool {
39	-	}	39	+	return key != "" && key[0] != '#' && key == strings.TrimSpace(key) &&
40	-	}	40	+	!strings.ContainsAny(key, "=\n\r")
41	-	}
42	-	return false
43		}	41		}
44			42
45		func appendQuoted(b []byte, v string) []byte {	43		func appendQuoted(b []byte, v string) []byte {

@@ -103,12 +101,16 @@ func Marshal(v any) ([]byte, error) {
103		fv := rv.FieldByIndex(s.catchAll.index)	101		fv := rv.FieldByIndex(s.catchAll.index)
104		if es, ok := fv.Interface().([]Entry); ok {	102		if es, ok := fv.Interface().([]Entry); ok {
105		for _, e := range es {	103		for _, e := range es {
106	-	m.raw(e.Key, e.Value)	104	+	if err := m.raw(e.Key, e.Value); err != nil {
			105	+	return nil, err
			106	+	}
107		}	107		}
108		} else {	108		} else {
109		kv := fv.Interface().(map[string]string)	109		kv := fv.Interface().(map[string]string)
110		for _, k := range slices.Sorted(maps.Keys(kv)) {	110		for _, k := range slices.Sorted(maps.Keys(kv)) {
111	-	m.raw(k, kv[k])	111	+	if err := m.raw(k, kv[k]); err != nil {
			112	+	return nil, err
			113	+	}
112		}	114		}
113		}	115		}
114		}	116		}

@@ -120,16 +122,23 @@ type marshaler struct {
120		tmp []byte // the value being formatted, reused	122		tmp []byte // the value being formatted, reused
121		}	123		}
122			124
123	-	func (m *marshaler) raw(key, value string) {	125	+	func (m *marshaler) raw(key, value string) error {
			126	+	if !validKey(key) {
			127	+	return fmt.Errorf("kvconf: invalid key %q", key)
			128	+	}
124		m.b = append(m.b, key...)	129		m.b = append(m.b, key...)
125		m.b = append(m.b, " = "...)	130		m.b = append(m.b, " = "...)
126		m.b = appendQuoted(m.b, value)	131		m.b = appendQuoted(m.b, value)
127		m.b = append(m.b, '\n')	132		m.b = append(m.b, '\n')
			133	+	return nil
128		}	134		}
129			135
130		// line writes prefix+name[.sub] = value.	136		// line writes prefix+name[.sub] = value.
131		func (m marshaler) line(prefix, name, sub string, v reflect.Value, f field) error {	137		func (m marshaler) line(prefix, name, sub string, v reflect.Value, f field) error {
132		var err error	138		var err error
			139	+	if name != "" && (strings.ContainsAny(name, "=\n\r") \|\| name != strings.TrimSpace(name)) {
			140	+	return fmt.Errorf("kvconf: %s: invalid map name %q", prefix, name)
			141	+	}
133		if m.tmp, err = appendValue(m.tmp[:0], v, f); err != nil {	142		if m.tmp, err = appendValue(m.tmp[:0], v, f); err != nil {
134		key := prefix + name	143		key := prefix + name
135		if sub != "" {	144		if sub != "" {

@@ -154,8 +163,19 @@ func appendValue(b []byte, v reflect.Value, f *field) ([]byte, error) {
154		if f.off && v.IsZero() {	163		if f.off && v.IsZero() {
155		return append(b, "off"...), nil	164		return append(b, "off"...), nil
156		}	165		}
157	-	if v.Type().Implements(marshalType) {	166	+	if f.marshal {
158	-	text, err := v.Interface().(encoding.TextMarshaler).MarshalText()	167	+	if !v.CanAddr() { // a map element: copy it to call a pointer method
			168	+	p := reflect.New(v.Type())
			169	+	p.Elem().Set(v)
			170	+	v = p.Elem()
			171	+	}
			172	+	var tm encoding.TextMarshaler
			173	+	if v.Type().Implements(marshalType) {
			174	+	tm = v.Interface().(encoding.TextMarshaler)
			175	+	} else {
			176	+	tm = v.Addr().Interface().(encoding.TextMarshaler)
			177	+	}
			178	+	text, err := tm.MarshalText()
159		return append(b, text...), err	179		return append(b, text...), err
160		}	180		}
161		switch v.Kind() {	181		switch v.Kind() {

M open_other.go +1 -1


@@ -6,7 +6,7 @@ import "os"
6	6
7	7		func openNonblocking(path string) (*os.File, error) {
8	8		if info, err := os.Stat(path); err == nil && !info.Mode().IsRegular() {
9		-	return nil, &os.PathError{Op: "open", Path: path, Err: errNotRegular}
	9	+	return nil, &os.PathError{Op: "open", Path: path, Err: ErrNotRegular}
10	10		}
11	11		return os.Open(path)
12	12		}


@@ -6,7 +6,7 @@ import "os"
6			6
7		func openNonblocking(path string) (*os.File, error) {	7		func openNonblocking(path string) (*os.File, error) {
8		if info, err := os.Stat(path); err == nil && !info.Mode().IsRegular() {	8		if info, err := os.Stat(path); err == nil && !info.Mode().IsRegular() {
9	-	return nil, &os.PathError{Op: "open", Path: path, Err: errNotRegular}	9	+	return nil, &os.PathError{Op: "open", Path: path, Err: ErrNotRegular}
10		}	10		}
11		return os.Open(path)	11		return os.Open(path)
12		}	12		}

M parse.go +5 -7


@@ -206,13 +206,10 @@ func ParseBytes(data []byte, opts Options) ([]Entry, []Warning, error) {
206	206		}
207	207		continue
208	208		}
	209	+	// A key may contain spaces: output names such as "LG Electronics 27GR95UM".
209	210		key := strings.TrimSpace(line[:eq])
210		-	if key == "" \|\| strings.ContainsAny(key, " \t") {
211		-	msg := "missing key"
212		-	if key != "" {
213		-	msg = "key contains a space"
214		-	}
215		-	if !p.add(n, "", msg) {
	211	+	if key == "" {
	212	+	if !p.add(n, "", "missing key") {
216	213		break
217	214		}
218	215		continue

@@ -250,7 +247,8 @@ func stripComment(v string, style CommentStyle) string {
250	247		} else if c == quote {
251	248		quote = 0
252	249		}
253		-	case c == '"' \|\| c == '\'':
	250	+	case (c == '"' \|\| c == '\'') && (i == 0 \|\| v[i-1] == ' ' \|\| v[i-1] == '\t'):
	251	+	// Only a quote that starts a word opens a string: it's stays text.
254	252		quote = c
255	253		case c == '#' && (style == CommentAny \|\| i > 0 && (v[i-1] == ' ' \|\| v[i-1] == '\t')):
256	254		return strings.TrimSpace(v[:i])

 			}
 			continue
 		}
+		// A key may contain spaces: output names such as "LG Electronics 27GR95UM".
 		key := strings.TrimSpace(line[:eq])
-		if key == "" || strings.ContainsAny(key, " \t") {
+		if key == "" {
-			msg := "missing key"
+			if !p.add(n, "", "missing key") {
-			if key != "" {
-				msg = "key contains a space"
-			}
-			if !p.add(n, "", msg) {
 				break
 			}
 			continue
 			} else if c == quote {
 				quote = 0
 			}
-		case c == '"' || c == '\'':
+		case (c == '"' || c == '\'') && (i == 0 || v[i-1] == ' ' || v[i-1] == '\t'):
+			// Only a quote that starts a word opens a string: it's stays text.
 			quote = c
 		case c == '#' && (style == CommentAny || i > 0 && (v[i-1] == ' ' || v[i-1] == '\t')):
 			return strings.TrimSpace(v[:i])

M parse_test.go +13 -3


@@ -79,6 +79,11 @@ g = spawn "foo bar"
79	79		{"g", `spawn "foo bar"`, 7},
80	80		},
81	81		},
	82	+	{
	83	+	name: "apostrophe inside a word keeps the comment",
	84	+	in: "title = it's fine # a note\nb = say \"x # y\" # c\n",
	85	+	want: []Entry{{"title", "it's fine", 1}, {"b", `say "x # y"`, 2}},
	86	+	},
82	87		{
83	88		name: "raw values",
84	89		in: `a = "x"` + "\n",

@@ -99,9 +104,14 @@ g = spawn "foo bar"
99	104		},
100	105		{
101	106		name: "bad lines are skipped",
102		-	in: "nope\n= 1\na b = 1\nok = 1\n",
103		-	want: []Entry{{"ok", "1", 4}},
104		-	warnings: []string{"line 1: expected key = value", "line 2: missing key", "line 3: key contains a space"},
	107	+	in: "nope\n= 1\nok = 1\n",
	108	+	want: []Entry{{"ok", "1", 3}},
	109	+	warnings: []string{"line 1: expected key = value", "line 2: missing key"},
	110	+	},
	111	+	{
	112	+	name: "key with spaces",
	113	+	in: "layout.LG Electronics 27GR95UM.overflow = scroll\n",
	114	+	want: []Entry{{"layout.LG Electronics 27GR95UM.overflow", "scroll", 1}},
105	115		},
106	116		{
107	117		name: "strict stops at first problem",

 				{"g", `spawn "foo bar"`, 7},
 			},
 		},
+		{
+			name: "apostrophe inside a word keeps the comment",
+			in:   "title = it's fine   # a note\nb = say \"x # y\" # c\n",
+			want: []Entry{{"title", "it's fine", 1}, {"b", `say "x # y"`, 2}},
+		},
 		{
 			name: "raw values",
 			in:   `a = "x"` + "\n",
 		},
 		{
 			name:     "bad lines are skipped",
-			in:       "nope\n= 1\na b = 1\nok = 1\n",
+			in:       "nope\n= 1\nok = 1\n",
-			want:     []Entry{{"ok", "1", 4}},
+			want:     []Entry{{"ok", "1", 3}},
-			warnings: []string{"line 1: expected key = value", "line 2: missing key", "line 3: key contains a space"},
+			warnings: []string{"line 1: expected key = value", "line 2: missing key"},
+		},
+		{
+			name: "key with spaces",
+			in:   "layout.LG Electronics 27GR95UM.overflow  = scroll\n",
+			want: []Entry{{"layout.LG Electronics 27GR95UM.overflow", "scroll", 1}},
 		},
 		{
 			name:     "strict stops at first problem",

M rename_other.go +11 -0


@@ -5,3 +5,14 @@ package kvconf
5	5		import "os"
6	6
7	7		func rename(from, to string) error { return os.Rename(from, to) }
	8	+
	9	+	// syncDir makes a rename in dir survive a power loss. Some file systems
	10	+	// cannot sync a directory; the rename is done anyway.
	11	+	func syncDir(dir string) error {
	12	+	d, err := os.Open(dir)
	13	+	if err != nil {
	14	+	return nil
	15	+	}
	16	+	_ = d.Sync()
	17	+	return d.Close()
	18	+	}


@@ -5,3 +5,14 @@ package kvconf
5	import "os"	5		import "os"
6		6
7	func rename(from, to string) error { return os.Rename(from, to) }	7		func rename(from, to string) error { return os.Rename(from, to) }
		8	+
		9	+	// syncDir makes a rename in dir survive a power loss. Some file systems
		10	+	// cannot sync a directory; the rename is done anyway.
		11	+	func syncDir(dir string) error {
		12	+	d, err := os.Open(dir)
		13	+	if err != nil {
		14	+	return nil
		15	+	}
		16	+	_ = d.Sync()
		17	+	return d.Close()
		18	+	}

M rename_windows.go +3 -0


@@ -7,6 +7,9 @@ import (
7	7		"time"
8	8		)
9	9
	10	+	// syncDir: Windows makes a rename durable with the file system journal.
	11	+	func syncDir(string) error { return nil }
	12	+
10	13		// rename retries while another process (an editor, an indexer, our own
11	14		// watcher) holds the target open, which Windows reports as a sharing error.
12	15		func rename(from, to string) error {


@@ -7,6 +7,9 @@ import (
7	"time"	7		"time"
8	)	8		)
9		9
		10	+	// syncDir: Windows makes a rename durable with the file system journal.
		11	+	func syncDir(string) error { return nil }
		12	+
10	// rename retries while another process (an editor, an indexer, our own	13		// rename retries while another process (an editor, an indexer, our own
11	// watcher) holds the target open, which Windows reports as a sharing error.	14		// watcher) holds the target open, which Windows reports as a sharing error.
12	func rename(from, to string) error {	15		func rename(from, to string) error {

M watch.go +109 -33


@@ -126,6 +126,11 @@ func Watch(ctx context.Context, path string, opts WatchOptions, out chan<- struc
126	126		}
127	127		if !pending.IsZero() && !now.Before(pending) {
128	128		pending = time.Time{}
	129	+	// Record what is reported, so a later rewatch does not report
	130	+	// the same change again.
	131	+	for _, t := range w.targets {
	132	+	w.stampsChanged(t)
	133	+	}
129	134		select {
130	135		case out <- struct{}{}:
131	136		default:

@@ -158,6 +163,24 @@ type target struct {
158	163		// next is the missing child on the way to the wanted directory: its
159	164		// creation means the watches must move down.
160	165		next string
	166	+	// paths are dir joined with names and next, computed once.
	167	+	paths []string
	168	+	// moved lists watched child directories whose rename must move the
	169	+	// watches (Windows does not report a watched directory's own rename).
	170	+	moved []string
	171	+	}
	172	+
	173	+	// stampPaths lists the full paths whose stamps matter.
	174	+	func (t *target) stampPaths() []string {
	175	+	if t.paths == nil {
	176	+	for _, n := range t.names {
	177	+	t.paths = append(t.paths, filepath.Join(t.dir, n))
	178	+	}
	179	+	if t.next != "" {
	180	+	t.paths = append(t.paths, filepath.Join(t.dir, t.next))
	181	+	}
	182	+	}
	183	+	return t.paths
161	184		}
162	185
163	186		func (t *target) has(name string) bool {

@@ -187,27 +210,67 @@ type watcher struct {
187	210		stamps map[string]stamp
188	211		}
189	212
190		-	// chain lists path and every symlink target it goes through.
	213	+	// chain lists the paths whose change can change what path names: path
	214	+	// itself, every symlink met while resolving it, in the file name or in a
	215	+	// parent directory (GNU stow links whole directories), and the final file.
	216	+	// The resolved file is last.
191	217		func chain(path string) []string {
192		-	out := []string{filepath.Clean(path)}
193		-	for len(out) <= 40 { // the kernel's symlink limit
194		-	cur := out[len(out)-1]
195		-	link, err := os.Readlink(cur)
196		-	if err != nil {
	218	+	path = filepath.Clean(path)
	219	+	out := []string{path}
	220	+	resolved, links := path, 0
	221	+	for {
	222	+	next, link, ok := resolveOnce(resolved)
	223	+	if !ok {
197	224		break
198	225		}
199		-	if !filepath.IsAbs(link) {
200		-	link = filepath.Join(filepath.Dir(cur), link)
	226	+	if links++; links > 40 { // the kernel's symlink limit
	227	+	break
	228	+	}
	229	+	if !slices.Contains(out, link) {
	230	+	out = append(out, link)
201	231		}
202		-	link = filepath.Clean(link)
203		-	if slices.Contains(out, link) {
	232	+	if slices.Contains(out, next) && next != path {
204	233		break // a loop: the file is unreadable anyway
205	234		}
206		-	out = append(out, link)
	235	+	resolved = next
	236	+	}
	237	+	if resolved != path {
	238	+	out = append(slices.DeleteFunc(out, func(p string) bool { return p == resolved }), resolved)
207	239		}
208	240		return out
209	241		}
210	242
	243	+	// resolveOnce replaces the first symlink in path, from the root down, by its
	244	+	// target. link is the symlink's own path.
	245	+	func resolveOnce(path string) (next, link string, ok bool) {
	246	+	vol := filepath.VolumeName(path)
	247	+	rest := path[len(vol):]
	248	+	cur := vol
	249	+	for rest != "" {
	250	+	sep := strings.IndexByte(rest, filepath.Separator)
	251	+	var part string
	252	+	if sep < 0 {
	253	+	part, rest = rest, ""
	254	+	} else {
	255	+	part, rest = rest[:sep], rest[sep+1:]
	256	+	}
	257	+	if part == "" {
	258	+	cur += string(filepath.Separator)
	259	+	continue
	260	+	}
	261	+	cur = filepath.Join(cur, part)
	262	+	target, err := os.Readlink(cur)
	263	+	if err != nil {
	264	+	continue // not a link, or missing: later parts cannot resolve
	265	+	}
	266	+	if !filepath.IsAbs(target) {
	267	+	target = filepath.Join(filepath.Dir(cur), target)
	268	+	}
	269	+	return filepath.Join(target, rest), cur, true
	270	+	}
	271	+	return path, "", false
	272	+	}
	273	+
211	274		func isDir(path string) bool {
212	275		info, err := os.Stat(path)
213	276		return err == nil && info.IsDir()

@@ -255,6 +318,14 @@ func (w watcher) plan() (targets []target, files []string) {
255	318		add(filepath.Dir(f), normName(filepath.Base(f)), false)
256	319		}
257	320		}
	321	+	if runtime.GOOS == "windows" {
	322	+	for _, t := range slices.Clone(targets) {
	323	+	if parent := filepath.Dir(t.dir); parent != t.dir && isDir(parent) {
	324	+	p := get(parent)
	325	+	p.moved = append(p.moved, normName(filepath.Base(t.dir)))
	326	+	}
	327	+	}
	328	+	}
258	329		return targets, files
259	330		}
260	331

@@ -285,12 +356,14 @@ func (w *watcher) rewatchChanged(b backend) (missing, changed bool, err error) {
285	356
286	357		func sameTargets(a, b []*target) bool {
287	358		return slices.EqualFunc(a, b, func(x, y *target) bool {
288		-	return x.dir == y.dir && x.all == y.all && x.next == y.next && slices.Equal(x.names, y.names)
	359	+	return x.dir == y.dir && x.all == y.all && x.next == y.next && slices.Equal(x.names, y.names) &&
	360	+	slices.Equal(x.moved, y.moved)
289	361		})
290	362		}
291	363
292		-	// classify reports whether evs touch a watched file, and whether the
293		-	// watches must be installed again.
	364	+	// classify reports whether evs touch a watched file, whether the watches
	365	+	// must be installed again, and whether to look again later because a
	366	+	// watch was lost.
294	367		func (w *watcher) classify(evs []event) (relevant, rewatch, recheck bool) {
295	368		relinked := false
296	369		for _, ev := range evs {

@@ -310,23 +383,31 @@ func (w *watcher) classify(evs []event) (relevant, rewatch, recheck bool) {
310	383		continue
311	384		}
312	385		name := normName(ev.name)
313		-	if t.next != "" && name == t.next {
	386	+	if t.next != "" && name == t.next \|\| ev.moved && slices.Contains(t.moved, name) {
314	387		relevant, rewatch = true, true
315	388		}
316		-	// Some systems (Windows, network file systems) do not report a
317		-	// watched directory that was removed: check it on removals.
318	389		if ev.moved && !rewatch && !isDir(t.dir) {
319		-	relevant, rewatch = true, true
	390	+	// Some systems (Windows, network file systems) do not report a
	391	+	// watched directory that was removed: look again later.
	392	+	relevant, rewatch, recheck = true, true, true
	393	+	continue
320	394		}
321	395		if t.has(name) {
	396	+	if t.all && ev.moved && isDir(filepath.Join(t.dir, name)) {
	397	+	continue // Dir mode ignores subdirectories
	398	+	}
322	399		relevant = true
323	400		relinked = relinked \|\| ev.moved
	401	+	// Windows reports a removed directory only once its last handle,
	402	+	// ours, closes: after a removal inside it, look again later. The
	403	+	// stamps keep that second look from reporting the change twice.
	404	+	recheck = recheck \|\| ev.moved && runtime.GOOS == "windows"
324	405		}
325	406		}
326	407		if relinked && !rewatch && !w.dir && !slices.Equal(chain(w.path), w.chain) {
327	408		rewatch = true // a link points elsewhere now
328	409		}
329		-	return relevant, rewatch, recheck \|\| relinked
	410	+	return relevant, rewatch, recheck
330	411		}
331	412
332	413		func (w watcher) find(dir string) target {

@@ -354,8 +435,7 @@ func stampOf(info fs.FileInfo) stamp {
354	435		// reports whether any was created, changed or removed.
355	436		func (w watcher) stampsChanged(t target) bool {
356	437		changed := false
357		-	check := func(name string, info fs.FileInfo) {
358		-	key := t.dir + string(filepath.Separator) + name
	438	+	check := func(key string, info fs.FileInfo) {
359	439		old, had := w.stamps[key]
360	440		if info == nil {
361	441		if had {

@@ -369,20 +449,16 @@ func (w watcher) stampsChanged(t target) bool {
369	449		changed = true
370	450		}
371	451		}
372		-	names := t.names
373		-	if t.next != "" {
374		-	names = append(slices.Clip(names), t.next)
375		-	}
376		-	for _, name := range names {
377		-	info, err := os.Lstat(filepath.Join(t.dir, name))
	452	+	paths := t.stampPaths()
	453	+	for _, p := range paths {
	454	+	info, err := os.Lstat(p)
378	455		if err != nil {
379	456		info = nil
380	457		}
381		-	check(name, info)
	458	+	check(p, info)
382	459		}
383	460		if t.all {
384	461		entries, _ := os.ReadDir(t.dir)
385		-	prefix := t.dir + string(filepath.Separator)
386	462		live := map[string]bool{}
387	463		for _, e := range entries {
388	464		if !visible(e.Name()) \|\| e.IsDir() {

@@ -392,12 +468,12 @@ func (w watcher) stampsChanged(t target) bool {
392	468		if err != nil {
393	469		continue
394	470		}
395		-	live[e.Name()] = true
396		-	check(e.Name(), info)
	471	+	key := filepath.Join(t.dir, e.Name())
	472	+	live[key] = true
	473	+	check(key, info)
397	474		}
398	475		for key := range w.stamps {
399		-	if rest, ok := strings.CutPrefix(key, prefix); ok && !strings.ContainsRune(rest, filepath.Separator) &&
400		-	!live[rest] && !slices.Contains(names, rest) {
	476	+	if filepath.Dir(key) == t.dir && !live[key] && !slices.Contains(paths, key) {
401	477		delete(w.stamps, key)
402	478		changed = true
403	479		}

 		}
 		if !pending.IsZero() && !now.Before(pending) {
 			pending = time.Time{}
+			// Record what is reported, so a later rewatch does not report
+			// the same change again.
+			for _, t := range w.targets {
+				w.stampsChanged(t)
+			}
 			select {
 			case out <- struct{}{}:
 			default:
 	// next is the missing child on the way to the wanted directory: its
 	// creation means the watches must move down.
 	next string
+	// paths are dir joined with names and next, computed once.
+	paths []string
+	// moved lists watched child directories whose rename must move the
+	// watches (Windows does not report a watched directory's own rename).
+	moved []string
+}
+// stampPaths lists the full paths whose stamps matter.
+func (t *target) stampPaths() []string {
+	if t.paths == nil {
+		for _, n := range t.names {
+			t.paths = append(t.paths, filepath.Join(t.dir, n))
+		}
+		if t.next != "" {
+			t.paths = append(t.paths, filepath.Join(t.dir, t.next))
+		}
+	}
+	return t.paths
 }
 func (t *target) has(name string) bool {
 	stamps  map[string]stamp
 }
-// chain lists path and every symlink target it goes through.
+// chain lists the paths whose change can change what path names: path
+// itself, every symlink met while resolving it, in the file name or in a
+// parent directory (GNU stow links whole directories), and the final file.
+// The resolved file is last.
 func chain(path string) []string {
-	out := []string{filepath.Clean(path)}
+	path = filepath.Clean(path)
-	for len(out) <= 40 { // the kernel's symlink limit
+	out := []string{path}
-		cur := out[len(out)-1]
+	resolved, links := path, 0
-		link, err := os.Readlink(cur)
+	for {
-		if err != nil {
+		next, link, ok := resolveOnce(resolved)
+		if !ok {
 			break
 		}
-		if !filepath.IsAbs(link) {
+		if links++; links > 40 { // the kernel's symlink limit
-			link = filepath.Join(filepath.Dir(cur), link)
+			break
+		}
+		if !slices.Contains(out, link) {
+			out = append(out, link)
 		}
-		link = filepath.Clean(link)
+		if slices.Contains(out, next) && next != path {
-		if slices.Contains(out, link) {
 			break // a loop: the file is unreadable anyway
 		}
-		out = append(out, link)
+		resolved = next
+	}
+	if resolved != path {
+		out = append(slices.DeleteFunc(out, func(p string) bool { return p == resolved }), resolved)
 	}
 	return out
 }
+// resolveOnce replaces the first symlink in path, from the root down, by its
+// target. link is the symlink's own path.
+func resolveOnce(path string) (next, link string, ok bool) {
+	vol := filepath.VolumeName(path)
+	rest := path[len(vol):]
+	cur := vol
+	for rest != "" {
+		sep := strings.IndexByte(rest, filepath.Separator)
+		var part string
+		if sep < 0 {
+			part, rest = rest, ""
+		} else {
+			part, rest = rest[:sep], rest[sep+1:]
+		}
+		if part == "" {
+			cur += string(filepath.Separator)
+			continue
+		}
+		cur = filepath.Join(cur, part)
+		target, err := os.Readlink(cur)
+		if err != nil {
+			continue // not a link, or missing: later parts cannot resolve
+		}
+		if !filepath.IsAbs(target) {
+			target = filepath.Join(filepath.Dir(cur), target)
+		}
+		return filepath.Join(target, rest), cur, true
+	}
+	return path, "", false
+}
 func isDir(path string) bool {
 	info, err := os.Stat(path)
 	return err == nil && info.IsDir()
 			add(filepath.Dir(f), normName(filepath.Base(f)), false)
 		}
 	}
+	if runtime.GOOS == "windows" {
+		for _, t := range slices.Clone(targets) {
+			if parent := filepath.Dir(t.dir); parent != t.dir && isDir(parent) {
+				p := get(parent)
+				p.moved = append(p.moved, normName(filepath.Base(t.dir)))
+			}
+		}
+	}
 	return targets, files
 }
 func sameTargets(a, b []*target) bool {
 	return slices.EqualFunc(a, b, func(x, y *target) bool {
-		return x.dir == y.dir && x.all == y.all && x.next == y.next && slices.Equal(x.names, y.names)
+		return x.dir == y.dir && x.all == y.all && x.next == y.next && slices.Equal(x.names, y.names) &&
+			slices.Equal(x.moved, y.moved)
 	})
 }
-// classify reports whether evs touch a watched file, and whether the
+// classify reports whether evs touch a watched file, whether the watches
-// watches must be installed again.
+// must be installed again, and whether to look again later because a
+// watch was lost.
 func (w *watcher) classify(evs []event) (relevant, rewatch, recheck bool) {
 	relinked := false
 	for _, ev := range evs {
 			continue
 		}
 		name := normName(ev.name)
-		if t.next != "" && name == t.next {
+		if t.next != "" && name == t.next || ev.moved && slices.Contains(t.moved, name) {
 			relevant, rewatch = true, true
 		}
-		// Some systems (Windows, network file systems) do not report a
-		// watched directory that was removed: check it on removals.
 		if ev.moved && !rewatch && !isDir(t.dir) {
-			relevant, rewatch = true, true
+			// Some systems (Windows, network file systems) do not report a
+			// watched directory that was removed: look again later.
+			relevant, rewatch, recheck = true, true, true
+			continue
 		}
 		if t.has(name) {
+			if t.all && ev.moved && isDir(filepath.Join(t.dir, name)) {
+				continue // Dir mode ignores subdirectories
+			}
 			relevant = true
 			relinked = relinked || ev.moved
+			// Windows reports a removed directory only once its last handle,
+			// ours, closes: after a removal inside it, look again later. The
+			// stamps keep that second look from reporting the change twice.
+			recheck = recheck || ev.moved && runtime.GOOS == "windows"
 		}
 	}
 	if relinked && !rewatch && !w.dir && !slices.Equal(chain(w.path), w.chain) {
 		rewatch = true // a link points elsewhere now
 	}
-	return relevant, rewatch, recheck || relinked
+	return relevant, rewatch, recheck
 }
 func (w *watcher) find(dir string) *target {
 // reports whether any was created, changed or removed.
 func (w *watcher) stampsChanged(t *target) bool {
 	changed := false
-	check := func(name string, info fs.FileInfo) {
+	check := func(key string, info fs.FileInfo) {
-		key := t.dir + string(filepath.Separator) + name
 		old, had := w.stamps[key]
 		if info == nil {
 			if had {
 			changed = true
 		}
 	}
-	names := t.names
+	paths := t.stampPaths()
-	if t.next != "" {
+	for _, p := range paths {
-		names = append(slices.Clip(names), t.next)
+		info, err := os.Lstat(p)
-	}
-	for _, name := range names {
-		info, err := os.Lstat(filepath.Join(t.dir, name))
 		if err != nil {
 			info = nil
 		}
-		check(name, info)
+		check(p, info)
 	}
 	if t.all {
 		entries, _ := os.ReadDir(t.dir)
-		prefix := t.dir + string(filepath.Separator)
 		live := map[string]bool{}
 		for _, e := range entries {
 			if !visible(e.Name()) || e.IsDir() {
 			if err != nil {
 				continue
 			}
-			live[e.Name()] = true
+			key := filepath.Join(t.dir, e.Name())
-			check(e.Name(), info)
+			live[key] = true
+			check(key, info)
 		}
 		for key := range w.stamps {
-			if rest, ok := strings.CutPrefix(key, prefix); ok && !strings.ContainsRune(rest, filepath.Separator) &&
+			if filepath.Dir(key) == t.dir && !live[key] && !slices.Contains(paths, key) {
-				!live[rest] && !slices.Contains(names, rest) {
 				delete(w.stamps, key)
 				changed = true
 			}

M watch_linux.go +14 -5


@@ -58,10 +58,15 @@ func (in inotify) set(targets []target) (missing bool, err error) {
58	58		}
59	59		}
60	60		for dir := range want {
61		-	if _, ok := in.wds[dir]; ok {
62		-	continue
63		-	}
	61	+	// Always add: a directory renamed away and replaced keeps its path
	62	+	// but not its inode, and adding returns the watch of the current
	63	+	// one (the same descriptor when nothing moved).
64	64		wd, err := syscall.InotifyAddWatch(in.fd, dir, inotifyMask)
	65	+	if old, ok := in.wds[dir]; ok && (err != nil \|\| old != int32(wd)) {
	66	+	_, _ = syscall.InotifyRmWatch(in.fd, uint32(old))
	67	+	delete(in.wds, dir)
	68	+	delete(in.dirs, old)
	69	+	}
65	70		if errors.Is(err, syscall.ENOENT) \|\| errors.Is(err, syscall.ENOTDIR) {
66	71		missing = true
67	72		continue

@@ -110,8 +115,12 @@ func (in *inotify) wait(timeout time.Duration, evs []event) ([]event, error) {
110	115		continue // a watch we removed
111	116		}
112	117		if ev.Mask&(syscall.IN_IGNORED\|syscall.IN_DELETE_SELF\|syscall.IN_MOVE_SELF) != 0 {
113		-	if ev.Mask&syscall.IN_IGNORED != 0 {
114		-	delete(in.dirs, ev.Wd)
	118	+	// The watch follows the inode, not the path: drop it.
	119	+	if ev.Mask&syscall.IN_IGNORED == 0 {
	120	+	_, _ = syscall.InotifyRmWatch(in.fd, uint32(ev.Wd))
	121	+	}
	122	+	delete(in.dirs, ev.Wd)
	123	+	if in.wds[dir] == ev.Wd {
115	124		delete(in.wds, dir)
116	125		}
117	126		evs = append(evs, event{dir: dir, lost: true})

 		}
 	}
 	for dir := range want {
-		if _, ok := in.wds[dir]; ok {
+		// Always add: a directory renamed away and replaced keeps its path
-			continue
+		// but not its inode, and adding returns the watch of the current
-		}
+		// one (the same descriptor when nothing moved).
 		wd, err := syscall.InotifyAddWatch(in.fd, dir, inotifyMask)
+		if old, ok := in.wds[dir]; ok && (err != nil || old != int32(wd)) {
+			_, _ = syscall.InotifyRmWatch(in.fd, uint32(old))
+			delete(in.wds, dir)
+			delete(in.dirs, old)
+		}
 		if errors.Is(err, syscall.ENOENT) || errors.Is(err, syscall.ENOTDIR) {
 			missing = true
 			continue
 			continue // a watch we removed
 		}
 		if ev.Mask&(syscall.IN_IGNORED|syscall.IN_DELETE_SELF|syscall.IN_MOVE_SELF) != 0 {
-			if ev.Mask&syscall.IN_IGNORED != 0 {
+			// The watch follows the inode, not the path: drop it.
-				delete(in.dirs, ev.Wd)
+			if ev.Mask&syscall.IN_IGNORED == 0 {
+				_, _ = syscall.InotifyRmWatch(in.fd, uint32(ev.Wd))
+			}
+			delete(in.dirs, ev.Wd)
+			if in.wds[dir] == ev.Wd {
 				delete(in.wds, dir)
 			}
 			evs = append(evs, event{dir: dir, lost: true})

M watch_test.go +79 -0


@@ -184,6 +184,54 @@ func TestWatch(t *testing.T) {
184	184		e.expectQuiet("a write at the old target")
185	185		},
186	186		},
	187	+	{
	188	+	name: "directory renamed away and replaced",
	189	+	rel: "app/config",
	190	+	setup: func(e *watchEnv) { e.write("app/config", "a = 1\n"); e.write("new/config", "a = 2\n") },
	191	+	steps: func(e *watchEnv) {
	192	+	must(e.t, os.Rename(filepath.Join(e.dir, "app"), filepath.Join(e.dir, "app.old")))
	193	+	must(e.t, os.Rename(filepath.Join(e.dir, "new"), filepath.Join(e.dir, "app")))
	194	+	e.expectChange("the directory swap")
	195	+	e.settle()
	196	+	e.write("app/config", "a = 3\n")
	197	+	e.expectChange("a write in the new directory")
	198	+	e.settle()
	199	+	e.write("app.old/config", "a = 4\n")
	200	+	e.expectQuiet("a write in the old directory")
	201	+	},
	202	+	},
	203	+	{
	204	+	name: "symlinked parent directory retargeted",
	205	+	rel: "app/config",
	206	+	setup: func(e *watchEnv) {
	207	+	e.write("one/config", "a = 1\n")
	208	+	e.write("two/config", "a = 2\n")
	209	+	e.symlink("one", "app")
	210	+	},
	211	+	steps: func(e *watchEnv) {
	212	+	e.write("one/config", "a = 3\n")
	213	+	e.expectChange("a write through the linked directory")
	214	+	e.settle()
	215	+	e.symlink("two", "app")
	216	+	e.expectChange("retargeting the directory link")
	217	+	e.settle()
	218	+	e.write("two/config", "a = 4\n")
	219	+	e.expectChange("a write in the new target")
	220	+	e.settle()
	221	+	e.write("one/config", "a = 5\n")
	222	+	e.expectQuiet("a write in the old target")
	223	+	},
	224	+	},
	225	+	{
	226	+	name: "directory mode ignores subdirectories",
	227	+	rel: "scripts",
	228	+	dir: true,
	229	+	setup: func(e *watchEnv) { e.write("scripts/a.sh", "1") },
	230	+	steps: func(e *watchEnv) {
	231	+	must(e.t, os.Mkdir(filepath.Join(e.dir, "scripts", "sub"), 0o700))
	232	+	e.expectQuiet("creating a subdirectory")
	233	+	},
	234	+	},
187	235		{
188	236		name: "directory mode",
189	237		rel: "scripts",

@@ -270,6 +318,37 @@ func TestWatchDebounce(t *testing.T) {
270	318		}
271	319		}
272	320
	321	+	// An atomic save is one change: nothing is reported again when the watcher
	322	+	// looks at the files later.
	323	+	func TestWatchReportsASaveOnce(t *testing.T) {
	324	+	for _, bc := range backends() {
	325	+	t.Run(bc.name, func(t *testing.T) {
	326	+	dir := t.TempDir()
	327	+	path := filepath.Join(dir, "config")
	328	+	must(t, os.WriteFile(path, []byte("a = 1\n"), 0o600))
	329	+	out := make(chan struct{}, 10)
	330	+	ctx, cancel := context.WithCancel(context.Background())
	331	+	defer cancel()
	332	+	ready := make(chan struct{})
	333	+	go func() {
	334	+	_ = Watch(ctx, path, WatchOptions{Poll: bc.poll, Debounce: 20 * time.Millisecond, Ready: ready}, out)
	335	+	}()
	336	+	<-ready
	337	+	must(t, WriteFile(path, []byte("a = 2\n"), 0o600))
	338	+	select {
	339	+	case <-out:
	340	+	case <-time.After(5 * time.Second):
	341	+	t.Fatal("no change reported")
	342	+	}
	343	+	select {
	344	+	case <-out:
	345	+	t.Fatal("one save was reported twice")
	346	+	case <-time.After(retryMissing + 500*time.Millisecond):
	347	+	}
	348	+	})
	349	+	}
	350	+	}
	351	+
273	352		func TestWatchCancelBeforeEvents(t *testing.T) {
274	353		for _, bc := range backends() {
275	354		t.Run(bc.name, func(t *testing.T) {


@@ -184,6 +184,54 @@ func TestWatch(t *testing.T) {
184	e.expectQuiet("a write at the old target")	184		e.expectQuiet("a write at the old target")
185	},	185		},
186	},	186		},
		187	+	{
		188	+	name: "directory renamed away and replaced",
		189	+	rel: "app/config",
		190	+	setup: func(e *watchEnv) { e.write("app/config", "a = 1\n"); e.write("new/config", "a = 2\n") },
		191	+	steps: func(e *watchEnv) {
		192	+	must(e.t, os.Rename(filepath.Join(e.dir, "app"), filepath.Join(e.dir, "app.old")))
		193	+	must(e.t, os.Rename(filepath.Join(e.dir, "new"), filepath.Join(e.dir, "app")))
		194	+	e.expectChange("the directory swap")
		195	+	e.settle()
		196	+	e.write("app/config", "a = 3\n")
		197	+	e.expectChange("a write in the new directory")
		198	+	e.settle()
		199	+	e.write("app.old/config", "a = 4\n")
		200	+	e.expectQuiet("a write in the old directory")
		201	+	},
		202	+	},
		203	+	{
		204	+	name: "symlinked parent directory retargeted",
		205	+	rel: "app/config",
		206	+	setup: func(e *watchEnv) {
		207	+	e.write("one/config", "a = 1\n")
		208	+	e.write("two/config", "a = 2\n")
		209	+	e.symlink("one", "app")
		210	+	},
		211	+	steps: func(e *watchEnv) {
		212	+	e.write("one/config", "a = 3\n")
		213	+	e.expectChange("a write through the linked directory")
		214	+	e.settle()
		215	+	e.symlink("two", "app")
		216	+	e.expectChange("retargeting the directory link")
		217	+	e.settle()
		218	+	e.write("two/config", "a = 4\n")
		219	+	e.expectChange("a write in the new target")
		220	+	e.settle()
		221	+	e.write("one/config", "a = 5\n")
		222	+	e.expectQuiet("a write in the old target")
		223	+	},
		224	+	},
		225	+	{
		226	+	name: "directory mode ignores subdirectories",
		227	+	rel: "scripts",
		228	+	dir: true,
		229	+	setup: func(e *watchEnv) { e.write("scripts/a.sh", "1") },
		230	+	steps: func(e *watchEnv) {
		231	+	must(e.t, os.Mkdir(filepath.Join(e.dir, "scripts", "sub"), 0o700))
		232	+	e.expectQuiet("creating a subdirectory")
		233	+	},
		234	+	},
187	{	235		{
188	name: "directory mode",	236		name: "directory mode",
189	rel: "scripts",	237		rel: "scripts",

@@ -270,6 +318,37 @@ func TestWatchDebounce(t *testing.T) {
270	}	318		}
271	}	319		}
272		320
		321	+	// An atomic save is one change: nothing is reported again when the watcher
		322	+	// looks at the files later.
		323	+	func TestWatchReportsASaveOnce(t *testing.T) {
		324	+	for _, bc := range backends() {
		325	+	t.Run(bc.name, func(t *testing.T) {
		326	+	dir := t.TempDir()
		327	+	path := filepath.Join(dir, "config")
		328	+	must(t, os.WriteFile(path, []byte("a = 1\n"), 0o600))
		329	+	out := make(chan struct{}, 10)
		330	+	ctx, cancel := context.WithCancel(context.Background())
		331	+	defer cancel()
		332	+	ready := make(chan struct{})
		333	+	go func() {
		334	+	_ = Watch(ctx, path, WatchOptions{Poll: bc.poll, Debounce: 20 * time.Millisecond, Ready: ready}, out)
		335	+	}()
		336	+	<-ready
		337	+	must(t, WriteFile(path, []byte("a = 2\n"), 0o600))
		338	+	select {
		339	+	case <-out:
		340	+	case <-time.After(5 * time.Second):
		341	+	t.Fatal("no change reported")
		342	+	}
		343	+	select {
		344	+	case <-out:
		345	+	t.Fatal("one save was reported twice")
		346	+	case <-time.After(retryMissing + 500*time.Millisecond):
		347	+	}
		348	+	})
		349	+	}
		350	+	}
		351	+
273	func TestWatchCancelBeforeEvents(t *testing.T) {	352		func TestWatchCancelBeforeEvents(t *testing.T) {
274	for _, bc := range backends() {	353		for _, bc := range backends() {
275	t.Run(bc.name, func(t *testing.T) {	354		t.Run(bc.name, func(t *testing.T) {

M write.go +18 -6


@@ -2,6 +2,7 @@ package kvconf
2	2
3	3		import (
4	4		"bytes"
	5	+	"fmt"
5	6		"os"
6	7		"path/filepath"
7	8		"runtime"

@@ -14,10 +15,15 @@ import (
14	15		// last one) is replaced; otherwise a commented-out `# key = ...` is enabled;
15	16		// otherwise the key goes after the last key sharing its longest dotted
16	17		// parent, or at the end. A nil value removes every assignment of the key.
17		-	// Keys are applied in sorted order, so the output is deterministic.
18		-	func Set(data []byte, values map[string]*string) []byte {
	18	+	// Keys are applied in sorted order, so the output is deterministic. A key
	19	+	// that cannot be read back as written (empty, with '=' or a line break,
	20	+	// starting with '#') is an error and nothing is changed.
	21	+	func Set(data []byte, values map[string]*string) ([]byte, error) {
19	22		keys := make([]string, 0, len(values))
20	23		for k := range values {
	24	+	if !validKey(k) {
	25	+	return nil, fmt.Errorf("kvconf: invalid key %q", k)
	26	+	}
21	27		keys = append(keys, k)
22	28		}
23	29		slices.Sort(keys)

@@ -28,7 +34,7 @@ func Set(data []byte, values map[string]*string) []byte {
28	34		data = setKey(data, k, *v)
29	35		}
30	36		}
31		-	return data
	37	+	return data, nil
32	38		}
33	39
34	40		// lineKey returns the key of an assignment line, or "" for anything else.

@@ -160,7 +166,10 @@ func WriteFile(path string, data []byte, perm os.FileMode) error {
160	166		if err := tmp.Close(); err != nil {
161	167		return err
162	168		}
163		-	return rename(tmp.Name(), path)
	169	+	if err := rename(tmp.Name(), path); err != nil {
	170	+	return err
	171	+	}
	172	+	return syncDir(filepath.Dir(path))
164	173		}
165	174
166	175		func writeSync(f *os.File, data []byte, perm os.FileMode) error {

@@ -177,9 +186,12 @@ func writeSync(f *os.File, data []byte, perm os.FileMode) error {
177	186		// SetFile applies Set to the file at path and writes it back atomically. A
178	187		// missing file is created with perm.
179	188		func SetFile(path string, values map[string]*string, perm os.FileMode) error {
180		-	data, err := os.ReadFile(path)
	189	+	data, err := ReadFile(path, Options{})
181	190		if err != nil && !isNotExist(err) {
182	191		return err
183	192		}
184		-	return WriteFile(path, Set(data, values), perm)
	193	+	if data, err = Set(data, values); err != nil {
	194	+	return err
	195	+	}
	196	+	return WriteFile(path, data, perm)
185	197		}

 import (
 	"bytes"
+	"fmt"
 	"os"
 	"path/filepath"
 	"runtime"
 // last one) is replaced; otherwise a commented-out `# key = ...` is enabled;
 // otherwise the key goes after the last key sharing its longest dotted
 // parent, or at the end. A nil value removes every assignment of the key.
-// Keys are applied in sorted order, so the output is deterministic.
+// Keys are applied in sorted order, so the output is deterministic. A key
-func Set(data []byte, values map[string]*string) []byte {
+// that cannot be read back as written (empty, with '=' or a line break,
+// starting with '#') is an error and nothing is changed.
+func Set(data []byte, values map[string]*string) ([]byte, error) {
 	keys := make([]string, 0, len(values))
 	for k := range values {
+		if !validKey(k) {
+			return nil, fmt.Errorf("kvconf: invalid key %q", k)
+		}
 		keys = append(keys, k)
 	}
 	slices.Sort(keys)
 			data = setKey(data, k, *v)
 		}
 	}
-	return data
+	return data, nil
 }
 // lineKey returns the key of an assignment line, or "" for anything else.
 	if err := tmp.Close(); err != nil {
 		return err
 	}
-	return rename(tmp.Name(), path)
+	if err := rename(tmp.Name(), path); err != nil {
+		return err
+	}
+	return syncDir(filepath.Dir(path))
 }
 func writeSync(f *os.File, data []byte, perm os.FileMode) error {
 // SetFile applies Set to the file at path and writes it back atomically. A
 // missing file is created with perm.
 func SetFile(path string, values map[string]*string, perm os.FileMode) error {
-	data, err := os.ReadFile(path)
+	data, err := ReadFile(path, Options{})
 	if err != nil && !isNotExist(err) {
 		return err
 	}
-	return WriteFile(path, Set(data, values), perm)
+	if data, err = Set(data, values); err != nil {
+		return err
+	}
+	return WriteFile(path, data, perm)
 }

M write_test.go +48 -2


@@ -71,15 +71,61 @@ func TestSet(t *testing.T) {
71	71		}
72	72		for _, tt := range tests {
73	73		t.Run(tt.name, func(t *testing.T) {
74		-	if got := string(Set([]byte(tt.in), tt.values)); got != tt.want {
	74	+	got, err := Set([]byte(tt.in), tt.values)
	75	+	must(t, err)
	76	+	if string(got) != tt.want {
75	77		t.Errorf("got\n%q\nwant\n%q", got, tt.want)
76	78		}
77	79		})
78	80		}
79	81		}
80	82
	83	+	func TestSetRejectsInjection(t *testing.T) {
	84	+	for _, key := range []string{"", "a = b", "a\nadmin", "bind.q = y\nadmin", "# a", " a"} {
	85	+	t.Run(key, func(t *testing.T) {
	86	+	if _, err := Set(nil, map[string]*string{key: ptr("v")}); err == nil {
	87	+	t.Fatalf("Set accepted key %q", key)
	88	+	}
	89	+	})
	90	+	}
	91	+	}
	92	+
	93	+	func TestMarshalRejectsInjection(t *testing.T) {
	94	+	tests := []struct {
	95	+	name string
	96	+	v any
	97	+	}{
	98	+	{"map name with newline", &marshalConfig{Binds: map[string]string{"x = 1\nadmin": "y"}}},
	99	+	{"map name with equals", &marshalConfig{Binds: map[string]string{"a=b": "y"}}},
	100	+	{"catch-all key", &marshalConfig{Rest: map[string]string{"a\nb": "y"}}},
	101	+	}
	102	+	for _, tt := range tests {
	103	+	t.Run(tt.name, func(t *testing.T) {
	104	+	if _, err := Marshal(tt.v); err == nil {
	105	+	t.Fatal("Marshal accepted an injected key")
	106	+	}
	107	+	})
	108	+	}
	109	+	}
	110	+
	111	+	// upper marshals with a pointer receiver, like many TextMarshalers.
	112	+	type upper string
	113	+
	114	+	func (u upper) UnmarshalText(b []byte) error { u = upper(strings.ToLower(string(b))); return nil }
	115	+	func (u upper) MarshalText() ([]byte, error) { return []byte(strings.ToUpper(string(u))), nil }
	116	+
	117	+	func TestMarshalPointerReceiver(t *testing.T) {
	118	+	type cfg struct {
	119	+	U upper `kv:"u"`
	120	+	M map[string]upper `kv:"m.*"`
	121	+	}
	122	+	data, err := Marshal(&cfg{U: "hello", M: map[string]upper{"a": "x"}})
	123	+	must(t, err)
	124	+	eq(t, string(data), "u = HELLO\nm.a = X\n")
	125	+	}
	126	+
81	127		func TestQuoteRoundTrip(t *testing.T) {
82		-	for _, v := range []string{"", "plain", "#ff0000", "a # b", " lead", "trail ", `"q"`, "it's", "a\nb", `back\slash`, "x\t#y"} {
	128	+	for _, v := range []string{"", "plain", "#ff0000", "a # b", " lead", "trail ", `"q"`, "it's", "a\nb", `back\slash`, "x\t#y", `it's "x # y"`, "don't # x"} {
83	129		t.Run(v, func(t *testing.T) {
84	130		entries, _, err := ParseBytes([]byte("k = "+Quote(v)+"\n"), Options{})
85	131		if err != nil \|\| len(entries) != 1 \|\| entries[0].Value != v {


@@ -71,15 +71,61 @@ func TestSet(t *testing.T) {
71		}	71		}
72		for _, tt := range tests {	72		for _, tt := range tests {
73		t.Run(tt.name, func(t *testing.T) {	73		t.Run(tt.name, func(t *testing.T) {
74	-	if got := string(Set([]byte(tt.in), tt.values)); got != tt.want {	74	+	got, err := Set([]byte(tt.in), tt.values)
			75	+	must(t, err)
			76	+	if string(got) != tt.want {
75		t.Errorf("got\n%q\nwant\n%q", got, tt.want)	77		t.Errorf("got\n%q\nwant\n%q", got, tt.want)
76		}	78		}
77		})	79		})
78		}	80		}
79		}	81		}
80			82
			83	+	func TestSetRejectsInjection(t *testing.T) {
			84	+	for _, key := range []string{"", "a = b", "a\nadmin", "bind.q = y\nadmin", "# a", " a"} {
			85	+	t.Run(key, func(t *testing.T) {
			86	+	if _, err := Set(nil, map[string]*string{key: ptr("v")}); err == nil {
			87	+	t.Fatalf("Set accepted key %q", key)
			88	+	}
			89	+	})
			90	+	}
			91	+	}
			92	+
			93	+	func TestMarshalRejectsInjection(t *testing.T) {
			94	+	tests := []struct {
			95	+	name string
			96	+	v any
			97	+	}{
			98	+	{"map name with newline", &marshalConfig{Binds: map[string]string{"x = 1\nadmin": "y"}}},
			99	+	{"map name with equals", &marshalConfig{Binds: map[string]string{"a=b": "y"}}},
			100	+	{"catch-all key", &marshalConfig{Rest: map[string]string{"a\nb": "y"}}},
			101	+	}
			102	+	for _, tt := range tests {
			103	+	t.Run(tt.name, func(t *testing.T) {
			104	+	if _, err := Marshal(tt.v); err == nil {
			105	+	t.Fatal("Marshal accepted an injected key")
			106	+	}
			107	+	})
			108	+	}
			109	+	}
			110	+
			111	+	// upper marshals with a pointer receiver, like many TextMarshalers.
			112	+	type upper string
			113	+
			114	+	func (u upper) UnmarshalText(b []byte) error { u = upper(strings.ToLower(string(b))); return nil }
			115	+	func (u upper) MarshalText() ([]byte, error) { return []byte(strings.ToUpper(string(u))), nil }
			116	+
			117	+	func TestMarshalPointerReceiver(t *testing.T) {
			118	+	type cfg struct {
			119	+	U upper `kv:"u"`
			120	+	M map[string]upper `kv:"m.*"`
			121	+	}
			122	+	data, err := Marshal(&cfg{U: "hello", M: map[string]upper{"a": "x"}})
			123	+	must(t, err)
			124	+	eq(t, string(data), "u = HELLO\nm.a = X\n")
			125	+	}
			126	+
81		func TestQuoteRoundTrip(t *testing.T) {	127		func TestQuoteRoundTrip(t *testing.T) {
82	-	for _, v := range []string{"", "plain", "#ff0000", "a # b", " lead", "trail ", `"q"`, "it's", "a\nb", `back\slash`, "x\t#y"} {	128	+	for _, v := range []string{"", "plain", "#ff0000", "a # b", " lead", "trail ", `"q"`, "it's", "a\nb", `back\slash`, "x\t#y", `it's "x # y"`, "don't # x"} {
83		t.Run(v, func(t *testing.T) {	129		t.Run(v, func(t *testing.T) {
84		entries, _, err := ParseBytes([]byte("k = "+Quote(v)+"\n"), Options{})	130		entries, _, err := ParseBytes([]byte("k = "+Quote(v)+"\n"), Options{})
85		if err != nil \|\| len(entries) != 1 \|\| entries[0].Value != v {	131		if err != nil \|\| len(entries) != 1 \|\| entries[0].Value != v {