commit fb6882101d9a6e202aa6da50862040ad65e6698f
parent 826c8bc4edcfdc7ec551274af4d0b4f34f1f3d62
Author: Matsuda Kenji <info@mtkn.jp>
Date: Mon, 18 Dec 2023 10:01:08 +0900
fix fd leak
Diffstat:
| M | server.go | | | 145 | +++++++++++++++++++++++++++++++++++++++++-------------------------------------- |
| M | uid.go | | | 8 | +------- |
2 files changed, 77 insertions(+), 76 deletions(-)
diff --git a/server.go b/server.go
@@ -406,7 +406,7 @@ func sOpen(ctx context.Context, s *Server, r *Req) {
// See open(5).
// In plan9 implementation, ifcall.Mode() is ANDed with ^ORCLOSE,
// but ORCLOSE is also prohibitted by the protocol...
- st, err := r.Fid.File.Stat()
+ st, err := fs.Stat(ExportFS{s.fs}, r.Fid.path)
if err != nil {
Respond(ctx, r, fmt.Errorf("stat: %v", err))
return
@@ -436,19 +436,18 @@ func sOpen(ctx context.Context, s *Server, r *Req) {
Respond(ctx, r, ErrPerm)
return
}
- if !hasPerm(r.Fid.File, r.Fid.Uid, p) {
+ if !hasPerm(st, r.Fid.Uid, p) {
Respond(ctx, r, ErrPerm)
return
}
if ifcall.Mode&ORCLOSE != 0 {
parentPath := path.Dir(r.Fid.path)
- parent, err := s.fs.OpenFile(parentPath, OREAD)
- defer parent.Close()
+ st, err := fs.Stat(ExportFS{s.fs}, parentPath)
if err != nil {
- Respond(ctx, r, fmt.Errorf("open parent"))
+ Respond(ctx, r, fmt.Errorf("stat parent: %v"))
return
}
- if !hasPerm(parent, r.Fid.Uid, AWRITE) {
+ if !hasPerm(st, r.Fid.Uid, AWRITE) {
Respond(ctx, r, ErrPerm)
return
}
@@ -465,7 +464,6 @@ func rOpen(r *Req, err error) {
setError(r, err)
return
}
- r.Fid.File.Close()
r.Fid.OMode = r.Ifcall.(*TOpen).Mode
f, err := r.Srv.fs.OpenFile(r.Fid.path, r.Fid.OMode)
if err != nil {
@@ -483,8 +481,7 @@ func sCreate(ctx context.Context, s *Server, r *Req) {
Respond(ctx, r, ErrUnknownFid)
return
}
- dir := r.Fid.File
- dirstat, err := dir.Stat()
+ dirstat, err := fs.Stat(ExportFS{s.fs}, r.Fid.path)
if err != nil {
Respond(ctx, r, fmt.Errorf("stat: %v", err))
return
@@ -493,11 +490,11 @@ func sCreate(ctx context.Context, s *Server, r *Req) {
Respond(ctx, r, fmt.Errorf("create in non-dir"))
return
}
- if !hasPerm(dir, r.Fid.Uid, AWRITE) {
+ if !hasPerm(dirstat, r.Fid.Uid, AWRITE) {
Respond(ctx, r, ErrPerm)
return
}
- cffs, ok := s.fs.(CreaterFS)
+ cfs, ok := s.fs.(CreaterFS)
if !ok {
Respond(ctx, r, ErrOperation)
return
@@ -510,14 +507,14 @@ func sCreate(ctx context.Context, s *Server, r *Req) {
perm &= ^FileMode(0777) | (dirperm & FileMode(0777))
}
cpath := path.Join(r.Fid.path, ifcall.Name)
- file, err := cffs.Create(cpath, r.Fid.Uid, ifcall.Mode, perm)
+ file, err := cfs.Create(cpath, r.Fid.Uid, ifcall.Mode, perm)
if err != nil {
Respond(ctx, r, fmt.Errorf("create: %v", err))
return
}
r.Fid.File = file
- r.Fid.path = path.Join(r.Fid.path, ifcall.Name)
- r.Fid.OMode = r.Ifcall.(*TCreate).Mode
+ r.Fid.path = cpath
+ r.Fid.OMode = ifcall.Mode
st, err := r.Fid.File.Stat()
if err != nil {
Respond(ctx, r, fmt.Errorf("stat: %v", err))
@@ -553,33 +550,33 @@ func sRead(ctx context.Context, s *Server, r *Req) {
Respond(ctx, r, ErrPerm)
return
}
- data := make([]byte, ifcall.Count)
- done := make(chan struct{})
- var (
- n int
- err error
- wg sync.WaitGroup
- )
fi, err := r.Fid.File.Stat()
if err != nil {
log.Printf("Stat: %v", err)
Respond(ctx, r, fmt.Errorf("internal error"))
return
}
+ data := make([]byte, ifcall.Count)
+ errc := make(chan error)
+ var (
+ n int
+ wg sync.WaitGroup
+ )
wg.Add(1)
go func() {
go func() {
wg.Wait()
- close(done)
+ close(errc)
}()
defer wg.Done()
if fi.IsDir() {
- children, err := getChildren(s.fs, r.Fid.path)
- if err != nil {
- log.Printf("get children: %v", err)
- }
if ifcall.Offset != 0 && ifcall.Offset != r.Fid.dirOffset {
- Respond(ctx, r, fmt.Errorf("invalid dir offset"))
+ errc <- fmt.Errorf("invalid dir offset")
+ return
+ }
+ children, err := fs.Glob(ExportFS{s.fs}, path.Join(r.Fid.path, "*"))
+ if err != nil {
+ errc <- fmt.Errorf("glob children: %v", err)
return
}
if ifcall.Offset == 0 {
@@ -588,17 +585,11 @@ func sRead(ctx context.Context, s *Server, r *Req) {
}
k := r.Fid.dirIndex
for ; k < len(children); k++ {
- if children[k] == nil {
- continue
- }
- fi, err := children[k].Stat()
+ fi, err := fs.Stat(ExportFS{s.fs}, children[k])
if err != nil {
log.Printf("stat: %v", err)
continue
}
- if err := children[k].Close(); err != nil {
- log.Printf("close children: %v", err)
- }
st := fi.Sys().(*Stat)
buf := st.marshal()
if n+len(buf) > len(data) {
@@ -612,21 +603,26 @@ func sRead(ctx context.Context, s *Server, r *Req) {
r.Fid.dirOffset += uint64(n)
r.Fid.dirIndex = k
} else {
+ var err error
if reader, ok := r.Fid.File.(io.ReaderAt); ok {
n, err = reader.ReadAt(data, int64(ifcall.Offset))
} else {
n, err = r.Fid.File.Read(data)
}
if err != io.EOF && err != nil {
- log.Printf("sRead: %v\n", err)
- Respond(ctx, r, err)
+ errc <- err
return
}
}
}()
select {
- case <-done:
+ case err := <-errc:
+ if err != nil {
+ Respond(ctx, r, err)
+ return
+ }
case <-ctx.Done():
+ return
}
r.Ofcall = &RRead{
Count: uint32(n),
@@ -634,6 +630,7 @@ func sRead(ctx context.Context, s *Server, r *Req) {
}
Respond(ctx, r, nil)
}
+
func rRead(r *Req, err error) {
if err != nil {
setError(r, err)
@@ -659,37 +656,42 @@ func sWrite(ctx context.Context, s *Server, r *Req) {
}
ofcall := new(RWrite)
var wg sync.WaitGroup
- done := make(chan struct{})
- wg.Add(1)
+ errc := make(chan error)
+ wg.Add(1) // TODO: I think this is not needed. Same in sRead.
go func() {
- go func() {
+ go func() { // TODO: I think this goroutin is not needed
wg.Wait()
- close(done)
+ close(errc)
}()
+ defer wg.Done() // TODO: I think just "defer close(errc)" suffices...
switch file := r.Fid.File.(type) {
case io.WriterAt:
n, err := file.WriteAt(ifcall.Data, int64(ifcall.Offset))
if err != nil {
- Respond(ctx, r, fmt.Errorf("write: %v", err))
+ errc <- fmt.Errorf("write: %v", err)
return
}
ofcall.Count = uint32(n)
case io.Writer:
n, err := file.Write(ifcall.Data)
if err != nil {
- Respond(ctx, r, fmt.Errorf("write: %v", err))
+ errc <- fmt.Errorf("write: %v", err)
return
}
ofcall.Count = uint32(n)
default:
- Respond(ctx, r, ErrOperation)
+ errc <- ErrOperation
return
}
- wg.Done()
}()
select {
- case <-done:
+ case err := <-errc:
+ if err != nil {
+ Respond(ctx, r, err)
+ return
+ }
case <-ctx.Done():
+ return
}
r.Ofcall = ofcall
Respond(ctx, r, nil)
@@ -711,8 +713,7 @@ func sClunk(ctx context.Context, s *Server, r *Req) {
return
}
s.fPool.delete(ifcall.Fid)
- //if fid.OMode != -1 {
- if fid.File != nil {
+ if fid.OMode != -1 {
if err := fid.File.Close(); err != nil {
Respond(ctx, r, fmt.Errorf("close: %v", err))
return
@@ -737,18 +738,20 @@ func sRemove(ctx context.Context, s *Server, r *Req) {
Respond(ctx, r, ErrUnknownFid)
return
}
- defer s.fPool.delete(ifcall.Fid)
+ s.fPool.delete(ifcall.Fid)
+ if r.Fid.OMode != -1 {
+ r.Fid.File.Close()
+ }
parentPath := path.Dir(r.Fid.path)
- parent, err := s.fs.OpenFile(parentPath, OREAD)
+ pstat, err := fs.Stat(ExportFS{s.fs}, parentPath)
if err != nil {
- Respond(ctx, r, fmt.Errorf("open parent: %v", err))
+ Respond(ctx, r, fmt.Errorf("stat parent: %v", err))
return
}
- if !hasPerm(parent, r.Fid.Uid, AWRITE) {
+ if !hasPerm(pstat, r.Fid.Uid, AWRITE) {
Respond(ctx, r, ErrPerm)
return
}
- parent.Close()
rfs, ok := s.fs.(RemoverFS)
if !ok {
Respond(ctx, r, ErrOperation)
@@ -761,6 +764,7 @@ func sRemove(ctx context.Context, s *Server, r *Req) {
r.Ofcall = &RRemove{}
Respond(ctx, r, nil)
}
+
func rRemove(r *Req, err error) {
if err != nil {
setError(r, err)
@@ -775,14 +779,13 @@ func sStat(ctx context.Context, s *Server, r *Req) {
Respond(ctx, r, ErrUnknownFid)
return
}
- fileInfo, err := r.Fid.File.Stat()
+ fi, err := fs.Stat(ExportFS{s.fs}, r.Fid.path)
if err != nil {
- log.Printf("stat %v: %v", r.Fid.File, err)
- Respond(ctx, r, fmt.Errorf("internal error"))
+ Respond(ctx, r, fmt.Errorf("stat: %v", err))
return
}
r.Ofcall = &RStat{
- Stat: fileInfo.Sys().(*Stat),
+ Stat: fi.Sys().(*Stat),
}
Respond(ctx, r, nil)
}
@@ -801,6 +804,15 @@ func sWStat(ctx context.Context, s *Server, r *Req) {
Respond(ctx, r, ErrUnknownFid)
return
}
+ if r.Fid.OMode == -1 {
+ var err error
+ r.Fid.File, err = s.fs.OpenFile(r.Fid.path, OREAD)
+ if err != nil {
+ Respond(ctx, r, fmt.Errorf("open: %v", err))
+ return
+ }
+ defer r.Fid.File.Close()
+ }
wsfile, ok := r.Fid.File.(WriterStatFile)
if !ok {
Respond(ctx, r, ErrOperation)
@@ -826,12 +838,12 @@ func sWStat(ctx context.Context, s *Server, r *Req) {
}
if wstat.Name != "" && newStat.Name != wstat.Name {
parentPath := path.Dir(r.Fid.path)
- parent, err := s.fs.OpenFile(parentPath, OREAD)
+ pstat, err := fs.Stat(ExportFS{s.fs}, parentPath)
if err != nil {
- Respond(ctx, r, fmt.Errorf("get parent: %v", err))
+ Respond(ctx, r, fmt.Errorf("stat parent: %v", err))
return
}
- if !hasPerm(parent, r.Fid.Uid, AWRITE) {
+ if !hasPerm(pstat, r.Fid.Uid, AWRITE) {
Respond(ctx, r, ErrPerm)
return
}
@@ -841,18 +853,13 @@ func sWStat(ctx context.Context, s *Server, r *Req) {
// an existing file.
// but 9pfs, 9pfuse does the rename when used with `git init`.
/*
- children, err := getChildren(s.fs, parentPath)
+ children, err := fs.Glob(ExportFS{s.fs}, path.Join(parentPath, "*"))
if err != nil {
- Respond(ctx, r, fmt.Errorf("get children: %v", err))
+ Respond(ctx, r, fmt.Errorf("glob children: %v", err))
return
}
for _, f := range children {
- s, err := f.Stat()
- if err != nil {
- Respond(ctx, r, fmt.Errorf("stat: %v", err))
- return
- }
- if s.Name() == wstat.Name {
+ if path.Base(f) == wstat.Name {
Respond(ctx, r, fmt.Errorf("file already exists"))
return
}
@@ -861,7 +868,7 @@ func sWStat(ctx context.Context, s *Server, r *Req) {
newStat.Name = wstat.Name
}
if wstat.Length != ^int64(0) && wstat.Length != newStat.Length {
- if fi.IsDir() || !hasPerm(r.Fid.File, r.Fid.Uid, AWRITE) {
+ if fi.IsDir() || !hasPerm(fi, r.Fid.Uid, AWRITE) {
Respond(ctx, r, ErrPerm)
return
}
diff --git a/uid.go b/uid.go
@@ -2,18 +2,12 @@ package lib9p
import (
"io/fs"
- "log"
)
// hasPerm reports whether the user uid have the permission p on the File f.
// p is logical OR of AREAD, AWRITE, AEXEC.
// TODO: user is assumed to be the leader of the group
-func hasPerm(f File, uid string, p fs.FileMode) bool {
- fi, err := f.Stat()
- if err != nil {
- log.Printf("stat: %v", err)
- return false
- }
+func hasPerm(fi fs.FileInfo, uid string, p fs.FileMode) bool {
fp := fi.Mode().Perm()
stat := fi.Sys().(*Stat)
m := fp & 7 // other
