From ee7e6d33b2a017a53bebfdc55d182f53474d4d7d Mon Sep 17 00:00:00 2001 From: Nicolas Lacasse Date: Fri, 5 Apr 2019 15:48:26 -0700 Subject: Use string type for extended attribute values, instead of []byte. Strings are a better fit for this usage because they are immutable in Go, and can contain arbitrary bytes. It also allows us to avoid casting bytes to string (and the associated allocation) in the hot path when checking for overlay whiteouts. PiperOrigin-RevId: 242208856 Change-Id: I7699ae6302492eca71787dd0b72e0a5a217a3db2 --- pkg/sentry/fs/fsutil/inode.go | 16 ++++++++-------- pkg/sentry/fs/inode.go | 2 +- pkg/sentry/fs/inode_operations.go | 4 ++-- pkg/sentry/fs/inode_overlay.go | 18 +++++++++--------- pkg/sentry/fs/inode_overlay_test.go | 6 +++--- pkg/sentry/fs/tmpfs/tmpfs.go | 4 ++-- 6 files changed, 25 insertions(+), 25 deletions(-) (limited to 'pkg') diff --git a/pkg/sentry/fs/fsutil/inode.go b/pkg/sentry/fs/fsutil/inode.go index c1ad45e52..2673d73d7 100644 --- a/pkg/sentry/fs/fsutil/inode.go +++ b/pkg/sentry/fs/fsutil/inode.go @@ -197,25 +197,25 @@ func (i *InodeSimpleAttributes) NotifyStatusChange(ctx context.Context) { type InodeSimpleExtendedAttributes struct { // mu protects xattrs. mu sync.RWMutex `state:"nosave"` - xattrs map[string][]byte + xattrs map[string]string } // Getxattr implements fs.InodeOperations.Getxattr. -func (i *InodeSimpleExtendedAttributes) Getxattr(_ *fs.Inode, name string) ([]byte, error) { +func (i *InodeSimpleExtendedAttributes) Getxattr(_ *fs.Inode, name string) (string, error) { i.mu.RLock() value, ok := i.xattrs[name] i.mu.RUnlock() if !ok { - return nil, syserror.ENOATTR + return "", syserror.ENOATTR } return value, nil } // Setxattr implements fs.InodeOperations.Setxattr. -func (i *InodeSimpleExtendedAttributes) Setxattr(_ *fs.Inode, name string, value []byte) error { +func (i *InodeSimpleExtendedAttributes) Setxattr(_ *fs.Inode, name, value string) error { i.mu.Lock() if i.xattrs == nil { - i.xattrs = make(map[string][]byte) + i.xattrs = make(map[string]string) } i.xattrs[name] = value i.mu.Unlock() @@ -424,12 +424,12 @@ func (InodeNotSymlink) Getlink(context.Context, *fs.Inode) (*fs.Dirent, error) { type InodeNoExtendedAttributes struct{} // Getxattr implements fs.InodeOperations.Getxattr. -func (InodeNoExtendedAttributes) Getxattr(*fs.Inode, string) ([]byte, error) { - return nil, syserror.EOPNOTSUPP +func (InodeNoExtendedAttributes) Getxattr(*fs.Inode, string) (string, error) { + return "", syserror.EOPNOTSUPP } // Setxattr implements fs.InodeOperations.Setxattr. -func (InodeNoExtendedAttributes) Setxattr(*fs.Inode, string, []byte) error { +func (InodeNoExtendedAttributes) Setxattr(*fs.Inode, string, string) error { return syserror.EOPNOTSUPP } diff --git a/pkg/sentry/fs/inode.go b/pkg/sentry/fs/inode.go index b8b5c1528..d82f9740e 100644 --- a/pkg/sentry/fs/inode.go +++ b/pkg/sentry/fs/inode.go @@ -253,7 +253,7 @@ func (i *Inode) UnstableAttr(ctx context.Context) (UnstableAttr, error) { } // Getxattr calls i.InodeOperations.Getxattr with i as the Inode. -func (i *Inode) Getxattr(name string) ([]byte, error) { +func (i *Inode) Getxattr(name string) (string, error) { if i.overlay != nil { return overlayGetxattr(i.overlay, name) } diff --git a/pkg/sentry/fs/inode_operations.go b/pkg/sentry/fs/inode_operations.go index e8b9ab96b..ceacc7659 100644 --- a/pkg/sentry/fs/inode_operations.go +++ b/pkg/sentry/fs/inode_operations.go @@ -174,11 +174,11 @@ type InodeOperations interface { // do not support extended attributes return EOPNOTSUPP. Inodes that // support extended attributes but don't have a value at name return // ENODATA. - Getxattr(inode *Inode, name string) ([]byte, error) + Getxattr(inode *Inode, name string) (string, error) // Setxattr sets the value of extended attribute name. Inodes that // do not support extended attributes return EOPNOTSUPP. - Setxattr(inode *Inode, name string, value []byte) error + Setxattr(inode *Inode, name, value string) error // Listxattr returns the set of all extended attributes names that // have values. Inodes that do not support extended attributes return diff --git a/pkg/sentry/fs/inode_overlay.go b/pkg/sentry/fs/inode_overlay.go index 6e1dfecf9..254646176 100644 --- a/pkg/sentry/fs/inode_overlay.go +++ b/pkg/sentry/fs/inode_overlay.go @@ -25,12 +25,12 @@ import ( ) func overlayHasWhiteout(parent *Inode, name string) bool { - buf, err := parent.Getxattr(XattrOverlayWhiteout(name)) - return err == nil && string(buf) == "y" + s, err := parent.Getxattr(XattrOverlayWhiteout(name)) + return err == nil && s == "y" } func overlayCreateWhiteout(parent *Inode, name string) error { - return parent.InodeOperations.Setxattr(parent, XattrOverlayWhiteout(name), []byte("y")) + return parent.InodeOperations.Setxattr(parent, XattrOverlayWhiteout(name), "y") } func overlayWriteOut(ctx context.Context, o *overlayEntry) error { @@ -491,28 +491,28 @@ func overlayUnstableAttr(ctx context.Context, o *overlayEntry) (UnstableAttr, er return attr, err } -func overlayGetxattr(o *overlayEntry, name string) ([]byte, error) { +func overlayGetxattr(o *overlayEntry, name string) (string, error) { // Hot path. This is how the overlay checks for whiteout files. // Avoid defers. var ( - b []byte + s string err error ) // Don't forward the value of the extended attribute if it would // unexpectedly change the behavior of a wrapping overlay layer. if strings.HasPrefix(XattrOverlayPrefix, name) { - return nil, syserror.ENODATA + return "", syserror.ENODATA } o.copyMu.RLock() if o.upper != nil { - b, err = o.upper.Getxattr(name) + s, err = o.upper.Getxattr(name) } else { - b, err = o.lower.Getxattr(name) + s, err = o.lower.Getxattr(name) } o.copyMu.RUnlock() - return b, err + return s, err } func overlayListxattr(o *overlayEntry) (map[string]struct{}, error) { diff --git a/pkg/sentry/fs/inode_overlay_test.go b/pkg/sentry/fs/inode_overlay_test.go index bc91be226..fa8accf6c 100644 --- a/pkg/sentry/fs/inode_overlay_test.go +++ b/pkg/sentry/fs/inode_overlay_test.go @@ -383,13 +383,13 @@ type dir struct { } // Getxattr implements InodeOperations.Getxattr. -func (d *dir) Getxattr(inode *fs.Inode, name string) ([]byte, error) { +func (d *dir) Getxattr(inode *fs.Inode, name string) (string, error) { for _, n := range d.negative { if name == fs.XattrOverlayWhiteout(n) { - return []byte("y"), nil + return "y", nil } } - return nil, syserror.ENOATTR + return "", syserror.ENOATTR } // GetFile implements InodeOperations.GetFile. diff --git a/pkg/sentry/fs/tmpfs/tmpfs.go b/pkg/sentry/fs/tmpfs/tmpfs.go index a1672a4d0..555692505 100644 --- a/pkg/sentry/fs/tmpfs/tmpfs.go +++ b/pkg/sentry/fs/tmpfs/tmpfs.go @@ -150,12 +150,12 @@ func (d *Dir) CreateFifo(ctx context.Context, dir *fs.Inode, name string, perms } // Getxattr implements fs.InodeOperations.Getxattr. -func (d *Dir) Getxattr(i *fs.Inode, name string) ([]byte, error) { +func (d *Dir) Getxattr(i *fs.Inode, name string) (string, error) { return d.ramfsDir.Getxattr(i, name) } // Setxattr implements fs.InodeOperations.Setxattr. -func (d *Dir) Setxattr(i *fs.Inode, name string, value []byte) error { +func (d *Dir) Setxattr(i *fs.Inode, name, value string) error { return d.ramfsDir.Setxattr(i, name, value) } -- cgit v1.2.3