Skip to content

Commit 1865e29

Browse files
committed
9p: all - refactor constructors/remove reflection
This replaces the reflect based field selector/setter, and dynamic function generation/calling - with generic type constraints and option functions which utilize interfaces. The previous code worked, but relied on strings to be used to reference field values that were common across different types (that typically inherited from each other). If a field name or shape of a struct ever changed, this could cause a panic at runtime if the relevant option function's constructor was not also updated. Now, field selection is based around setter methods, and assignment is handled by generic closure generators. If structures change, it shouldn't cause issues, and if it would, the program will fail to compile altogether rather than panic at runtime. The cost is requiring field setters, but seems worth the change as things can be further refactored in the future with more ease (compared to the reflection approach) if better solutions become apparent.
1 parent 96d7d96 commit 1865e29

File tree

11 files changed

+374
-600
lines changed

11 files changed

+374
-600
lines changed

internal/filesystem/9p/chan.go

Lines changed: 38 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -12,48 +12,62 @@ import (
1212
type (
1313
ChannelFile struct {
1414
templatefs.NoopFile
15-
metadata
16-
linkSync *linkSync
17-
emitter *chanEmitter[[]byte]
15+
*metadata
16+
*linkSync
17+
emitter *chanEmitter[[]byte]
1818
openFlags
1919
}
2020
channelSettings struct {
21-
fileOptions
2221
buffer int
2322
}
24-
ChannelOption func(*channelSettings) error
23+
channelFileSettings struct {
24+
fileSettings
25+
channelSettings
26+
}
27+
ChannelOption func(*channelFileSettings) error
28+
channelSetter[T any] interface {
29+
*T
30+
setBuffer(int)
31+
}
2532
)
2633

34+
func (cs *channelSettings) setBuffer(size int) { cs.buffer = size }
35+
2736
func NewChannelFile(ctx context.Context,
2837
options ...ChannelOption,
2938
) (p9.QID, *ChannelFile, <-chan []byte, error) {
30-
var settings channelSettings
31-
if err := parseOptions(&settings, options...); err != nil {
32-
return p9.QID{}, nil, nil, err
33-
}
34-
metadata, err := makeMetadata(p9.ModeRegular, settings.metaOptions...)
35-
if err != nil {
36-
return p9.QID{}, nil, nil, err
37-
}
38-
linkSync, err := newLinkSync(settings.linkOptions...)
39-
if err != nil {
39+
var settings channelFileSettings
40+
settings.metadata.initialize(p9.ModeRegular)
41+
if err := applyOptions(&settings, options...); err != nil {
4042
return p9.QID{}, nil, nil, err
4143
}
4244
var (
43-
emitter = makeChannelEmitter[[]byte](ctx, settings.buffer)
45+
emitter = makeChannelEmitter[[]byte](
46+
ctx,
47+
settings.buffer,
48+
)
4449
bytesChan = emitter.ch
45-
chanFile = &ChannelFile{
46-
metadata: metadata,
47-
linkSync: linkSync,
50+
file = &ChannelFile{
51+
metadata: &settings.metadata,
52+
linkSync: &settings.linkSync,
4853
emitter: emitter,
4954
}
5055
)
51-
metadata.incrementPath()
52-
return *chanFile.QID, chanFile, bytesChan, nil
56+
settings.metadata.fillDefaults()
57+
settings.metadata.incrementPath()
58+
return settings.QID, file, bytesChan, nil
5359
}
5460

55-
func WithBuffer[OT ChannelOptions](size int) OT {
56-
return makeFieldSetter[OT]("buffer", size)
61+
func WithBuffer[
62+
OT optionFunc[T],
63+
T any,
64+
I channelSetter[T],
65+
](size int,
66+
) OT {
67+
return func(channelFile *T) error {
68+
any(channelFile).(I).setBuffer(size)
69+
return nil
70+
}
5771
}
5872

5973
func (cf *ChannelFile) Walk(names []string) ([]p9.QID, p9.File, error) {
@@ -79,7 +93,7 @@ func (cf *ChannelFile) Open(mode p9.OpenFlags) (p9.QID, ioUnit, error) {
7993
return p9.QID{}, 0, perrors.EINVAL
8094
}
8195
cf.openFlags = cf.withOpenedFlag(mode)
82-
return *cf.QID, 0, nil
96+
return cf.QID, 0, nil
8397
}
8498

8599
func (cf *ChannelFile) Close() error {

internal/filesystem/9p/directory.go

Lines changed: 61 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -19,50 +19,80 @@ type (
1919
Directory struct {
2020
templatefs.NoopFile
2121
*fileTableSync
22-
metadata
22+
*metadata
2323
*linkSync
2424
opened,
2525
cleanupElements bool
2626
}
27-
directorySettings struct {
28-
fileOptions
29-
cleanupSelf,
30-
cleanupElements bool
31-
}
32-
DirectoryOption func(*directorySettings) error
33-
ephemeralDir struct {
27+
ephemeralDir struct {
3428
directory
3529
refs *atomic.Uintptr
3630
unlinkOnClose,
3731
unlinking *atomic.Bool
3832
closed bool
3933
}
34+
directorySettings struct {
35+
fileSettings
36+
cleanupSelf,
37+
cleanupElements bool
38+
}
39+
DirectoryOption func(*directorySettings) error
40+
directorySetter[T any] interface {
41+
*T
42+
setCleanupSelf(bool)
43+
setCleanupElements(bool)
44+
}
4045
)
4146

47+
func (ds *directorySettings) setCleanupSelf(b bool) { ds.cleanupSelf = b }
48+
func (ds *directorySettings) setCleanupElements(b bool) { ds.cleanupElements = b }
49+
50+
// UnlinkWhenEmpty causes files to unlink from their parent
51+
// after they are considered empty and the last reference
52+
// held by a Walk has been closed.
53+
func UnlinkWhenEmpty[
54+
OT optionFunc[T],
55+
T any,
56+
I directorySetter[T],
57+
](b bool,
58+
) OT {
59+
return func(settings *T) error {
60+
any(settings).(I).setCleanupSelf(b)
61+
return nil
62+
}
63+
}
64+
65+
// UnlinkEmptyChildren sets [UnlinkWhenEmpty]
66+
// on files created by this file.
67+
func UnlinkEmptyChildren[
68+
OT optionFunc[T],
69+
T any,
70+
I directorySetter[T],
71+
](b bool,
72+
) OT {
73+
return func(settings *T) error {
74+
any(settings).(I).setCleanupElements(b)
75+
return nil
76+
}
77+
}
78+
4279
func NewDirectory(options ...DirectoryOption) (p9.QID, p9.File, error) {
4380
var settings directorySettings
44-
if err := parseOptions(&settings, options...); err != nil {
45-
return p9.QID{}, nil, err
46-
}
47-
metadata, err := makeMetadata(p9.ModeDirectory, settings.metaOptions...)
48-
if err != nil {
81+
settings.metadata.initialize(p9.ModeDirectory)
82+
if err := applyOptions(&settings, options...); err != nil {
4983
return p9.QID{}, nil, err
5084
}
51-
linkSync, err := newLinkSync(settings.linkOptions...)
52-
if err != nil {
53-
return p9.QID{}, nil, err
85+
return newDirectory(&settings)
86+
}
87+
88+
func newDirectory(settings *directorySettings) (p9.QID, p9.File, error) {
89+
var file p9.File = &Directory{
90+
fileTableSync: newFileTable(),
91+
metadata: &settings.metadata,
92+
linkSync: &settings.linkSync,
5493
}
55-
var (
56-
directory = Directory{
57-
fileTableSync: newFileTable(),
58-
metadata: metadata,
59-
linkSync: linkSync,
60-
}
61-
qid = metadata.QID
62-
file p9.File = &directory
63-
)
6494
if settings.cleanupSelf {
65-
if parent := linkSync.parent; parent == nil {
95+
if parent := settings.linkSync.parent; parent == nil {
6696
err := generic.ConstError("cannot unlink self without parent file")
6797
return p9.QID{}, nil, err
6898
}
@@ -73,8 +103,9 @@ func NewDirectory(options ...DirectoryOption) (p9.QID, p9.File, error) {
73103
unlinking: new(atomic.Bool),
74104
}
75105
}
76-
metadata.incrementPath()
77-
return *qid, file, nil
106+
settings.metadata.fillDefaults()
107+
settings.metadata.incrementPath()
108+
return settings.QID, file, nil
78109
}
79110

80111
func (dir *Directory) Walk(names []string) ([]p9.QID, p9.File, error) {
@@ -170,7 +201,7 @@ func (dir *Directory) Open(mode p9.OpenFlags) (p9.QID, ioUnit, error) {
170201
return p9.QID{}, noIOUnit, perrors.EINVAL
171202
}
172203
dir.opened = true
173-
return *dir.QID, noIOUnit, nil
204+
return dir.QID, noIOUnit, nil
174205
}
175206

176207
func (dir *Directory) Link(file p9.File, name string) error {
@@ -200,7 +231,7 @@ func (dir *Directory) Mkdir(name string, permissions p9.FileMode, uid p9.UID, gi
200231
WithParent[DirectoryOption](dir, name),
201232
UnlinkWhenEmpty[DirectoryOption](dir.cleanupElements),
202233
UnlinkEmptyChildren[DirectoryOption](dir.cleanupElements),
203-
WithoutRename[DirectoryOption](dir.linkSync.disabled),
234+
WithoutRename[DirectoryOption](dir.linkSync.renameDisabled),
204235
)
205236
if err == nil {
206237
err = dir.Link(newDir, name)

internal/filesystem/9p/guest.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ type (
1313
makeMountPointFn MakeMountPointFunc
1414
}
1515
guestSettings struct {
16-
directoryOptions []DirectoryOption
16+
directorySettings
1717
}
1818
GuestOption func(*guestSettings) error
1919
// MakeMountPointFunc should handle file creation operations
@@ -27,13 +27,15 @@ type (
2727
}
2828
)
2929

30-
func NewGuestFile(makeMountPointFn MakeMountPointFunc, options ...GuestOption,
30+
func NewGuestFile(makeMountPointFn MakeMountPointFunc,
31+
options ...GuestOption,
3132
) (p9.QID, *GuestFile, error) {
3233
var settings guestSettings
33-
if err := parseOptions(&settings, options...); err != nil {
34+
settings.metadata.initialize(p9.ModeDirectory)
35+
if err := applyOptions(&settings, options...); err != nil {
3436
return p9.QID{}, nil, err
3537
}
36-
qid, directory, err := NewDirectory(settings.directoryOptions...)
38+
qid, directory, err := newDirectory(&settings.directorySettings)
3739
if err != nil {
3840
return p9.QID{}, nil, err
3941
}

internal/filesystem/9p/host.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ type (
1414
makeGuestFn MakeGuestFunc
1515
}
1616
hosterSettings struct {
17-
directoryOptions []DirectoryOption
17+
directorySettings
1818
}
1919
HosterOption func(*hosterSettings) error
2020
// MakeGuestFunc should handle file creation operations
@@ -29,10 +29,11 @@ func NewHostFile(makeGuestFn MakeGuestFunc,
2929
options ...HosterOption,
3030
) (p9.QID, *HostFile, error) {
3131
var settings hosterSettings
32-
if err := parseOptions(&settings, options...); err != nil {
32+
settings.metadata.initialize(p9.ModeDirectory)
33+
if err := applyOptions(&settings, options...); err != nil {
3334
return p9.QID{}, nil, err
3435
}
35-
qid, directory, err := NewDirectory(settings.directoryOptions...)
36+
qid, directory, err := newDirectory(&settings.directorySettings)
3637
if err != nil {
3738
return p9.QID{}, nil, err
3839
}

0 commit comments

Comments
 (0)