Move Request.handle to *Request

Updates pkg#192

Move handle() onto *Request, this has flow on effects to the interface
types declared in request-interface.go, the examples and the tests.

The decision to address pkg#192 started with advice from gopl.io, but I
think has uncovered several bugs as many of the request methods operate
on copies of data stored in the RequestServer's cache. I think this is
unintentional, but may point to some correctness issues, see pkg#193.

request-example.go

@@ -25,7 +25,7 @@ func InMemHandler() Handlers {
 }
 
 // Handlers
-func (fs *root) Fileread(r Request) (io.ReaderAt, error) {
+func (fs *root) Fileread(r *Request) (io.ReaderAt, error) {
 	fs.filesLock.Lock()
 	defer fs.filesLock.Unlock()
 	file, err := fs.fetch(r.Filepath)
@@ -41,7 +41,7 @@ func (fs *root) Fileread(r Request) (io.ReaderAt, error) {
 	return file.ReaderAt()
 }
 
-func (fs *root) Filewrite(r Request) (io.WriterAt, error) {
+func (fs *root) Filewrite(r *Request) (io.WriterAt, error) {
 	fs.filesLock.Lock()
 	defer fs.filesLock.Unlock()
 	file, err := fs.fetch(r.Filepath)
@@ -59,7 +59,7 @@ func (fs *root) Filewrite(r Request) (io.WriterAt, error) {
 	return file.WriterAt()
 }
 
-func (fs *root) Filecmd(r Request) error {
+func (fs *root) Filecmd(r *Request) error {
 	fs.filesLock.Lock()
 	defer fs.filesLock.Unlock()
 	switch r.Method {
@@ -115,7 +115,7 @@ func (f listerat) ListAt(ls []os.FileInfo, offset int64) (int, error) {
 	return n, nil
 }
 
-func (fs *root) Filelist(r Request) (ListerAt, error) {
+func (fs *root) Filelist(r *Request) (ListerAt, error) {
 	fs.filesLock.Lock()
 	defer fs.filesLock.Unlock()
 

request-interfaces.go

@@ -10,22 +10,22 @@ import (
 
 // FileReader should return an io.Reader for the filepath
 type FileReader interface {
-	Fileread(Request) (io.ReaderAt, error)
+	Fileread(*Request) (io.ReaderAt, error)
 }
 
 // FileWriter should return an io.Writer for the filepath
 type FileWriter interface {
-	Filewrite(Request) (io.WriterAt, error)
+	Filewrite(*Request) (io.WriterAt, error)
 }
 
 // FileCmder should return an error (rename, remove, setstate, etc.)
 type FileCmder interface {
-	Filecmd(Request) error
+	Filecmd(*Request) error
 }
 
 // FileLister should return file info interface and errors (readdir, stat)
 type FileLister interface {
-	Filelist(Request) (ListerAt, error)
+	Filelist(*Request) (ListerAt, error)
 }
 
 // ListerAt does for file lists what io.ReaderAt does for files.

request.go

@@ -152,26 +152,23 @@ func (r *Request) popPacket() packet_data {
 }
 
 // called from worker to handle packet/request
-func (r Request) handle(handlers Handlers) (responsePacket, error) {
-	var err error
-	var rpkt responsePacket
+func (r *Request) handle(handlers Handlers) (responsePacket, error) {
 	switch r.Method {
 	case "Get":
-		rpkt, err = fileget(handlers.FileGet, r)
+		return fileget(handlers.FileGet, r)
 	case "Put": // add "Append" to this to handle append only file writes
-		rpkt, err = fileput(handlers.FilePut, r)
+		return fileput(handlers.FilePut, r)
 	case "Setstat", "Rename", "Rmdir", "Mkdir", "Symlink", "Remove":
-		rpkt, err = filecmd(handlers.FileCmd, r)
+		return filecmd(handlers.FileCmd, r)
 	case "List", "Stat", "Readlink":
-		rpkt, err = filelist(handlers.FileList, r)
+		return filelist(handlers.FileList, r)
 	default:
-		return rpkt, errors.Errorf("unexpected method: %s", r.Method)
+		return nil, errors.Errorf("unexpected method: %s", r.Method)
 	}
-	return rpkt, err
 }
 
 // wrap FileReader handler
-func fileget(h FileReader, r Request) (responsePacket, error) {
+func fileget(h FileReader, r *Request) (responsePacket, error) {
 	var err error
 	reader := r.getReader()
 	if reader == nil {
@@ -197,7 +194,7 @@ func fileget(h FileReader, r Request) (responsePacket, error) {
 }
 
 // wrap FileWriter handler
-func fileput(h FileWriter, r Request) (responsePacket, error) {
+func fileput(h FileWriter, r *Request) (responsePacket, error) {
 	var err error
 	writer := r.getWriter()
 	if writer == nil {
@@ -217,11 +214,12 @@ func fileput(h FileWriter, r Request) (responsePacket, error) {
 		ID: pd.id,
 		StatusError: StatusError{
 			Code: ssh_FX_OK,
-		}}, nil
+		},
+	}, nil
 }
 
 // wrap FileCmder handler
-func filecmd(h FileCmder, r Request) (responsePacket, error) {
+func filecmd(h FileCmder, r *Request) (responsePacket, error) {
 	err := h.Filecmd(r)
 	if err != nil {
 		return nil, err
@@ -230,11 +228,12 @@ func filecmd(h FileCmder, r Request) (responsePacket, error) {
 		ID: r.pkt_id,
 		StatusError: StatusError{
 			Code: ssh_FX_OK,
-		}}, nil
+		},
+	}, nil
 }
 
 // wrap FileLister handler
-func filelist(h FileLister, r Request) (responsePacket, error) {
+func filelist(h FileLister, r *Request) (responsePacket, error) {
 	var err error
 	lister := r.getLister()
 	if lister == nil {

request_test.go

@@ -18,28 +18,25 @@ type testHandler struct {
 	err          error       // dummy error, should be file related
 }
 
-func (t *testHandler) Fileread(r Request) (io.ReaderAt, error) {
+func (t *testHandler) Fileread(r *Request) (io.ReaderAt, error) {
 	if t.err != nil {
 		return nil, t.err
 	}
 	return bytes.NewReader(t.filecontents), nil
 }
 
-func (t *testHandler) Filewrite(r Request) (io.WriterAt, error) {
+func (t *testHandler) Filewrite(r *Request) (io.WriterAt, error) {
 	if t.err != nil {
 		return nil, t.err
 	}
 	return io.WriterAt(t.output), nil
 }
 
-func (t *testHandler) Filecmd(r Request) error {
-	if t.err != nil {
-		return t.err
-	}
-	return nil
+func (t *testHandler) Filecmd(r *Request) error {
+	return t.err
 }
 
-func (t *testHandler) Filelist(r Request) (ListerAt, error) {
+func (t *testHandler) Filelist(r *Request) (ListerAt, error) {
 	if t.err != nil {
 		return nil, t.err
 	}