Skip to content

Commit

Permalink
micro optimizations spotted using the go-critic linter
Browse files Browse the repository at this point in the history
  • Loading branch information
@drakkan
drakkan committed Feb 16, 2021
1 parent b1ce6eb commit be9230e
Show file tree
Hide file tree
Showing 29 changed files with 160 additions and 189 deletions.
2 changes: 1 addition & 1 deletion cmd/startsubsys.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ Command-line flags should be specified in the Subsystem declaration.
os.Exit(1)
}
}
err = sftpd.ServeSubSystemConnection(user, connectionID, os.Stdin, os.Stdout)
err = sftpd.ServeSubSystemConnection(&user, connectionID, os.Stdin, os.Stdout)
if err != nil && err != io.EOF {
logger.Warn(logSender, connectionID, "serving subsystem finished with error: %v", err)
os.Exit(1)
Expand Down
14 changes: 7 additions & 7 deletions common/actions.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func SSHCommandActionNotification(user *dataprovider.User, filePath, target, ssh

// ActionHandler handles a notification for a Protocol Action.
type ActionHandler interface {
Handle(notification ActionNotification) error
Handle(notification *ActionNotification) error
}

// ActionNotification defines a notification for a Protocol Action.
Expand All @@ -75,7 +75,7 @@ func newActionNotification(
operation, filePath, target, sshCmd, protocol string,
fileSize int64,
err error,
) ActionNotification {
) *ActionNotification {
var bucket, endpoint string
status := 1

Expand All @@ -99,7 +99,7 @@ func newActionNotification(
status = 0
}

return ActionNotification{
return &ActionNotification{
Action: operation,
Username: user.Username,
Path: filePath,
Expand All @@ -116,7 +116,7 @@ func newActionNotification(

type defaultActionHandler struct{}

func (h *defaultActionHandler) Handle(notification ActionNotification) error {
func (h *defaultActionHandler) Handle(notification *ActionNotification) error {
if !utils.IsStringInSlice(notification.Action, Config.Actions.ExecuteOn) {
return errUnconfiguredAction
}
Expand All @@ -134,7 +134,7 @@ func (h *defaultActionHandler) Handle(notification ActionNotification) error {
return h.handleCommand(notification)
}

func (h *defaultActionHandler) handleHTTP(notification ActionNotification) error {
func (h *defaultActionHandler) handleHTTP(notification *ActionNotification) error {
u, err := url.Parse(Config.Actions.Hook)
if err != nil {
logger.Warn(notification.Protocol, "", "Invalid hook %#v for operation %#v: %v", Config.Actions.Hook, notification.Action, err)
Expand Down Expand Up @@ -165,7 +165,7 @@ func (h *defaultActionHandler) handleHTTP(notification ActionNotification) error
return err
}

func (h *defaultActionHandler) handleCommand(notification ActionNotification) error {
func (h *defaultActionHandler) handleCommand(notification *ActionNotification) error {
if !filepath.IsAbs(Config.Actions.Hook) {
err := fmt.Errorf("invalid notification command %#v", Config.Actions.Hook)
logger.Warn(notification.Protocol, "", "unable to execute notification command: %v", err)
Expand All @@ -188,7 +188,7 @@ func (h *defaultActionHandler) handleCommand(notification ActionNotification) er
return err
}

func notificationAsEnvVars(notification ActionNotification) []string {
func notificationAsEnvVars(notification *ActionNotification) []string {
return []string{
fmt.Sprintf("SFTPGO_ACTION=%v", notification.Action),
fmt.Sprintf("SFTPGO_ACTION_USERNAME=%v", notification.Username),
Expand Down
4 changes: 2 additions & 2 deletions common/actions_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ type actionHandlerStub struct {
called bool
}

func (h *actionHandlerStub) Handle(notification ActionNotification) error {
func (h *actionHandlerStub) Handle(notification *ActionNotification) error {
h.called = true

return nil
Expand All @@ -215,7 +215,7 @@ func TestInitializeActionHandler(t *testing.T) {
InitializeActionHandler(&defaultActionHandler{})
})

err := actionHandler.Handle(ActionNotification{})
err := actionHandler.Handle(&ActionNotification{})

assert.NoError(t, err)
assert.True(t, handler.called)
Expand Down
12 changes: 6 additions & 6 deletions common/common.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -630,13 +630,13 @@ func (conns *ActiveConnections) IsNewConnectionAllowed() bool {
}

// GetStats returns stats for active connections
func (conns *ActiveConnections) GetStats() []ConnectionStatus {
func (conns *ActiveConnections) GetStats() []*ConnectionStatus {
conns.RLock()
defer conns.RUnlock()

stats := make([]ConnectionStatus, 0, len(conns.connections))
stats := make([]*ConnectionStatus, 0, len(conns.connections))
for _, c := range conns.connections {
stat := ConnectionStatus{
stat := &ConnectionStatus{
Username: c.GetUsername(),
ConnectionID: c.GetID(),
ClientVersion: c.GetClientVersion(),
Expand Down Expand Up @@ -675,14 +675,14 @@ type ConnectionStatus struct {
}

// GetConnectionDuration returns the connection duration as string
func (c ConnectionStatus) GetConnectionDuration() string {
func (c *ConnectionStatus) GetConnectionDuration() string {
elapsed := time.Since(utils.GetTimeFromMsecSinceEpoch(c.ConnectionTime))
return utils.GetDurationAsString(elapsed)
}

// GetConnectionInfo returns connection info.
// Protocol,Client Version and RemoteAddress are returned.
func (c ConnectionStatus) GetConnectionInfo() string {
func (c *ConnectionStatus) GetConnectionInfo() string {
var result strings.Builder

result.WriteString(fmt.Sprintf("%v. Client: %#v From: %#v", c.Protocol, c.ClientVersion, c.RemoteAddress))
Expand All @@ -702,7 +702,7 @@ func (c ConnectionStatus) GetConnectionInfo() string {
}

// GetTransfersAsString returns the active transfers as string
func (c ConnectionStatus) GetTransfersAsString() string {
func (c *ConnectionStatus) GetTransfersAsString() string {
result := ""
for _, t := range c.Transfers {
if result != "" {
Expand Down
66 changes: 33 additions & 33 deletions common/connection.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ type BaseConnection struct {
}

// NewBaseConnection returns a new BaseConnection
func NewBaseConnection(ID, protocol string, user dataprovider.User, fs vfs.Fs) *BaseConnection {
connID := ID
func NewBaseConnection(id, protocol string, user dataprovider.User, fs vfs.Fs) *BaseConnection {
connID := id
if utils.IsStringInSlice(protocol, supportedProtocols) {
connID = fmt.Sprintf("%v_%v", protocol, ID)
connID = fmt.Sprintf("%v_%v", protocol, id)
}
return &BaseConnection{
ID: connID,
Expand Down Expand Up @@ -272,12 +272,12 @@ func (c *BaseConnection) RemoveFile(fsPath, virtualPath string, info os.FileInfo
if info.Mode()&os.ModeSymlink == 0 {
vfolder, err := c.User.GetVirtualFolderForPath(path.Dir(virtualPath))
if err == nil {
dataprovider.UpdateVirtualFolderQuota(vfolder.BaseVirtualFolder, -1, -size, false) //nolint:errcheck
dataprovider.UpdateVirtualFolderQuota(&vfolder.BaseVirtualFolder, -1, -size, false) //nolint:errcheck
if vfolder.IsIncludedInUserQuota() {
dataprovider.UpdateUserQuota(c.User, -1, -size, false) //nolint:errcheck
dataprovider.UpdateUserQuota(&c.User, -1, -size, false) //nolint:errcheck
}
} else {
dataprovider.UpdateUserQuota(c.User, -1, -size, false) //nolint:errcheck
dataprovider.UpdateUserQuota(&c.User, -1, -size, false) //nolint:errcheck
}
}
if actionErr != nil {
Expand Down Expand Up @@ -577,12 +577,12 @@ func (c *BaseConnection) truncateFile(fsPath, virtualPath string, size int64) er
sizeDiff := initialSize - size
vfolder, err := c.User.GetVirtualFolderForPath(path.Dir(virtualPath))
if err == nil {
dataprovider.UpdateVirtualFolderQuota(vfolder.BaseVirtualFolder, 0, -sizeDiff, false) //nolint:errcheck
dataprovider.UpdateVirtualFolderQuota(&vfolder.BaseVirtualFolder, 0, -sizeDiff, false) //nolint:errcheck
if vfolder.IsIncludedInUserQuota() {
dataprovider.UpdateUserQuota(c.User, 0, -sizeDiff, false) //nolint:errcheck
dataprovider.UpdateUserQuota(&c.User, 0, -sizeDiff, false) //nolint:errcheck
}
} else {
dataprovider.UpdateUserQuota(c.User, 0, -sizeDiff, false) //nolint:errcheck
dataprovider.UpdateUserQuota(&c.User, 0, -sizeDiff, false) //nolint:errcheck
}
}
return err
Expand Down Expand Up @@ -835,64 +835,64 @@ func (c *BaseConnection) isCrossFoldersRequest(virtualSourcePath, virtualTargetP
return true
}

func (c *BaseConnection) updateQuotaMoveBetweenVFolders(sourceFolder, dstFolder vfs.VirtualFolder, initialSize,
func (c *BaseConnection) updateQuotaMoveBetweenVFolders(sourceFolder, dstFolder *vfs.VirtualFolder, initialSize,
filesSize int64, numFiles int) {
if sourceFolder.MappedPath == dstFolder.MappedPath {
// both files are inside the same virtual folder
if initialSize != -1 {
dataprovider.UpdateVirtualFolderQuota(dstFolder.BaseVirtualFolder, -numFiles, -initialSize, false) //nolint:errcheck
dataprovider.UpdateVirtualFolderQuota(&dstFolder.BaseVirtualFolder, -numFiles, -initialSize, false) //nolint:errcheck
if dstFolder.IsIncludedInUserQuota() {
dataprovider.UpdateUserQuota(c.User, -numFiles, -initialSize, false) //nolint:errcheck
dataprovider.UpdateUserQuota(&c.User, -numFiles, -initialSize, false) //nolint:errcheck
}
}
return
}
// files are inside different virtual folders
dataprovider.UpdateVirtualFolderQuota(sourceFolder.BaseVirtualFolder, -numFiles, -filesSize, false) //nolint:errcheck
dataprovider.UpdateVirtualFolderQuota(&sourceFolder.BaseVirtualFolder, -numFiles, -filesSize, false) //nolint:errcheck
if sourceFolder.IsIncludedInUserQuota() {
dataprovider.UpdateUserQuota(c.User, -numFiles, -filesSize, false) //nolint:errcheck
dataprovider.UpdateUserQuota(&c.User, -numFiles, -filesSize, false) //nolint:errcheck
}
if initialSize == -1 {
dataprovider.UpdateVirtualFolderQuota(dstFolder.BaseVirtualFolder, numFiles, filesSize, false) //nolint:errcheck
dataprovider.UpdateVirtualFolderQuota(&dstFolder.BaseVirtualFolder, numFiles, filesSize, false) //nolint:errcheck
if dstFolder.IsIncludedInUserQuota() {
dataprovider.UpdateUserQuota(c.User, numFiles, filesSize, false) //nolint:errcheck
dataprovider.UpdateUserQuota(&c.User, numFiles, filesSize, false) //nolint:errcheck
}
} else {
// we cannot have a directory here, initialSize != -1 only for files
dataprovider.UpdateVirtualFolderQuota(dstFolder.BaseVirtualFolder, 0, filesSize-initialSize, false) //nolint:errcheck
dataprovider.UpdateVirtualFolderQuota(&dstFolder.BaseVirtualFolder, 0, filesSize-initialSize, false) //nolint:errcheck
if dstFolder.IsIncludedInUserQuota() {
dataprovider.UpdateUserQuota(c.User, 0, filesSize-initialSize, false) //nolint:errcheck
dataprovider.UpdateUserQuota(&c.User, 0, filesSize-initialSize, false) //nolint:errcheck
}
}
}

func (c *BaseConnection) updateQuotaMoveFromVFolder(sourceFolder vfs.VirtualFolder, initialSize, filesSize int64, numFiles int) {
func (c *BaseConnection) updateQuotaMoveFromVFolder(sourceFolder *vfs.VirtualFolder, initialSize, filesSize int64, numFiles int) {
// move between a virtual folder and the user home dir
dataprovider.UpdateVirtualFolderQuota(sourceFolder.BaseVirtualFolder, -numFiles, -filesSize, false) //nolint:errcheck
dataprovider.UpdateVirtualFolderQuota(&sourceFolder.BaseVirtualFolder, -numFiles, -filesSize, false) //nolint:errcheck
if sourceFolder.IsIncludedInUserQuota() {
dataprovider.UpdateUserQuota(c.User, -numFiles, -filesSize, false) //nolint:errcheck
dataprovider.UpdateUserQuota(&c.User, -numFiles, -filesSize, false) //nolint:errcheck
}
if initialSize == -1 {
dataprovider.UpdateUserQuota(c.User, numFiles, filesSize, false) //nolint:errcheck
dataprovider.UpdateUserQuota(&c.User, numFiles, filesSize, false) //nolint:errcheck
} else {
// we cannot have a directory here, initialSize != -1 only for files
dataprovider.UpdateUserQuota(c.User, 0, filesSize-initialSize, false) //nolint:errcheck
dataprovider.UpdateUserQuota(&c.User, 0, filesSize-initialSize, false) //nolint:errcheck
}
}

func (c *BaseConnection) updateQuotaMoveToVFolder(dstFolder vfs.VirtualFolder, initialSize, filesSize int64, numFiles int) {
func (c *BaseConnection) updateQuotaMoveToVFolder(dstFolder *vfs.VirtualFolder, initialSize, filesSize int64, numFiles int) {
// move between the user home dir and a virtual folder
dataprovider.UpdateUserQuota(c.User, -numFiles, -filesSize, false) //nolint:errcheck
dataprovider.UpdateUserQuota(&c.User, -numFiles, -filesSize, false) //nolint:errcheck
if initialSize == -1 {
dataprovider.UpdateVirtualFolderQuota(dstFolder.BaseVirtualFolder, numFiles, filesSize, false) //nolint:errcheck
dataprovider.UpdateVirtualFolderQuota(&dstFolder.BaseVirtualFolder, numFiles, filesSize, false) //nolint:errcheck
if dstFolder.IsIncludedInUserQuota() {
dataprovider.UpdateUserQuota(c.User, numFiles, filesSize, false) //nolint:errcheck
dataprovider.UpdateUserQuota(&c.User, numFiles, filesSize, false) //nolint:errcheck
}
} else {
// we cannot have a directory here, initialSize != -1 only for files
dataprovider.UpdateVirtualFolderQuota(dstFolder.BaseVirtualFolder, 0, filesSize-initialSize, false) //nolint:errcheck
dataprovider.UpdateVirtualFolderQuota(&dstFolder.BaseVirtualFolder, 0, filesSize-initialSize, false) //nolint:errcheck
if dstFolder.IsIncludedInUserQuota() {
dataprovider.UpdateUserQuota(c.User, 0, filesSize-initialSize, false) //nolint:errcheck
dataprovider.UpdateUserQuota(&c.User, 0, filesSize-initialSize, false) //nolint:errcheck
}
}
}
Expand All @@ -909,7 +909,7 @@ func (c *BaseConnection) updateQuotaAfterRename(virtualSourcePath, virtualTarget
// both files are contained inside the user home dir
if initialSize != -1 {
// we cannot have a directory here
dataprovider.UpdateUserQuota(c.User, -1, -initialSize, false) //nolint:errcheck
dataprovider.UpdateUserQuota(&c.User, -1, -initialSize, false) //nolint:errcheck
}
return nil
}
Expand All @@ -932,13 +932,13 @@ func (c *BaseConnection) updateQuotaAfterRename(virtualSourcePath, virtualTarget
return err
}
if errSrc == nil && errDst == nil {
c.updateQuotaMoveBetweenVFolders(sourceFolder, dstFolder, initialSize, filesSize, numFiles)
c.updateQuotaMoveBetweenVFolders(&sourceFolder, &dstFolder, initialSize, filesSize, numFiles)
}
if errSrc == nil && errDst != nil {
c.updateQuotaMoveFromVFolder(sourceFolder, initialSize, filesSize, numFiles)
c.updateQuotaMoveFromVFolder(&sourceFolder, initialSize, filesSize, numFiles)
}
if errSrc != nil && errDst == nil {
c.updateQuotaMoveToVFolder(dstFolder, initialSize, filesSize, numFiles)
c.updateQuotaMoveToVFolder(&dstFolder, initialSize, filesSize, numFiles)
}
return nil
}
Expand Down
16 changes: 8 additions & 8 deletions common/connection_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1054,7 +1054,7 @@ func TestHasSpace(t *testing.T) {

folder, err := dataprovider.GetFolderByName(folderName)
assert.NoError(t, err)
err = dataprovider.UpdateVirtualFolderQuota(folder, 10, 1048576, true)
err = dataprovider.UpdateVirtualFolderQuota(&folder, 10, 1048576, true)
assert.NoError(t, err)
quotaResult = c.HasSpace(true, false, "/vdir/file1")
assert.False(t, quotaResult.HasSpace)
Expand Down Expand Up @@ -1105,14 +1105,14 @@ func TestUpdateQuotaMoveVFolders(t *testing.T) {
assert.NoError(t, err)
folder2, err := dataprovider.GetFolderByName(folderName2)
assert.NoError(t, err)
err = dataprovider.UpdateVirtualFolderQuota(folder1, 1, 100, true)
err = dataprovider.UpdateVirtualFolderQuota(&folder1, 1, 100, true)
assert.NoError(t, err)
err = dataprovider.UpdateVirtualFolderQuota(folder2, 2, 150, true)
err = dataprovider.UpdateVirtualFolderQuota(&folder2, 2, 150, true)
assert.NoError(t, err)
fs, err := user.GetFilesystem("id")
assert.NoError(t, err)
c := NewBaseConnection("", ProtocolSFTP, user, fs)
c.updateQuotaMoveBetweenVFolders(user.VirtualFolders[0], user.VirtualFolders[1], -1, 100, 1)
c.updateQuotaMoveBetweenVFolders(&user.VirtualFolders[0], &user.VirtualFolders[1], -1, 100, 1)
folder1, err = dataprovider.GetFolderByName(folderName1)
assert.NoError(t, err)
assert.Equal(t, 0, folder1.UsedQuotaFiles)
Expand All @@ -1122,7 +1122,7 @@ func TestUpdateQuotaMoveVFolders(t *testing.T) {
assert.Equal(t, 3, folder2.UsedQuotaFiles)
assert.Equal(t, int64(250), folder2.UsedQuotaSize)

c.updateQuotaMoveBetweenVFolders(user.VirtualFolders[1], user.VirtualFolders[0], 10, 100, 1)
c.updateQuotaMoveBetweenVFolders(&user.VirtualFolders[1], &user.VirtualFolders[0], 10, 100, 1)
folder1, err = dataprovider.GetFolderByName(folderName1)
assert.NoError(t, err)
assert.Equal(t, 0, folder1.UsedQuotaFiles)
Expand All @@ -1132,9 +1132,9 @@ func TestUpdateQuotaMoveVFolders(t *testing.T) {
assert.Equal(t, 2, folder2.UsedQuotaFiles)
assert.Equal(t, int64(150), folder2.UsedQuotaSize)

err = dataprovider.UpdateUserQuota(user, 1, 100, true)
err = dataprovider.UpdateUserQuota(&user, 1, 100, true)
assert.NoError(t, err)
c.updateQuotaMoveFromVFolder(user.VirtualFolders[1], -1, 50, 1)
c.updateQuotaMoveFromVFolder(&user.VirtualFolders[1], -1, 50, 1)
folder2, err = dataprovider.GetFolderByName(folderName2)
assert.NoError(t, err)
assert.Equal(t, 1, folder2.UsedQuotaFiles)
Expand All @@ -1144,7 +1144,7 @@ func TestUpdateQuotaMoveVFolders(t *testing.T) {
assert.Equal(t, 1, user.UsedQuotaFiles)
assert.Equal(t, int64(100), user.UsedQuotaSize)

c.updateQuotaMoveToVFolder(user.VirtualFolders[1], -1, 100, 1)
c.updateQuotaMoveToVFolder(&user.VirtualFolders[1], -1, 100, 1)
folder2, err = dataprovider.GetFolderByName(folderName2)
assert.NoError(t, err)
assert.Equal(t, 2, folder2.UsedQuotaFiles)
Expand Down
6 changes: 3 additions & 3 deletions common/transfer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -266,13 +266,13 @@ func (t *BaseTransfer) updateQuota(numFiles int, fileSize int64) bool {
if t.transferType == TransferUpload && (numFiles != 0 || sizeDiff > 0) {
vfolder, err := t.Connection.User.GetVirtualFolderForPath(path.Dir(t.requestPath))
if err == nil {
dataprovider.UpdateVirtualFolderQuota(vfolder.BaseVirtualFolder, numFiles, //nolint:errcheck
dataprovider.UpdateVirtualFolderQuota(&vfolder.BaseVirtualFolder, numFiles, //nolint:errcheck
sizeDiff, false)
if vfolder.IsIncludedInUserQuota() {
dataprovider.UpdateUserQuota(t.Connection.User, numFiles, sizeDiff, false) //nolint:errcheck
dataprovider.UpdateUserQuota(&t.Connection.User, numFiles, sizeDiff, false) //nolint:errcheck
}
} else {
dataprovider.UpdateUserQuota(t.Connection.User, numFiles, sizeDiff, false) //nolint:errcheck
dataprovider.UpdateUserQuota(&t.Connection.User, numFiles, sizeDiff, false) //nolint:errcheck
}
return true
}
Expand Down
7 changes: 3 additions & 4 deletions dataprovider/bolt.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ const (
)

var (
usersBucket = []byte("users")
//usersIDIdxBucket = []byte("users_id_idx")
usersBucket = []byte("users")
foldersBucket = []byte("folders")
adminsBucket = []byte("admins")
dbVersionBucket = []byte("db_version")
Expand Down Expand Up @@ -113,7 +112,7 @@ func (p *BoltProvider) validateUserAndPass(username, password, ip, protocol stri
providerLog(logger.LevelWarn, "error authenticating user %#v: %v", username, err)
return user, err
}
return checkUserAndPass(user, password, ip, protocol)
return checkUserAndPass(&user, password, ip, protocol)
}

func (p *BoltProvider) validateAdminAndPass(username, password, ip string) (Admin, error) {
Expand All @@ -136,7 +135,7 @@ func (p *BoltProvider) validateUserAndPubKey(username string, pubKey []byte) (Us
providerLog(logger.LevelWarn, "error authenticating user %#v: %v", username, err)
return user, "", err
}
return checkUserAndPubKey(user, pubKey)
return checkUserAndPubKey(&user, pubKey)
}

func (p *BoltProvider) updateLastLogin(username string) error {
Expand Down
Loading

0 comments on commit be9230e

Please sign in to comment.