From c4d09ce5f7665976a405f4a3facf8126a6e7ce0c Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 20 Jul 2016 21:18:27 -0400 Subject: [PATCH] vis: refactored code to reflect using vis/unvis for file names Added some more test cases for `vis`ing and `unvis`ing strings, and a test case that walks/checks a directory with filenames that require encoding. Had to change Path() and String() to account for possible errors Vis() and Unvis() could return. Signed-off-by: Stephen Chung --- check.go | 12 ++++++++---- check_test.go | 35 +++++++++++++++++++++++++++++++++++ hierarchy.go | 39 +++++++++++++++++++++++++++------------ unvis.go | 22 +++++++++++++++------- vis.go | 26 +++++++++++++++++++------- vis_test.go | 16 +++++++++++++++- walk.go | 13 ++++++++++--- 7 files changed, 129 insertions(+), 34 deletions(-) diff --git a/check.go b/check.go index d27b50f..5c17a88 100644 --- a/check.go +++ b/check.go @@ -50,7 +50,11 @@ func Check(root string, dh *DirectoryHierarchy, keywords []string) (*Result, err creator.curSet = nil } case RelativeType, FullType: - info, err := os.Lstat(e.Path()) + pathname, err := e.Path() + if err != nil { + return nil, err + } + info, err := os.Lstat(pathname) if err != nil { return nil, err } @@ -65,17 +69,17 @@ func Check(root string, dh *DirectoryHierarchy, keywords []string) (*Result, err for _, kv := range kvs { keywordFunc, ok := KeywordFuncs[kv.Keyword()] if !ok { - return nil, fmt.Errorf("Unknown keyword %q for file %q", kv.Keyword(), e.Path()) + return nil, fmt.Errorf("Unknown keyword %q for file %q", kv.Keyword(), pathname) } if keywords != nil && !inSlice(kv.Keyword(), keywords) { continue } - curKeyVal, err := keywordFunc(filepath.Join(root, e.Path()), info) + curKeyVal, err := keywordFunc(filepath.Join(root, pathname), info) if err != nil { return nil, err } if string(kv) != curKeyVal { - failure := Failure{Path: e.Path(), Keyword: kv.Keyword(), Expected: kv.Value(), Got: KeyVal(curKeyVal).Value()} + failure := Failure{Path: pathname, Keyword: kv.Keyword(), Expected: kv.Value(), Got: KeyVal(curKeyVal).Value()} result.Failures = append(result.Failures, failure) } } diff --git a/check_test.go b/check_test.go index d1ad6de..9686bec 100644 --- a/check_test.go +++ b/check_test.go @@ -208,3 +208,38 @@ func TestIgnoreComments(t *testing.T) { t.Fatal(res.Failures) } } + +func TestCheckNeedsEncoding(t *testing.T) { + dir, err := ioutil.TempDir("", "test-needs-encoding") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(dir) + + fh, err := os.Create(filepath.Join(dir, "file[ ")) + if err != nil { + t.Fatal(err) + } + if err := fh.Close(); err != nil { + t.Error(err) + } + fh, err = os.Create(filepath.Join(dir, " , should work")) + if err != nil { + t.Fatal(err) + } + if err := fh.Close(); err != nil { + t.Error(err) + } + + dh, err := Walk(dir, nil, DefaultKeywords) + if err != nil { + t.Fatal(err) + } + res, err := Check(dir, dh, nil) + if err != nil { + t.Fatal(err) + } + if len(res.Failures) > 0 { + t.Fatal(res.Failures) + } +} diff --git a/hierarchy.go b/hierarchy.go index b591773..e6e9145 100644 --- a/hierarchy.go +++ b/hierarchy.go @@ -19,7 +19,11 @@ func (dh DirectoryHierarchy) WriteTo(w io.Writer) (n int64, err error) { sort.Sort(byPos(dh.Entries)) var sum int64 for _, e := range dh.Entries { - i, err := io.WriteString(w, e.String()+"\n") + str, err := e.String() + if err != nil { + return sum, err + } + i, err := io.WriteString(w, str+"\n") if err != nil { return sum, err } @@ -47,28 +51,39 @@ type Entry struct { } // Path provides the full path of the file, despite RelativeType or FullType -func (e Entry) Path() string { - if e.Parent == nil || e.Type == FullType { - return filepath.Clean(e.Name) +func (e Entry) Path() (string, error) { + decodedName, err := Unvis(e.Name) + if err != nil { + return "", err } - return filepath.Clean(filepath.Join(e.Parent.Path(), e.Name)) + if e.Parent == nil || e.Type == FullType { + return filepath.Clean(decodedName), nil + } + parentName, err := e.Parent.Path() + if err != nil { + return "", err + } + return filepath.Clean(filepath.Join(parentName, decodedName)), nil } -func (e Entry) String() string { +func (e Entry) String() (string, error) { if e.Raw != "" { - return e.Raw + return e.Raw, nil } if e.Type == BlankType { - return "" + return "", nil } if e.Type == DotDotType { - return e.Name + return e.Name, nil + } + decodedName, err := Unvis(e.Name) + if err != nil { + return "", err } - // TODO(vbatts) if type is RelativeType and a keyword of not type=dir if e.Type == SpecialType || e.Type == FullType || inSlice("type=dir", e.Keywords) { - return fmt.Sprintf("%s %s", e.Name, strings.Join(e.Keywords, " ")) + return fmt.Sprintf("%s %s", decodedName, strings.Join(e.Keywords, " ")), nil } - return fmt.Sprintf(" %s %s", e.Name, strings.Join(e.Keywords, " ")) + return fmt.Sprintf(" %s %s", decodedName, strings.Join(e.Keywords, " ")), nil } // EntryType are the formats of lines in an mtree spec file diff --git a/unvis.go b/unvis.go index 74d5ad1..809cf61 100644 --- a/unvis.go +++ b/unvis.go @@ -1,15 +1,23 @@ package mtree // #include "vis.h" +// #include import "C" -import "fmt" +import ( + "fmt" + "unsafe" +) -func Unvis(str string) (string, error) { - dst := new(C.char) - ret := C.strunvis(dst, C.CString(str)) - if ret == 0 { - return "", fmt.Errorf("failed to encode string") +// Unvis is a wrapper for the C implementation of unvis, which decodes a string +// that potentially has characters that are encoded with Vis +func Unvis(src string) (string, error) { + cDst, cSrc := C.CString(string(make([]byte, len(src)+1))), C.CString(src) + defer C.free(unsafe.Pointer(cDst)) + defer C.free(unsafe.Pointer(cSrc)) + ret := C.strunvis(cDst, cSrc) + if ret == -1 { + return "", fmt.Errorf("failed to decode: %q", src) } - return C.GoString(dst), nil + return C.GoString(cDst), nil } diff --git a/vis.go b/vis.go index 9da8545..75b9d79 100644 --- a/vis.go +++ b/vis.go @@ -1,14 +1,26 @@ package mtree // #include "vis.h" +// #include import "C" -import "fmt" +import ( + "fmt" + "math" + "unsafe" +) -func Vis(str string) (string, error) { - dst := new(C.char) - ret := C.strvis(dst, C.CString(str), C.VIS_WHITE|C.VIS_OCTAL|C.VIS_GLOB) - if ret == 0 { - return "", fmt.Errorf("failed to encode string") +// Vis is a wrapper of the C implementation of the function vis, which encodes +// a character with a particular format/style +func Vis(src string) (string, error) { + // dst needs to be 4 times the length of str, must check appropriate size + if uint32(len(src)*4+1) >= math.MaxUint32/4 { + return "", fmt.Errorf("failed to encode: %q", src) } - return C.GoString(dst), nil + dst := string(make([]byte, 4*len(src)+1)) + cDst, cSrc := C.CString(dst), C.CString(src) + defer C.free(unsafe.Pointer(cDst)) + defer C.free(unsafe.Pointer(cSrc)) + C.strvis(cDst, cSrc, C.VIS_WHITE|C.VIS_OCTAL|C.VIS_GLOB) + + return C.GoString(cDst), nil } diff --git a/vis_test.go b/vis_test.go index bdcd1b3..eb78f33 100644 --- a/vis_test.go +++ b/vis_test.go @@ -9,13 +9,17 @@ func TestVis(t *testing.T) { {"[", "\\133"}, {" ", "\\040"}, {" ", "\\011"}, + {"dir with space", "dir\\040with\\040space"}, + {"consec spaces", "consec\\040\\040\\040spaces"}, + {"trailingsymbol[", "trailingsymbol\\133"}, + {" [ leadingsymbols", "\\040\\133\\040leadingsymbols"}, + {"no_need_for_encoding", "no_need_for_encoding"}, } for i := range testset { got, err := Vis(testset[i].Src) if err != nil { t.Errorf("working with %q: %s", testset[i].Src, err) - continue } if got != testset[i].Dest { t.Errorf("expected %#v; got %#v", testset[i].Dest, got) @@ -33,3 +37,13 @@ func TestVis(t *testing.T) { } } } + +// The resulting string of Vis output could potentially be four times longer than +// the original. Vis must handle this possibility. +func TestVisLength(t *testing.T) { + testString := "All work and no play makes Jack a dull boy\n" + for i := 0; i < 20; i++ { + Vis(testString) + testString = testString + testString + } +} diff --git a/walk.go b/walk.go index b6c4577..f0dca12 100644 --- a/walk.go +++ b/walk.go @@ -53,9 +53,13 @@ func Walk(root string, exlcudes []ExcludeFunc, keywords []string) (*DirectoryHie // Insert a comment of the full path of the directory's name if creator.curDir != nil { + dirname, err := creator.curDir.Path() + if err != nil { + return err + } creator.DH.Entries = append(creator.DH.Entries, Entry{ Pos: len(creator.DH.Entries), - Raw: "# " + filepath.Join(creator.curDir.Path(), entryPathName), + Raw: "# " + filepath.Join(dirname, entryPathName), Type: CommentType, }) } else { @@ -113,9 +117,12 @@ func Walk(root string, exlcudes []ExcludeFunc, keywords []string) (*DirectoryHie } } } - + encodedEntryName, err := Vis(entryPathName) + if err != nil { + return err + } e := Entry{ - Name: entryPathName, + Name: encodedEntryName, Pos: len(creator.DH.Entries), Type: RelativeType, Set: creator.curSet,